diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index 955b3b3fb..21a469b13 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -17,11 +17,11 @@ jobs: runs-on: ubuntu-latest steps: - name: Harden Runner - uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1 + uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2 with: egress-policy: audit - name: 'Checkout Repository' uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: 'Dependency Review' - uses: actions/dependency-review-action@4081bf99e2866ebe428fc0477b69eb4fcda7220a # v4.4.0 + uses: actions/dependency-review-action@3b139cfc5fae8b618d3eae3675e383bb1769c019 # v4.5.0 diff --git a/.github/workflows/fuzzing.yml b/.github/workflows/fuzzing.yml index af13aa801..385ccc75d 100644 --- a/.github/workflows/fuzzing.yml +++ b/.github/workflows/fuzzing.yml @@ -31,7 +31,7 @@ jobs: steps: - name: Harden Runner - uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1 + uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2 with: egress-policy: audit @@ -44,7 +44,7 @@ jobs: run: echo "VALUE=platform-${{ matrix.platform }}_arch=${{ matrix.arch }}_type=fuzzing" >> $GITHUB_OUTPUT - name: Update the cache (ccache) - uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2 + uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0 with: path: ccache key: ${{ steps.cache_key.outputs.VALUE }}_ccache @@ -135,7 +135,7 @@ jobs: steps: - name: Harden Runner - uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1 + uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2 with: egress-policy: audit @@ -144,7 +144,7 @@ jobs: submodules: 'recursive' - name: Cache the build folder - uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a + uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 env: cache-name: cache-nuget-modules with: diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1f7c90928..7617da335 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -368,7 +368,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Harden Runner - uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1 + uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2 with: egress-policy: audit diff --git a/.github/workflows/posix.yml b/.github/workflows/posix.yml index edc1453ab..aef34dba2 100644 --- a/.github/workflows/posix.yml +++ b/.github/workflows/posix.yml @@ -61,7 +61,7 @@ jobs: steps: - name: Harden Runner - uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1 + uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2 with: egress-policy: audit @@ -71,7 +71,7 @@ jobs: - name: Initialize CodeQL if: inputs.build_codeql == true - uses: github/codeql-action/init@ea9e4e37992a54ee68a9622e985e60c8e8f12d9f + uses: github/codeql-action/init@df409f7d9260372bd5f19e5b04e83cb3c43714ae with: languages: 'cpp' @@ -80,7 +80,7 @@ jobs: run: echo "VALUE=platform-${{ inputs.platform }}_arch=${{ inputs.arch }}_type-${{ inputs.build_type }}_sanitizers-${{ inputs.enable_sanitizers }}_coverage-${{ inputs.enable_coverage }}_scan_build-${{ inputs.scan_build }}_retpolines-${{ inputs.disable_retpolines }}" >> $GITHUB_OUTPUT - name: Update the cache (ccache) - uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2 + uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0 with: path: ccache key: ${{ steps.cache_key.outputs.VALUE }}_ccache @@ -306,4 +306,4 @@ jobs: - name: Perform CodeQL Analysis if: inputs.build_codeql == true - uses: github/codeql-action/analyze@ea9e4e37992a54ee68a9622e985e60c8e8f12d9f + uses: github/codeql-action/analyze@df409f7d9260372bd5f19e5b04e83cb3c43714ae diff --git a/.github/workflows/scorecards.yml b/.github/workflows/scorecards.yml index 4d9dd71b7..35656465b 100644 --- a/.github/workflows/scorecards.yml +++ b/.github/workflows/scorecards.yml @@ -31,7 +31,7 @@ jobs: steps: - name: Harden Runner - uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1 + uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2 with: egress-policy: audit @@ -71,6 +71,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard. - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@ea9e4e37992a54ee68a9622e985e60c8e8f12d9f # v3.27.4 + uses: github/codeql-action/upload-sarif@df409f7d9260372bd5f19e5b04e83cb3c43714ae # v3.27.9 with: sarif_file: results.sarif diff --git a/.github/workflows/update-docs.yml b/.github/workflows/update-docs.yml index 62b6d060e..9e87b0567 100644 --- a/.github/workflows/update-docs.yml +++ b/.github/workflows/update-docs.yml @@ -30,7 +30,7 @@ jobs: steps: - name: Harden Runner - uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 + uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f with: egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 86d92d0b5..9738f7ae5 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -47,7 +47,7 @@ jobs: steps: - name: Harden Runner - uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1 + uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2 with: egress-policy: audit @@ -57,12 +57,12 @@ jobs: - name: Initialize CodeQL if: inputs.build_codeql == true - uses: github/codeql-action/init@ea9e4e37992a54ee68a9622e985e60c8e8f12d9f + uses: github/codeql-action/init@df409f7d9260372bd5f19e5b04e83cb3c43714ae with: languages: 'cpp' - name: Cache the build folder - uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a + uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 env: cache-name: cache-nuget-modules with: @@ -124,4 +124,4 @@ jobs: - name: Perform CodeQL Analysis if: inputs.build_codeql == true - uses: github/codeql-action/analyze@ea9e4e37992a54ee68a9622e985e60c8e8f12d9f + uses: github/codeql-action/analyze@df409f7d9260372bd5f19e5b04e83cb3c43714ae diff --git a/.gitignore b/.gitignore index 672fbe7f2..2222dedae 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ __pycache__/ # Distribution / packaging .Python build/ +build_fuzzer/ develop-eggs/ dist/ downloads/ 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/external/bpf_conformance b/external/bpf_conformance index afc0282f7..514fa974b 160000 --- a/external/bpf_conformance +++ b/external/bpf_conformance @@ -1 +1 @@ -Subproject commit afc0282f775ce3f94b98b73498b9d4f77b1f2ff7 +Subproject commit 514fa974bdf0274a1365a2c4e738b5fe5b23cb75 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.