Skip to content

Commit

Permalink
Add more negative unmarshaling tests (#576)
Browse files Browse the repository at this point in the history 
* Add more negative unmarshaling test cases
* Bug fixes
* Test invalid opcodes and src0 opcodes
* Test invalid imm0 opcodes
* Test invalid off0 opcodes

Signed-off-by: Dave Thaler <[email protected]>
  • Loading branch information
@dthaler
dthaler authored Jan 25, 2024
1 parent 8b0c660 commit 47d78f4
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 67 deletions.
8 changes: 4 additions & 4 deletions src/asm_marshal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ struct MarshalVisitor {
case Un::Op::NEG:
return {ebpf_inst{
// FIX: should be INST_CLS_ALU / INST_CLS_ALU64
.opcode = static_cast<uint8_t>(INST_CLS_ALU | 0x3 | (0x8 << 4)),
.opcode = static_cast<uint8_t>(INST_CLS_ALU | 0x3 | INST_ALU_OP_NEG),
.dst = b.dst.v,
.src = 0,
.offset = 0,
Expand All @@ -143,7 +143,7 @@ struct MarshalVisitor {
case Un::Op::LE32:
case Un::Op::LE64:
return {ebpf_inst{
.opcode = static_cast<uint8_t>(INST_CLS_ALU | (0xd << 4)),
.opcode = static_cast<uint8_t>(INST_CLS_ALU | INST_ALU_OP_END),
.dst = b.dst.v,
.src = 0,
.offset = 0,
Expand All @@ -153,7 +153,7 @@ struct MarshalVisitor {
case Un::Op::BE32:
case Un::Op::BE64:
return {ebpf_inst{
.opcode = static_cast<uint8_t>(INST_CLS_ALU | 0x8 | (0xd << 4)),
.opcode = static_cast<uint8_t>(INST_CLS_ALU | INST_END_BE | INST_ALU_OP_END),
.dst = b.dst.v,
.src = 0,
.offset = 0,
Expand All @@ -163,7 +163,7 @@ struct MarshalVisitor {
case Un::Op::SWAP32:
case Un::Op::SWAP64:
return {ebpf_inst{
.opcode = static_cast<uint8_t>(INST_CLS_ALU64 | (0xd << 4)),
.opcode = static_cast<uint8_t>(INST_CLS_ALU64 | INST_ALU_OP_END),
.dst = b.dst.v,
.src = 0,
.offset = 0,
Expand Down
88 changes: 66 additions & 22 deletions src/asm_unmarshal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,17 @@ void compare(const string& field, T actual, T expected) {
std::cerr << field << ": (actual) " << std::hex << (int)actual << " != " << (int)expected << " (expected)\n";
}

static std::string make_opcode_message(const char* msg, uint8_t opcode) {
std::ostringstream oss;
oss << msg << " op 0x" << std::hex << (int)opcode;
return oss.str();
}

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} {}
};

struct UnsupportedMemoryMode : std::invalid_argument {
Expand Down Expand Up @@ -113,7 +121,7 @@ struct Unmarshaller {

// All the rest require a zero offset.
if (inst.offset != 0)
throw InvalidInstruction{pc, "nonzero offset for register alu op"};
throw InvalidInstruction(pc, make_opcode_message("nonzero offset for", inst.opcode));

switch (inst.opcode & INST_ALU_OP_MASK) {
case INST_ALU_OP_ADD: return Bin::Op::ADD;
Expand All @@ -123,16 +131,26 @@ struct Unmarshaller {
case INST_ALU_OP_AND: return Bin::Op::AND;
case INST_ALU_OP_LSH: return Bin::Op::LSH;
case INST_ALU_OP_RSH: return Bin::Op::RSH;
case INST_ALU_OP_NEG: return Un::Op::NEG;
case INST_ALU_OP_NEG:
// Negation is a unary operation. The SRC bit, src, and imm must be all 0.
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)};
if (inst.imm != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));
return Un::Op::NEG;
case INST_ALU_OP_XOR: return Bin::Op::XOR;
case INST_ALU_OP_ARSH:
if ((inst.opcode & INST_CLS_MASK) == INST_CLS_ALU)
note("arsh32 is not allowed");
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)};
if ((inst.opcode & INST_CLS_MASK) == INST_CLS_ALU64) {
if (inst.opcode & INST_END_BE)
throw InvalidInstruction(pc, "invalid endian immediate");
throw InvalidInstruction(pc, inst.opcode);
switch (inst.imm) {
case 16: return Un::Op::SWAP16;
case 32: return Un::Op::SWAP32;
Expand All @@ -147,8 +165,8 @@ struct Unmarshaller {
default:
throw InvalidInstruction(pc, "invalid endian immediate");
}
case 0xe0: throw InvalidInstruction{pc, "invalid ALU op 0xe0"};
case 0xf0: throw InvalidInstruction{pc, "invalid ALU op 0xf0"};
case 0xe0: throw InvalidInstruction{pc, inst.opcode};
case 0xf0: throw InvalidInstruction{pc, inst.opcode};
}
return {};
}
Expand All @@ -160,11 +178,11 @@ struct Unmarshaller {
auto getBinValue(pc_t pc, ebpf_inst inst) -> Value {
if (inst.opcode & INST_SRC_REG) {
if (inst.imm != 0)
throw InvalidInstruction{pc, "nonzero imm for register alu op"};
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));
return Reg{inst.src};
} else {
if (inst.src != 0)
throw InvalidInstruction{pc, "nonzero src for register alu op"};
throw InvalidInstruction{pc, make_opcode_message("nonzero src for register", inst.opcode)};
// Imm is a signed 32-bit number. Sign extend it to 64-bits for storage.
return Imm{sign_extend(inst.imm)};
}
Expand All @@ -187,7 +205,8 @@ struct Unmarshaller {
case 0xb: return Op::LE;
case 0xc: return Op::SLT;
case 0xd: return Op::SLE;
case 0xe: throw InvalidInstruction(pc, "invalid JMP op 0xe");
case 0xe: throw InvalidInstruction(pc, opcode);
case 0xf: throw InvalidInstruction(pc, opcode);
}
return {};
}
Expand All @@ -199,8 +218,7 @@ struct Unmarshaller {
int width = getMemWidth(inst.opcode);
bool isLD = (inst.opcode & INST_CLS_MASK) == INST_CLS_LD;
switch ((inst.opcode & INST_MODE_MASK) >> 5) {
case 0:
throw InvalidInstruction(pc, "Bad instruction");
case 0: throw InvalidInstruction(pc, inst.opcode);
case INST_ABS:
if (!isLD)
throw InvalidInstruction(pc, "ABS but not LD");
Expand All @@ -217,11 +235,15 @@ struct Unmarshaller {

case INST_MEM: {
if (isLD)
throw InvalidInstruction(pc, "plain LD");
throw InvalidInstruction(pc, inst.opcode);
bool isLoad = getMemIsLoad(inst.opcode);
if (isLoad && inst.dst == R10_STACK_POINTER)
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));
if (!isImm && inst.imm != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));

assert(!(isLoad && isImm));
uint8_t basereg = isLoad ? inst.src : inst.dst;
Expand All @@ -247,7 +269,7 @@ struct Unmarshaller {
if (((inst.opcode & INST_CLS_MASK) != INST_CLS_STX) ||
((inst.opcode & INST_SIZE_MASK) != INST_SIZE_W &&
(inst.opcode & INST_SIZE_MASK) != INST_SIZE_DW))
throw InvalidInstruction(pc, "Bad instruction");
throw InvalidInstruction(pc, inst.opcode);
if (inst.imm != 0)
throw InvalidInstruction(pc, "Unsupported atomic instruction");
return LockAdd{
Expand All @@ -259,8 +281,7 @@ struct Unmarshaller {
},
.valreg = Reg{inst.src},
};
default:
throw InvalidInstruction(pc, "Bad instruction");
default: throw InvalidInstruction(pc, inst.opcode);
}
return {};
}
Expand Down Expand Up @@ -382,21 +403,44 @@ struct Unmarshaller {

auto makeJmp(ebpf_inst inst, const vector<ebpf_inst>& insts, pc_t pc) -> Instruction {
switch ((inst.opcode >> 4) & 0xF) {
case 0x8:
case INST_CALL:
if ((inst.opcode & INST_CLS_MASK) != INST_CLS_JMP)
throw InvalidInstruction(pc, inst.opcode);
if (inst.opcode & INST_SRC_REG)
throw InvalidInstruction(pc, "Bad instruction");
throw InvalidInstruction(pc, inst.opcode);
if (inst.offset != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero offset for", inst.opcode));
if (!info.platform->is_helper_usable(inst.imm))
throw InvalidInstruction(pc, "invalid helper function id");
return makeCall(inst.imm);
case 0x9:
if ((inst.opcode & INST_CLS_MASK) != INST_CLS_JMP)
throw InvalidInstruction(pc, "Bad instruction");
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));
if (inst.imm != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));
if (inst.offset != 0)
throw InvalidInstruction(pc, make_opcode_message("nonzero offset for", inst.opcode));
return Exit{};
case 0x0:
case INST_JA:
if ((inst.opcode & INST_CLS_MASK) != INST_CLS_JMP &&
(inst.opcode & INST_CLS_MASK) != INST_CLS_JMP32)
throw InvalidInstruction(pc, "Bad instruction");
throw InvalidInstruction(pc, inst.opcode);
if (inst.opcode & INST_SRC_REG)
throw InvalidInstruction(pc, inst.opcode);
if ((inst.opcode & INST_CLS_MASK) == INST_CLS_JMP && (inst.imm != 0))
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));
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));
if ((inst.opcode & INST_SRC_REG) && (inst.imm != 0))
throw InvalidInstruction(pc, make_opcode_message("nonzero imm for", inst.opcode));

int32_t offset = (inst.opcode == INST_OP_JA32) ? inst.imm : inst.offset;
pc_t new_pc = pc + 1 + offset;
if (new_pc >= insts.size())
Expand All @@ -407,7 +451,7 @@ struct Unmarshaller {
auto cond = (inst.opcode == INST_OP_JA16 || inst.opcode == INST_OP_JA32)
? std::optional<Condition>{}
: Condition{
.op = getJmpOp(pc, inst.opcode),
.op = op,
.left = Reg{inst.dst},
.right = (inst.opcode & INST_SRC_REG) ? (Value)Reg{inst.src}
: Imm{sign_extend(inst.imm)},
Expand Down
12 changes: 8 additions & 4 deletions src/ebpf_vm_isa.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,14 @@ enum {

INST_OP_LDDW_IMM = (INST_CLS_LD | INST_SRC_IMM | INST_SIZE_DW), // Special

INST_OP_JA32 = (INST_CLS_JMP32 | 0x00),
INST_OP_JA16 = (INST_CLS_JMP | 0x00),
INST_OP_CALL = (INST_CLS_JMP | 0x80),
INST_OP_EXIT = (INST_CLS_JMP | 0x90),
INST_JA = 0x0,
INST_CALL = 0x8,
INST_EXIT = 0x9,

INST_OP_JA32 = ((INST_JA << 4) | INST_CLS_JMP32),
INST_OP_JA16 = ((INST_JA << 4) | INST_CLS_JMP),
INST_OP_CALL = ((INST_CALL << 4) | INST_CLS_JMP),
INST_OP_EXIT = ((INST_EXIT << 4) | INST_CLS_JMP),

INST_ALU_OP_ADD = 0x00,
INST_ALU_OP_SUB = 0x10,
Expand Down
Loading

0 comments on commit 47d78f4

Please sign in to comment.