Skip to content

Commit

Permalink
Add unmarshaling tests for 64-bit immediate instructions
Browse files Browse the repository at this point in the history 
Blocked on answer to
ietf-wg-bpf/ebpf-docs#75

Signed-off-by: Dave Thaler <[email protected]>
  • Loading branch information
@dthaler @elazarg
dthaler authored and elazarg committed Feb 5, 2024
1 parent 96f4333 commit 55478db
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 5 deletions.
10 changes: 6 additions & 4 deletions src/asm_unmarshal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,19 +310,21 @@ struct Unmarshaller {
auto makeLddw(ebpf_inst inst, int32_t next_imm, const vector<ebpf_inst>& insts, pc_t pc) -> Instruction {
if (pc >= insts.size() - 1)
throw InvalidInstruction(pc, "incomplete LDDW");
ebpf_inst next = insts[pc + 1];
if (next.opcode != 0 || next.dst != 0 || next.src != 0 || next.offset != 0)
throw InvalidInstruction(pc, "invalid LDDW");
if (inst.src > 1 || inst.dst > R10_STACK_POINTER || inst.offset != 0)
throw InvalidInstruction(pc, "LDDW uses reserved fields");

if (inst.src == 1) {
// magic number, meaning we're a per-process file descriptor defining the map.
// (for details, look for BPF_PSEUDO_MAP_FD in the kernel)

if (next.imm != 0) {
throw InvalidInstruction(pc, "LDDW uses reserved fields");
}
return LoadMapFd{.dst = Reg{inst.dst}, .mapfd = inst.imm};
}

ebpf_inst next = insts[pc + 1];
if (next.opcode != 0 || next.dst != 0 || next.src != 0 || next.offset != 0)
throw InvalidInstruction(pc, "invalid LDDW");
return Bin{
.op = Bin::Op::MOV,
.dst = Reg{inst.dst},
Expand Down
64 changes: 63 additions & 1 deletion src/test/test_marshal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,26 @@ static void compare_unmarshal_marshal(const ebpf_inst& ins, const ebpf_inst& exp
REQUIRE(memcmp(&expected_result, &result, sizeof(result)) == 0);
}

// Verify that if we unmarshal a 64-bit immediate instruction and then re-marshal it,
// we get what we expect.
static void compare_unmarshal_marshal(const ebpf_inst& ins1, const ebpf_inst& ins2, const ebpf_inst& expected_result1,
const ebpf_inst& expected_result2) {
program_info info{.platform = &g_ebpf_platform_linux,
.type = g_ebpf_platform_linux.get_program_type("unspec", "unspec")};
const ebpf_inst exit{.opcode = INST_OP_EXIT};
InstructionSeq parsed = std::get<InstructionSeq>(unmarshal(raw_program{"", "", {ins1, ins2, exit, exit}, info}));
REQUIRE(parsed.size() == 3);
auto [_, single, _2] = parsed.front();
(void)_; // unused
(void)_2; // unused
std::vector<ebpf_inst> marshaled = marshal(single, 0);
REQUIRE(marshaled.size() == 2);
ebpf_inst result1 = marshaled.front();
REQUIRE(memcmp(&expected_result1, &result1, sizeof(result1)) == 0);
ebpf_inst result2 = marshaled.back();
REQUIRE(memcmp(&expected_result2, &result2, sizeof(result2)) == 0);
}

// Verify that if we marshal an instruction and then unmarshal it,
// we get the original.
static void compare_marshal_unmarshal(const Instruction& ins, bool double_cmd = false) {
Expand Down Expand Up @@ -53,6 +73,17 @@ static void check_unmarshal_fail(ebpf_inst inst, std::string expected_error_mess
REQUIRE(error_message == expected_error_message);
}

// Check that unmarshaling a 64-bit immediate instruction fails.
static void check_unmarshal_fail(ebpf_inst inst1, ebpf_inst inst2, std::string expected_error_message) {
program_info info{.platform = &g_ebpf_platform_linux,
.type = g_ebpf_platform_linux.get_program_type("unspec", "unspec")};
std::vector<ebpf_inst> insns = {inst1, inst2};
auto result = unmarshal(raw_program{"", "", insns, info});
REQUIRE(std::holds_alternative<std::string>(result));
std::string error_message = std::get<std::string>(result);
REQUIRE(error_message == expected_error_message);
}

static const auto ws = {1, 2, 4, 8};

TEST_CASE("disasm_marshal", "[disasm][marshal]") {
Expand Down Expand Up @@ -301,10 +332,41 @@ TEST_CASE("fail unmarshal offset opcodes", "[disasm][marshal]") {
}
}

TEST_CASE("unmarshal 64bit immediate", "[disasm][marshal]") {
compare_unmarshal_marshal(ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM, .src = 0, .imm = 1}, ebpf_inst{.imm = 2},
ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM, .src = 0, .imm = 1}, ebpf_inst{.imm = 2});
compare_unmarshal_marshal(ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM, .src = 0, .imm = 1}, ebpf_inst{},
ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM, .src = 0, .imm = 1}, ebpf_inst{});

for (uint8_t src = 0; src <= 7; src++) {
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM, .src = src}, "0: incomplete LDDW\n");
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM, .src = src},
ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM}, "0: invalid LDDW\n");
}

// No supported src values use the offset field.
for (uint8_t src = 0; src <= 1; src++) {
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM, .src = src, .offset = 1}, ebpf_inst{},
"0: LDDW uses reserved fields\n");
}

// Verify that unsupported src values fail.
// TODO: support src = 2 through 6.
for (uint8_t src = 2; src <= 7; src++) {
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM, .src = src}, ebpf_inst{},
"0: LDDW uses reserved fields\n");
}

// When src = {1, 3, 4, 5}, next_imm must be 0.
for (uint8_t src : {1, 3, 4, 5}) {
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM, .src = src}, ebpf_inst{.imm = 1},
"0: LDDW uses reserved fields\n");
}
}

TEST_CASE("fail unmarshal misc", "[disasm][marshal]") {
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x06 */ INST_CLS_JMP32}, "0: jump out of bounds\n");
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x16 */ 0x10 | INST_CLS_JMP32}, "0: jump out of bounds\n");
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x18 */ INST_OP_LDDW_IMM}, "0: incomplete LDDW\n");
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x21 */ (INST_ABS << 5) | INST_SIZE_W | INST_CLS_LDX, .imm = 8},
"0: ABS but not LD\n");
check_unmarshal_fail(ebpf_inst{.opcode = /* 0x41 */ (INST_IND << 5) | INST_SIZE_W | INST_CLS_LDX, .imm = 8},
Expand Down

0 comments on commit 55478db

Please sign in to comment.