Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more negative unmarshaling tests #591

Merged
merged 7 commits into from
Feb 28, 2024
Merged

Add more negative unmarshaling tests #591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f0b7cda
Add more negative unmarshaling tests
dthaler Feb 15, 2024
f365b7b
WIP: table based tests
dthaler Feb 19, 2024
6d36b37
Remove debugging output
dthaler Feb 21, 2024
7a8a0c4
Add comments
dthaler Feb 21, 2024
0e96fca
Address PR feedback
dthaler Feb 27, 2024
78fdc63
Lower case unmarshaling errors for consistency
dthaler Feb 28, 2024
4dc72eb
Simplify c_str uses
dthaler Feb 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 48 additions & 18 deletions src/asm_unmarshal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ struct InvalidInstruction : std::invalid_argument {
size_t pc;
explicit InvalidInstruction(size_t pc, const char* what) : std::invalid_argument{what}, pc{pc} {}
InvalidInstruction(size_t pc, std::string what) : std::invalid_argument{what}, pc{pc} {}
InvalidInstruction(size_t pc, uint8_t opcode) : std::invalid_argument{make_opcode_message("Bad instruction", opcode)}, pc{pc} {}
InvalidInstruction(size_t pc, uint8_t opcode) : std::invalid_argument{make_opcode_message("bad instruction", opcode)}, pc{pc} {}
};

struct UnsupportedMemoryMode : std::invalid_argument {
Expand Down Expand Up @@ -115,7 +115,7 @@ struct Unmarshaller {
}
case INST_ALU_OP_MOV:
if (inst.offset > 0 && !(inst.opcode & INST_SRC_REG))
throw InvalidInstruction(pc, make_opcode_message("invalid offset for", inst.opcode));
throw InvalidInstruction(pc, make_opcode_message("nonzero offset for", inst.opcode));
switch (inst.offset) {
case 0: return Bin::Op::MOV;
case 8: return Bin::Op::MOVSX8;
Expand All @@ -142,7 +142,7 @@ struct Unmarshaller {
if (inst.opcode & INST_SRC_REG)
throw InvalidInstruction{pc, inst.opcode};
if (inst.src != 0)
throw InvalidInstruction{pc, make_opcode_message("nonzero src for register", inst.opcode)};
throw InvalidInstruction{pc, inst.opcode};
if (inst.imm != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));
return Un::Op::NEG;
Expand All @@ -153,23 +153,23 @@ struct Unmarshaller {
return Bin::Op::ARSH;
case INST_ALU_OP_END:
if (inst.src != 0)
throw InvalidInstruction{pc, make_opcode_message("nonzero src for register", inst.opcode)};
throw InvalidInstruction{pc, inst.opcode};
if ((inst.opcode & INST_CLS_MASK) == INST_CLS_ALU64) {
if (inst.opcode & INST_END_BE)
throw InvalidInstruction(pc, inst.opcode);
switch (inst.imm) {
case 16: return Un::Op::SWAP16;
case 32: return Un::Op::SWAP32;
case 64: return Un::Op::SWAP64;
default: throw InvalidInstruction(pc, "invalid endian immediate");
default: throw InvalidInstruction(pc, "unsupported immediate");
}
}
switch (inst.imm) {
case 16: return (inst.opcode & INST_END_BE) ? Un::Op::BE16 : Un::Op::LE16;
case 32: return (inst.opcode & INST_END_BE) ? Un::Op::BE32 : Un::Op::LE32;
case 64: return (inst.opcode & INST_END_BE) ? Un::Op::BE64 : Un::Op::LE64;
default:
throw InvalidInstruction(pc, "invalid endian immediate");
throw InvalidInstruction(pc, "unsupported immediate");
}
case 0xe0: throw InvalidInstruction{pc, inst.opcode};
case 0xf0: throw InvalidInstruction{pc, inst.opcode};
Expand All @@ -188,7 +188,7 @@ struct Unmarshaller {
return Reg{inst.src};
} else {
if (inst.src != 0)
throw InvalidInstruction{pc, make_opcode_message("nonzero src for register", inst.opcode)};
throw InvalidInstruction{pc, inst.opcode};
// Imm is a signed 32-bit number. Sign extend it to 64-bits for storage.
return Imm{sign_extend(inst.imm)};
}
Expand Down Expand Up @@ -219,7 +219,7 @@ struct Unmarshaller {

auto makeMemOp(pc_t pc, ebpf_inst inst) -> Instruction {
if (inst.dst > R10_STACK_POINTER || inst.src > R10_STACK_POINTER)
throw InvalidInstruction(pc, "Bad register");
throw InvalidInstruction(pc, "bad register");

int width = getMemWidth(inst.opcode);
bool isLD = (inst.opcode & INST_CLS_MASK) == INST_CLS_LD;
Expand All @@ -228,11 +228,23 @@ struct Unmarshaller {
case INST_ABS:
if (!info.platform->legacy || !isLD || (width == 8))
throw InvalidInstruction(pc, inst.opcode);
if (inst.dst != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero dst for register", inst.opcode));
if (inst.src > 0)
throw InvalidInstruction(pc, make_opcode_message("bad instruction", inst.opcode));
if (inst.offset != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero offset for", inst.opcode));
return Packet{.width = width, .offset = inst.imm, .regoffset = {}};

case INST_IND:
if (!info.platform->legacy || !isLD || (width == 8))
throw InvalidInstruction(pc, inst.opcode);
if (inst.dst != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero dst for register", inst.opcode));
if (inst.src > R10_STACK_POINTER)
throw InvalidInstruction(pc, "bad register");
if (inst.offset != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero offset for", inst.opcode));
return Packet{.width = width, .offset = inst.imm, .regoffset = Reg{inst.src}};

case INST_MEM: {
Expand All @@ -243,7 +255,7 @@ struct Unmarshaller {
throw InvalidInstruction(pc, "Cannot modify r10");
bool isImm = !(inst.opcode & 1);
if (isImm && inst.src != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero src for register", inst.opcode));
throw InvalidInstruction(pc, inst.opcode);
if (!isImm && inst.imm != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));

Expand Down Expand Up @@ -273,7 +285,7 @@ struct Unmarshaller {
(inst.opcode & INST_SIZE_MASK) != INST_SIZE_DW))
throw InvalidInstruction(pc, inst.opcode);
if (inst.imm != 0)
throw InvalidInstruction(pc, "Unsupported atomic instruction");
throw InvalidInstruction(pc, "unsupported immediate");
return LockAdd{
.access =
Deref{
Expand All @@ -292,7 +304,7 @@ struct Unmarshaller {
if (inst.dst == R10_STACK_POINTER)
throw InvalidInstruction(pc, "Invalid target r10");
if (inst.dst > R10_STACK_POINTER || inst.src > R10_STACK_POINTER)
throw InvalidInstruction(pc, "Bad register");
throw InvalidInstruction(pc, "bad register");
bool is64 = (inst.opcode & INST_CLS_MASK) == INST_CLS_ALU64;
return std::visit(overloaded{[&](Un::Op op) -> Instruction { return Un{.op = op, .dst = Reg{inst.dst}, .is64 = is64}; },
[&](Bin::Op op) -> Instruction {
Expand All @@ -312,18 +324,22 @@ 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");
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");
throw InvalidInstruction(pc, "invalid lddw");
if (inst.src > 1)
throw InvalidInstruction(pc, make_opcode_message("bad instruction", inst.opcode));
if (inst.offset != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero offset for", inst.opcode));
if (inst.dst > R10_STACK_POINTER)
throw InvalidInstruction(pc, "bad register");

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");
throw InvalidInstruction(pc, "lddw uses reserved fields");
}
return LoadMapFd{.dst = Reg{inst.dst}, .mapfd = inst.imm};
}
Expand Down Expand Up @@ -413,16 +429,22 @@ struct Unmarshaller {
throw InvalidInstruction(pc, inst.opcode);
if (inst.opcode & INST_SRC_REG)
throw InvalidInstruction(pc, inst.opcode);
if (inst.src > 0)
throw InvalidInstruction(pc, inst.opcode);
if (inst.offset != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero offset for", inst.opcode));
if (inst.dst != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero dst for register", inst.opcode));
if (!info.platform->is_helper_usable(inst.imm))
throw InvalidInstruction(pc, "invalid helper function id");
return makeCall(inst.imm);
case INST_EXIT:
if ((inst.opcode & INST_CLS_MASK) != INST_CLS_JMP || (inst.opcode & INST_SRC_REG))
throw InvalidInstruction(pc, inst.opcode);
if (inst.src != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero src for register", inst.opcode));
throw InvalidInstruction(pc, inst.opcode);
if (inst.dst != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero dst for register", inst.opcode));
if (inst.imm != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));
if (inst.offset != 0)
Expand All @@ -438,11 +460,13 @@ struct Unmarshaller {
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));
if ((inst.opcode & INST_CLS_MASK) == INST_CLS_JMP32 && (inst.offset != 0))
throw InvalidInstruction(pc, make_opcode_message("nonzero offset for", inst.opcode));
if (inst.dst != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero dst for register", inst.opcode));
default: {
// First validate the opcode, src, and imm.
auto op = getJmpOp(pc, inst.opcode);
if (!(inst.opcode & INST_SRC_REG) && (inst.src != 0))
throw InvalidInstruction(pc, make_opcode_message("nonzero src for register", inst.opcode));
throw InvalidInstruction(pc, inst.opcode);
if ((inst.opcode & INST_SRC_REG) && (inst.imm != 0))
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));

Expand All @@ -452,6 +476,12 @@ struct Unmarshaller {
throw InvalidInstruction(pc, "jump out of bounds");
else if (insts[new_pc].opcode == 0)
throw InvalidInstruction(pc, "jump to middle of lddw");
if (inst.opcode != INST_OP_JA16 && inst.opcode != INST_OP_JA32) {
if (inst.dst > R10_STACK_POINTER)
throw InvalidInstruction(pc, "bad register");
if ((inst.opcode & INST_SRC_REG) && (inst.src > R10_STACK_POINTER))
throw InvalidInstruction(pc, "bad register");
}

auto cond = (inst.opcode == INST_OP_JA16 || inst.opcode == INST_OP_JA32)
? std::optional<Condition>{}
Expand Down
1 change: 1 addition & 0 deletions src/ebpf_vm_isa.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ struct ebpf_inst {
std::uint8_t src : 4; //< Source register
std::int16_t offset;
std::int32_t imm; //< Immediate constant
constexpr bool operator==(const ebpf_inst&) const = default;
};

enum {
Expand Down
2 changes: 1 addition & 1 deletion src/test/test_conformance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ void test_conformance(std::string filename, bpf_conformance_test_result_t expect
test_path.remove_filename().append("conformance_check" + extension.string()).string();
std::map<std::filesystem::path, std::tuple<bpf_conformance_test_result_t, std::string>> result = bpf_conformance(
test_files, plugin_path, {}, {}, {}, bpf_conformance_test_CPU_version_t::v4,
_bpf_conformance_groups::default_groups, bpf_conformance_list_instructions_t::LIST_INSTRUCTIONS_NONE, true);
bpf_conformance_groups_t::default_groups, bpf_conformance_list_instructions_t::LIST_INSTRUCTIONS_NONE, true);
for (auto file : test_files) {
auto& [file_result, reason] = result[file];
REQUIRE(file_result == expected_result);
Expand Down
Loading
Loading