Deprecate legacy packet access instructions

src/asm_syntax.hpp

@@ -202,7 +202,7 @@ struct Mem {
     bool is_load{};
 };
 
-/// A special instruction for checked access to packets; it is actually a
+/// A deprecated instruction for checked access to packets; it is actually a
 /// function call, and analyzed as one, e.g., by scratching caller-saved
 /// registers after it is performed.
 struct Packet {

src/asm_unmarshal.cpp

@@ -222,17 +222,13 @@ struct Unmarshaller {
         switch ((inst.opcode & INST_MODE_MASK) >> 5) {
         case 0: throw InvalidInstruction(pc, inst.opcode);
         case INST_ABS:
-            if (!isLD)
-                throw InvalidInstruction(pc, "ABS but not LD");
-            if (width == 8)
-                note("invalid opcode LDABSDW");
+            if (!info.platform->legacy || !isLD || (width == 8))
+                throw InvalidInstruction(pc, inst.opcode);
             return Packet{.width = width, .offset = inst.imm, .regoffset = {}};
 
         case INST_IND:
-            if (!isLD)
-                throw InvalidInstruction(pc, "IND but not LD");
-            if (width == 8)
-                note("invalid opcode LDINDDW");
+            if (!info.platform->legacy || !isLD || (width == 8))
+                throw InvalidInstruction(pc, inst.opcode);
             return Packet{.width = width, .offset = inst.imm, .regoffset = Reg{inst.src}};
 
         case INST_MEM: {

src/asm_unmarshal.hpp

@@ -16,7 +16,7 @@
  *  of Instructions.
  *
  *  \param raw_prog is the input program to parse.
- *  \param notes is where errors and warnings are written to.
+ *  \param[out] notes is a vector for storing errors and warnings.
  *  \return a sequence of instruction if successful, an error string otherwise.
  */
 std::variant<InstructionSeq, std::string> unmarshal(const raw_program& raw_prog, std::vector<std::vector<std::string>>& notes);

src/ebpf_vm_isa.hpp

@@ -44,8 +44,8 @@ enum {
 
     INST_MODE_MASK = 0xe0,
 
-    INST_ABS = 1,
-    INST_IND = 2,
+    INST_ABS = 1, // Deprecated
+    INST_IND = 2, // Deprecated
     INST_MEM = 3,
     INST_LEN = 4,
     INST_MSH = 5,

src/ebpf_yaml.cpp

@@ -57,7 +57,8 @@ ebpf_platform_t g_platform_test = {
     .map_record_size = 0,
     .parse_maps_section = ebpf_parse_maps_section,
     .get_map_descriptor = ebpf_get_map_descriptor,
-    .get_map_type = ebpf_get_map_type
+    .get_map_type = ebpf_get_map_type,
+    .legacy = true,
 };
 
 static EbpfProgramType make_program_type(const string& name, ebpf_context_descriptor_t* context_descriptor) {

src/linux/linux_platform.cpp

@@ -254,5 +254,6 @@ const ebpf_platform_t g_ebpf_platform_linux = {
     parse_maps_section_linux,
     get_map_descriptor_linux,
     get_map_type_linux,
-    resolve_inner_map_references_linux
+    resolve_inner_map_references_linux,
+    true // Legacy packet access instructions
 };

src/main/check.cpp

@@ -66,6 +66,8 @@ int main(int argc, char** argv) {
     app.add_flag("-f", ebpf_verifier_options.print_failures, "Print verifier's failure logs");
     app.add_flag("-s", ebpf_verifier_options.strict, "Apply additional checks that would cause runtime failures");
     app.add_flag("-v", verbose, "Print both invariants and failures");
+    bool legacy = false;
+    app.add_flag("--legacy", legacy, "Allow deprecated packet access instructions");
     bool no_division_by_zero = false;
     app.add_flag("--no-division-by-zero", no_division_by_zero, "Do not allow division by zero");
     app.add_flag("--no-simplify", ebpf_verifier_options.no_simplify, "Do not simplify");
@@ -111,12 +113,13 @@ int main(int argc, char** argv) {
 
     if (domain == "linux")
         ebpf_verifier_options.mock_map_fds = false;
-    const ebpf_platform_t* platform = &g_ebpf_platform_linux;
+    ebpf_platform_t platform = g_ebpf_platform_linux;
+    platform.legacy = legacy;
 
     // Read a set of raw program sections from an ELF file.
     vector<raw_program> raw_progs;
     try {
-        raw_progs = read_elf(filename, desired_section, &ebpf_verifier_options, platform);
+        raw_progs = read_elf(filename, desired_section, &ebpf_verifier_options, &platform);
     } catch (std::runtime_error& e) {
         std::cerr << "error: " << e.what() << std::endl;
         return 1;
@@ -130,7 +133,7 @@ int main(int argc, char** argv) {
         if (!desired_section.empty() && raw_progs.empty()) {
             // We could not find the desired section, so get the full list
             // of possibilities.
-            raw_progs = read_elf(filename, string(), &ebpf_verifier_options, platform);
+            raw_progs = read_elf(filename, string(), &ebpf_verifier_options, &platform);
         }
         for (const raw_program& raw_prog : raw_progs) {
             std::cout << raw_prog.section << " ";

src/platform.hpp

@@ -42,6 +42,9 @@ struct ebpf_platform_t {
     ebpf_get_map_descriptor_fn get_map_descriptor;
     ebpf_get_map_type_fn get_map_type;
     ebpf_resolve_inner_map_references_fn resolve_inner_map_references;
+
+    // Option indicating support for various deprecated instructions.
+    bool legacy;
 };
 
 extern const ebpf_platform_t g_ebpf_platform_linux;

src/test/test_marshal.cpp

@@ -8,9 +8,9 @@
 
 // Verify that if we unmarshal an instruction and then re-marshal it,
 // we get what we expect.
-static void compare_unmarshal_marshal(const ebpf_inst& ins, const ebpf_inst& expected_result) {
-    program_info info{.platform = &g_ebpf_platform_linux,
-                      .type = g_ebpf_platform_linux.get_program_type("unspec", "unspec")};
+static void compare_unmarshal_marshal(const ebpf_inst& ins, const ebpf_inst& expected_result, const ebpf_platform_t* platform = &g_ebpf_platform_linux) {
+    program_info info{.platform = platform,
+                      .type = platform->get_program_type("unspec", "unspec")};
     const ebpf_inst exit{.opcode = INST_OP_EXIT};
     InstructionSeq parsed = std::get<InstructionSeq>(unmarshal(raw_program{"", "", {ins, exit, exit}, info}));
     REQUIRE(parsed.size() == 3);
@@ -45,9 +45,9 @@ static void compare_unmarshal_marshal(const ebpf_inst& ins1, const ebpf_inst& in
 
 // 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) {
-    program_info info{.platform = &g_ebpf_platform_linux,
-                      .type = g_ebpf_platform_linux.get_program_type("unspec", "unspec")};
+static void compare_marshal_unmarshal(const Instruction& ins, bool double_cmd = false, const ebpf_platform_t* platform = &g_ebpf_platform_linux) {
+    program_info info{.platform = platform,
+                      .type = platform->get_program_type("unspec", "unspec")};
     InstructionSeq parsed = std::get<InstructionSeq>(unmarshal(raw_program{"", "", marshal(ins, 0), info}));
     REQUIRE(parsed.size() == 1);
     auto [_, single, _2] = parsed.back();
@@ -56,16 +56,16 @@ static void compare_marshal_unmarshal(const Instruction& ins, bool double_cmd =
     REQUIRE(single == ins);
 }
 
-static void check_marshal_unmarshal_fail(const Instruction& ins, std::string expected_error_message) {
-    program_info info{.platform = &g_ebpf_platform_linux,
-                      .type = g_ebpf_platform_linux.get_program_type("unspec", "unspec")};
+static void check_marshal_unmarshal_fail(const Instruction& ins, std::string expected_error_message, const ebpf_platform_t* platform = &g_ebpf_platform_linux) {
+    program_info info{.platform = platform,
+                      .type = platform->get_program_type("unspec", "unspec")};
     std::string error_message = std::get<std::string>(unmarshal(raw_program{"", "", marshal(ins, 0), info}));
     REQUIRE(error_message == expected_error_message);
 }
 
-static void check_unmarshal_fail(ebpf_inst inst, std::string expected_error_message) {
-    program_info info{.platform = &g_ebpf_platform_linux,
-                      .type = g_ebpf_platform_linux.get_program_type("unspec", "unspec")};
+static void check_unmarshal_fail(ebpf_inst inst, std::string expected_error_message, const ebpf_platform_t* platform = &g_ebpf_platform_linux) {
+    program_info info{.platform = platform,
+                      .type = platform->get_program_type("unspec", "unspec")};
     std::vector<ebpf_inst> insns = {inst};
     auto result = unmarshal(raw_program{"", "", insns, info});
     REQUIRE(std::holds_alternative<std::string>(result));
@@ -177,8 +177,10 @@ TEST_CASE("disasm_marshal", "[disasm][marshal]") {
 
     SECTION("Packet") {
         for (int w : ws) {
-            compare_marshal_unmarshal(Packet{.width = w, .offset = 7, .regoffset = {}});
-            compare_marshal_unmarshal(Packet{.width = w, .offset = 7, .regoffset = Reg{2}});
+            if (w != 8) {
+                compare_marshal_unmarshal(Packet{.width = w, .offset = 7, .regoffset = {}});
+                compare_marshal_unmarshal(Packet{.width = w, .offset = 7, .regoffset = Reg{2}});
+            }
         }
     }
 
@@ -336,6 +338,39 @@ TEST_CASE("fail unmarshal offset opcodes", "[disasm][marshal]") {
     }
 }
 
+TEST_CASE("check unmarshal legacy opcodes", "[disasm][marshal]") {
+    // The following opcodes are deprecated and should no longer be used.
+    static uint8_t supported_legacy_opcodes[] = {0x20, 0x28, 0x30, 0x40, 0x48, 0x50};
+    static uint8_t unsupported_legacy_opcodes[] = {0x21, 0x22, 0x23, 0x29, 0x2a, 0x2b, 0x31, 0x32, 0x33,
+                                                   0x38, 0x39, 0x3a, 0x3b, 0x41, 0x42, 0x43, 0x49, 0x4a,
+                                                   0x4b, 0x51, 0x52, 0x53, 0x58, 0x59, 0x5a, 0x5b};
+
+    for (uint8_t opcode : unsupported_legacy_opcodes) {
+        std::ostringstream oss;
+        oss << "0: Bad instruction op 0x" << std::hex << (int)opcode << std::endl;
+        check_unmarshal_fail(ebpf_inst{.opcode = opcode}, oss.str().c_str());
+    }
+
+    for (uint8_t opcode : supported_legacy_opcodes) {
+        compare_unmarshal_marshal(ebpf_inst{.opcode = opcode}, ebpf_inst{.opcode = opcode});
+    }
+
+    // Disable legacy support.
+    ebpf_platform_t platform = g_ebpf_platform_linux;
+    platform.legacy = false;
+
+    for (uint8_t opcode : unsupported_legacy_opcodes) {
+        std::ostringstream oss;
+        oss << "0: Bad instruction op 0x" << std::hex << (int)opcode << std::endl;
+        check_unmarshal_fail(ebpf_inst{.opcode = opcode}, oss.str().c_str(), &platform);
+    }
+    for (uint8_t opcode : supported_legacy_opcodes) {
+        std::ostringstream oss;
+        oss << "0: Bad instruction op 0x" << std::hex << (int)opcode << std::endl;
+        check_unmarshal_fail(ebpf_inst{.opcode = opcode}, oss.str().c_str(), &platform);
+    }
+}
+
 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});
@@ -368,13 +403,10 @@ TEST_CASE("unmarshal 64bit immediate", "[disasm][marshal]") {
     }
 }
 
+
 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 = /* 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},
-                         "0: IND but not LD\n");
     check_unmarshal_fail(ebpf_inst{.opcode = /* 0x71 */ ((INST_MEM << 5) | INST_SIZE_B | INST_CLS_LDX), .dst = 11, .imm = 8},
                          "0: Bad register\n");
     check_unmarshal_fail(ebpf_inst{.opcode = /* 0x71 */ ((INST_MEM << 5) | INST_SIZE_B | INST_CLS_LDX), .dst = 1, .src = 11},