From 9f97deba38fbe429c22e4ebf2d64f8ee043811d6 Mon Sep 17 00:00:00 2001 From: Alan Jowett Date: Tue, 3 May 2022 16:35:50 -0600 Subject: [PATCH] Resolve warnings in bpf2c generated code (#1058) * Resolve warnings in bpf2c generated code Signed-off-by: Alan Jowett * Add negative test cases for raw BPF programs Signed-off-by: Alan Jowett * Add negative test cases for raw BPF programs Signed-off-by: Alan Jowett Co-authored-by: Alan Jowett --- .../expected/divide_by_zero_dll.txt | 2 +- .../expected/divide_by_zero_raw.txt | 2 +- .../expected/divide_by_zero_sys.txt | 2 +- tests/bpf2c_tests/raw_bpf.cpp | 93 ++++++++++++++++++- tools/bpf2c/bpf_code_generator.cpp | 34 +++++-- 5 files changed, 118 insertions(+), 15 deletions(-) diff --git a/tests/bpf2c_tests/expected/divide_by_zero_dll.txt b/tests/bpf2c_tests/expected/divide_by_zero_dll.txt index 13aa81c43..85b9fdae0 100644 --- a/tests/bpf2c_tests/expected/divide_by_zero_dll.txt +++ b/tests/bpf2c_tests/expected/divide_by_zero_dll.txt @@ -134,7 +134,7 @@ static uint64_t divide_by_zero(void* context) r6 = IMMEDIATE(100000); // EBPF_OP_DIV64_REG pc=10 dst=r6 src=r1 offset=0 imm=0 #line 32 "sample/divide_by_zero.c" - if (r1 == 0) { division_by_zero(10); return -1; } + if (r1 == 0) { division_by_zero(10); return 0xffffffffffffffffui64; } #line 32 "sample/divide_by_zero.c" r6 /= r1; label_1: diff --git a/tests/bpf2c_tests/expected/divide_by_zero_raw.txt b/tests/bpf2c_tests/expected/divide_by_zero_raw.txt index c35cc030c..64158edd2 100644 --- a/tests/bpf2c_tests/expected/divide_by_zero_raw.txt +++ b/tests/bpf2c_tests/expected/divide_by_zero_raw.txt @@ -92,7 +92,7 @@ static uint64_t divide_by_zero(void* context) r6 = IMMEDIATE(100000); // EBPF_OP_DIV64_REG pc=10 dst=r6 src=r1 offset=0 imm=0 #line 32 "sample/divide_by_zero.c" - if (r1 == 0) { division_by_zero(10); return -1; } + if (r1 == 0) { division_by_zero(10); return 0xffffffffffffffffui64; } #line 32 "sample/divide_by_zero.c" r6 /= r1; label_1: diff --git a/tests/bpf2c_tests/expected/divide_by_zero_sys.txt b/tests/bpf2c_tests/expected/divide_by_zero_sys.txt index b82698a94..839a5d112 100644 --- a/tests/bpf2c_tests/expected/divide_by_zero_sys.txt +++ b/tests/bpf2c_tests/expected/divide_by_zero_sys.txt @@ -259,7 +259,7 @@ static uint64_t divide_by_zero(void* context) r6 = IMMEDIATE(100000); // EBPF_OP_DIV64_REG pc=10 dst=r6 src=r1 offset=0 imm=0 #line 32 "sample/divide_by_zero.c" - if (r1 == 0) { division_by_zero(10); return -1; } + if (r1 == 0) { division_by_zero(10); return 0xffffffffffffffffui64; } #line 32 "sample/divide_by_zero.c" r6 /= r1; label_1: diff --git a/tests/bpf2c_tests/raw_bpf.cpp b/tests/bpf2c_tests/raw_bpf.cpp index 49904b85a..02b4e4d8e 100644 --- a/tests/bpf2c_tests/raw_bpf.cpp +++ b/tests/bpf2c_tests/raw_bpf.cpp @@ -102,12 +102,12 @@ run_test(const std::string& data_file) result = result.substr(result.find("0x") + 2); } data_out.seekg(0); - auto intstructions = bpf_assembler(data_out); + auto instructions = bpf_assembler(data_out); std::ofstream c_file(std::string(prefix) + std::string(".c")); try { - bpf_code_generator code("test", intstructions); + bpf_code_generator code("test", instructions); code.generate("test"); code.emit_c_code(c_file); } catch (std::runtime_error& err) { @@ -277,3 +277,92 @@ DECLARE_TEST("stxw") DECLARE_TEST("subnet") // Test doesn't support unload directive. // DECLARE_TEST("unload_reload") + +void +verify_invalid_opcode_sequence(const std::vector& instructions, const std::string& error) +{ + bpf_code_generator code("test", instructions); + try { + code.generate("test"); + FAIL("bpf_code_generator permitted invalid sequence"); + } catch (const std::runtime_error& ex) { + REQUIRE(ex.what() == error); + } +} + +TEST_CASE("BE/LE", "[raw_bpf_code_gen][negative]") +{ + // EBPF_OP_LE/EBPF_OP_BE only supports imm == {16,32,64} + verify_invalid_opcode_sequence({{EBPF_OP_LE, 0, 0, 0, 15}}, "invalid operand"); + verify_invalid_opcode_sequence({{EBPF_OP_BE, 0, 0, 0, 15}}, "invalid operand"); +} + +TEST_CASE("div/mod by imm 0", "[raw_bpf_code_gen][negative]") +{ + // Division by immediate value of 0 is invalid. + verify_invalid_opcode_sequence({{EBPF_OP_DIV_IMM, 0, 0, 0, 0}}, "invalid instruction - constant division by zero"); + verify_invalid_opcode_sequence({{EBPF_OP_MOD_IMM, 0, 0, 0, 0}}, "invalid instruction - constant division by zero"); +} + +TEST_CASE("unknown op-class", "[raw_bpf_code_gen][negative]") +{ + // EBPF_CLS_JMP+1 isn't a valid op class + verify_invalid_opcode_sequence({{EBPF_CLS_JMP + 1, 0, 0, 0, 0}}, "invalid operand"); +} + +TEST_CASE("unknown EBPF_CLS_ALU operation", "[raw_bpf_code_gen][negative]") +{ + // EBPF_CLS_ALU + operations 0xe0 doesn't exist + verify_invalid_opcode_sequence({{(EBPF_CLS_ALU | EBPF_SRC_IMM | 0xe0), 0, 0, 0, 0}}, "invalid operand"); +} + +TEST_CASE("unknown EBPF_CLS_ALU64 operation", "[raw_bpf_code_gen][negative]") +{ + // EBPF_CLS_ALU64 + operations 0xe0 doesn't exist + verify_invalid_opcode_sequence({{(EBPF_CLS_ALU64 | EBPF_SRC_IMM | 0xe0), 0, 0, 0, 0}}, "invalid operand"); +} + +TEST_CASE("unknown EBPF_CLS_LD operation", "[raw_bpf_code_gen][negative]") +{ + // EBPF_CLS_LD is only valid with immediate and size _DW + verify_invalid_opcode_sequence({{(EBPF_CLS_LD | EBPF_MODE_MEM | EBPF_SIZE_DW), 0, 0, 0, 0}}, "invalid operand"); + verify_invalid_opcode_sequence({{(EBPF_CLS_LD | EBPF_MODE_IMM | EBPF_SIZE_W), 0, 0, 0, 0}}, "invalid operand"); +} + +TEST_CASE("malformed EBPF_CLS_LD operation", "[raw_bpf_code_gen][negative]") +{ + // EBPF_CLS_LD is always 2 instructions wide + verify_invalid_opcode_sequence({{(EBPF_CLS_LD | EBPF_MODE_IMM | EBPF_SIZE_DW), 0, 0, 0, 0}}, "invalid operand"); +} + +TEST_CASE("EBPF_CLS_JMP invalid target", "[raw_bpf_code_gen][negative]") +{ + // Offset > end of program + verify_invalid_opcode_sequence({{EBPF_OP_JA, 0, 0, 1, 0}}, "invalid jump target"); +} + +TEST_CASE("EBPF_CLS_JMP invalid operation", "[raw_bpf_code_gen][negative]") +{ + // 0xf0 is an invalid jump operation + verify_invalid_opcode_sequence( + {{(EBPF_CLS_JMP | 0xf0), 0, 0, 0, 0}, {EBPF_OP_EXIT, 0, 0, 0, 0}}, "invalid operand"); +} + +TEST_CASE("invalid register", "[raw_bpf_code_gen][negative]") +{ + // 14 and 15 aren't valid registers. + verify_invalid_opcode_sequence({{EBPF_OP_DIV_REG, 15, 14, 0, 0}}, "invalid register id"); +} + +TEST_CASE("invalid ELF stream", "[raw_bpf_code_gen][negative]") +{ + // An empty stream is not valid + std::string str; + std::stringstream stream(str); + try { + bpf_code_generator code(stream, "test"); + FAIL("bpf_code_generator failed to detect error"); + } catch (const std::runtime_error& ex) { + REQUIRE(ex.what() == std::string("can't process ELF file test")); + } +} diff --git a/tools/bpf2c/bpf_code_generator.cpp b/tools/bpf2c/bpf_code_generator.cpp index 1683b0561..0b1f0015c 100644 --- a/tools/bpf2c/bpf_code_generator.cpp +++ b/tools/bpf2c/bpf_code_generator.cpp @@ -112,7 +112,7 @@ std::string bpf_code_generator::get_register_name(uint8_t id) { if (id >= _countof(_register_names)) { - throw std::runtime_error("Invalid register id"); + throw std::runtime_error("invalid register id"); } else { current_section->referenced_registers.insert(_register_names[id]); return _register_names[id]; @@ -124,7 +124,7 @@ bpf_code_generator::bpf_code_generator( : current_section(nullptr), c_name(c_name), path(path), elf_file_hash(elf_file_hash) { if (!reader.load(stream)) { - throw std::runtime_error(std::string("Can't process ELF file ") + c_name); + throw std::runtime_error(std::string("can't process ELF file ") + c_name); } extract_btf_information(); @@ -363,6 +363,9 @@ bpf_code_generator::generate_labels() if (output.instruction.opcode == EBPF_OP_EXIT) { continue; } + if ((i + output.instruction.offset + 1) >= program_output.size()) { + throw std::runtime_error("invalid jump target"); + } program_output[i + output.instruction.offset + 1].jump_target = true; } @@ -424,8 +427,8 @@ bpf_code_generator::encode_instructions(const std::string& section_name) source = std::string("IMMEDIATE(") + std::to_string(inst.imm) + std::string(")"); bool is64bit = (inst.opcode & EBPF_CLS_MASK) == EBPF_CLS_ALU64; AluOperations operation = static_cast(inst.opcode >> 4); - std::string check_div_by_zero = - format_string("if (%s == 0) { division_by_zero(%s); return -1; }", source, std::to_string(i)); + std::string check_div_by_zero = format_string( + "if (%s == 0) { division_by_zero(%s); return 0xffffffffffffffffui64; }", source, std::to_string(i)); std::string swap_function; switch (operation) { case AluOperations::Add: @@ -438,7 +441,11 @@ bpf_code_generator::encode_instructions(const std::string& section_name) output.lines.push_back(format_string("%s *= %s;", destination, source)); break; case AluOperations::Div: - output.lines.push_back(check_div_by_zero); + if (inst.opcode & EBPF_SRC_REG) { + output.lines.push_back(check_div_by_zero); + } else if (inst.imm == 0) { + throw std::runtime_error("invalid instruction - constant division by zero"); + } if (is64bit) output.lines.push_back(format_string("%s /= %s;", destination, source)); else @@ -461,13 +468,14 @@ bpf_code_generator::encode_instructions(const std::string& section_name) output.lines.push_back(format_string("%s = (uint32_t)%s >> %s;", destination, destination, source)); break; case AluOperations::Neg: - if (is64bit) - output.lines.push_back(format_string("%s = -%s;", destination, destination)); - else - output.lines.push_back(format_string("%s = -(int64_t)%s;", destination, destination)); + output.lines.push_back(format_string("%s = -(int64_t)%s;", destination, destination)); break; case AluOperations::Mod: - output.lines.push_back(check_div_by_zero); + if (inst.opcode & EBPF_SRC_REG) { + output.lines.push_back(check_div_by_zero); + } else if (inst.imm == 0) { + throw std::runtime_error("invalid instruction - constant division by zero"); + } if (is64bit) output.lines.push_back(format_string("%s %%= %s;", destination, source)); else @@ -541,6 +549,9 @@ bpf_code_generator::encode_instructions(const std::string& section_name) if (inst.opcode != EBPF_OP_LDDW) { throw std::runtime_error("invalid operand"); } + if (i >= program_output.size()) { + throw std::runtime_error("invalid operand"); + } std::string destination = get_register_name(inst.dst); if (output.relocation.empty()) { uint64_t imm = static_cast(program_output[i].instruction.imm); @@ -619,6 +630,9 @@ bpf_code_generator::encode_instructions(const std::string& section_name) } else { source = std::string("IMMEDIATE(") + std::to_string(inst.imm) + std::string(")"); } + if ((inst.opcode >> 4) >= _countof(_predicate_format_string)) { + throw std::runtime_error("invalid operand"); + } auto& format = _predicate_format_string[inst.opcode >> 4]; if (inst.opcode == EBPF_OP_JA) { std::string target = program_output[i + inst.offset + 1].label;