From 3b77e0fa58722d619fa59e4c9b07e45916e8f6cb Mon Sep 17 00:00:00 2001 From: Will Hawkins Date: Fri, 22 Nov 2024 20:07:40 -0500 Subject: [PATCH] Validate specific atomic operation opcodes The eBPF RFC specifies an enumerated list of valid "sub" opcodes for atomic operations. These subopcodes are stored in the imm field of the instruction. This patch adds support for validating that an atomic operation contains exactly one of the enumerated values in the RFC. Signed-off-by: Will Hawkins --- .../data/ubpf_test_atomic_validate.input | 1 + .../descrs/ubpf_test_atomic_validate.md | 3 + .../srcs/ubpf_test_atomic_validate.cc | 51 ++++++++++++++ vm/ubpf_instruction_valid.c | 66 ++++++++++++++----- 4 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 custom_tests/data/ubpf_test_atomic_validate.input create mode 100644 custom_tests/descrs/ubpf_test_atomic_validate.md create mode 100644 custom_tests/srcs/ubpf_test_atomic_validate.cc diff --git a/custom_tests/data/ubpf_test_atomic_validate.input b/custom_tests/data/ubpf_test_atomic_validate.input new file mode 100644 index 000000000..8c23fd667 --- /dev/null +++ b/custom_tests/data/ubpf_test_atomic_validate.input @@ -0,0 +1 @@ +b4 00 00 00 00 00 00 00 db 01 00 00 42 00 00 00 95 00 00 00 00 00 00 00 diff --git a/custom_tests/descrs/ubpf_test_atomic_validate.md b/custom_tests/descrs/ubpf_test_atomic_validate.md new file mode 100644 index 000000000..27ecb3834 --- /dev/null +++ b/custom_tests/descrs/ubpf_test_atomic_validate.md @@ -0,0 +1,3 @@ +## Test Description + +This test verifies that the check for to validate an instruction properly handles the case where an atomic operation's immediate field does not contain a valid operation (according to Table 11 in the [spec](https://www.ietf.org/archive/id/draft-thaler-bpf-isa-00.html). \ No newline at end of file diff --git a/custom_tests/srcs/ubpf_test_atomic_validate.cc b/custom_tests/srcs/ubpf_test_atomic_validate.cc new file mode 100644 index 000000000..ab9bd5a19 --- /dev/null +++ b/custom_tests/srcs/ubpf_test_atomic_validate.cc @@ -0,0 +1,51 @@ +// Copyright (c) Will Hawkins +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include +#include + +extern "C" +{ +#include "ubpf.h" +} + +#include "ubpf_custom_test_support.h" + +int +main(int argc, char** argv) +{ + std::string program_string{}; + std::string error{}; + ubpf_jit_fn jit_fn; + + if (!get_program_string(argc, argv, program_string, error)) { + std::cerr << error << std::endl; + return 1; + } + + uint64_t memory_expected{0x123456789}; + uint64_t memory{0x123456789}; + + std::unique_ptr vm(ubpf_create(), ubpf_destroy); + if (!ubpf_setup_custom_test( + vm, program_string, [](ubpf_vm_up&, std::string&) { return true; }, jit_fn, error)) { + if (error == "Failed to load program: Invalid immediate value 66 for opcode DB.") { + return 0; + } + + return 1; + } + + return 1; + + uint64_t bpf_return_value; + if (ubpf_exec(vm.get(), &memory, sizeof(memory), &bpf_return_value)) { + std::cerr << "Problem executing program" << std::endl; + return 1; + } + + return !(memory == memory_expected); +} diff --git a/vm/ubpf_instruction_valid.c b/vm/ubpf_instruction_valid.c index c99480caa..07d2804bb 100644 --- a/vm/ubpf_instruction_valid.c +++ b/vm/ubpf_instruction_valid.c @@ -9,18 +9,32 @@ * @brief Structure to filter valid fields for each eBPF instruction. * Default values are all zeros, which means the field is reserved and must be zero. */ -typedef struct _ubpf_inst_filter { - uint8_t opcode; ///< The opcode of the instruction. - uint8_t source_lower_bound; ///< The lower bound of the source register. - uint8_t source_upper_bound; ///< The upper bound of the source register. - uint8_t destination_lower_bound; ///< The lower bound of the destination register. - uint8_t destination_upper_bound; ///< The upper bound of the destination register. - int16_t offset_lower_bound; ///< The lower bound of the offset. - int16_t offset_upper_bound; ///< The upper bound of the offset. - int32_t immediate_lower_bound; ///< The lower bound of the immediate value. - int32_t immediate_upper_bound; ///< The upper bound of the immediate value. +typedef struct _ubpf_inst_filter +{ + uint8_t opcode; ///< The opcode of the instruction. + uint8_t source_lower_bound; ///< The lower bound of the source register. + uint8_t source_upper_bound; ///< The upper bound of the source register. + uint8_t destination_lower_bound; ///< The lower bound of the destination register. + uint8_t destination_upper_bound; ///< The upper bound of the destination register. + int16_t offset_lower_bound; ///< The lower bound of the offset. + int16_t offset_upper_bound; ///< The upper bound of the offset. + int32_t immediate_lower_bound; ///< The lower bound of the immediate value. + int32_t immediate_upper_bound; ///< The upper bound of the immediate value. + int32_t* immediate_enumerated; ///< A specific enumeration of the valid immediate values. + uint32_t immediate_enumerated_length; ///< The number of valid enumerated immediate values. } ubpf_inst_filter_t; +static int32_t ebpf_atomic_store_immediate_enumerated[] = { + EBPF_ALU_OP_ADD, + EBPF_ALU_OP_ADD | EBPF_ATOMIC_OP_FETCH, + EBPF_ALU_OP_OR, + EBPF_ALU_OP_OR | EBPF_ATOMIC_OP_FETCH, + EBPF_ALU_OP_AND, + EBPF_ALU_OP_AND | EBPF_ATOMIC_OP_FETCH, + EBPF_ALU_OP_XOR, + EBPF_ALU_OP_XOR | EBPF_ATOMIC_OP_FETCH, + EBPF_ATOMIC_OP_XCHG | EBPF_ATOMIC_OP_FETCH, + EBPF_ATOMIC_OP_CMPXCHG | EBPF_ATOMIC_OP_FETCH}; /** * @brief Array of valid eBPF instructions and their fields. @@ -208,6 +222,7 @@ static ubpf_inst_filter_t _ubpf_instruction_filter[] = { .opcode = EBPF_OP_LE, .destination_lower_bound = BPF_REG_0, .destination_upper_bound = BPF_REG_9, + // specific valid values for the immediate field are checked in validate. .immediate_lower_bound = 0, .immediate_upper_bound = 64, }, @@ -215,6 +230,7 @@ static ubpf_inst_filter_t _ubpf_instruction_filter[] = { .opcode = EBPF_OP_BE, .destination_lower_bound = BPF_REG_0, .destination_upper_bound = BPF_REG_9, + // specific valid values for the immediate field are checked in validate. .immediate_lower_bound = 0, .immediate_upper_bound = 64, }, @@ -503,6 +519,9 @@ static ubpf_inst_filter_t _ubpf_instruction_filter[] = { .opcode = EBPF_OP_LDDW, .destination_lower_bound = BPF_REG_0, .destination_upper_bound = BPF_REG_10, + // specific valid source values are checked in validate. + .source_lower_bound = 0, + .source_upper_bound = 6, .immediate_lower_bound = INT32_MIN, .immediate_upper_bound = INT32_MAX, }, @@ -934,8 +953,8 @@ static ubpf_inst_filter_t _ubpf_instruction_filter[] = { .destination_upper_bound = BPF_REG_10, .source_lower_bound = BPF_REG_0, .source_upper_bound = BPF_REG_10, - .immediate_lower_bound = 0x0, - .immediate_upper_bound = 0xff, + .immediate_enumerated = ebpf_atomic_store_immediate_enumerated, + .immediate_enumerated_length = 10, .offset_lower_bound = INT16_MIN, .offset_upper_bound = INT16_MAX, }, @@ -994,10 +1013,25 @@ ubpf_is_valid_instruction(const struct ebpf_inst insts, char ** errmsg) return false; } - // Validate immediate value. - if (!_in_range(insts.imm, filter->immediate_lower_bound, filter->immediate_upper_bound)) { - *errmsg = ubpf_error("Invalid immediate value %d for opcode %2X.", insts.imm, insts.opcode); - return false; + // Validate immediate values in the presence of enumerated values. + if (filter->immediate_enumerated != NULL) { + bool valid = false; + for (int i = 0; i < filter->immediate_enumerated_length; i++) { + if (filter->immediate_enumerated[i] == insts.imm) { + valid = true; + break; + } + } + if (!valid) { + *errmsg = ubpf_error("Invalid immediate value %d for opcode %2X.", insts.imm, insts.opcode); + return false; + } + } else { + // Validate immediate value. + if (!_in_range(insts.imm, filter->immediate_lower_bound, filter->immediate_upper_bound)) { + *errmsg = ubpf_error("Invalid immediate value %d for opcode %2X.", insts.imm, insts.opcode); + return false; + } } // Validate offset value.