From 726a856cbc4f23d51707435d6f25c2ba941eb653 Mon Sep 17 00:00:00 2001 From: Dave Thaler Date: Fri, 22 Nov 2024 16:55:41 -0800 Subject: [PATCH] Update verifier to latest (#4033) * Update verifier Signed-off-by: Dave Thaler * Make some more tests pass Signed-off-by: Dave Thaler * More test fixes --------- Signed-off-by: Dave Thaler --- external/ebpf-verifier | 2 +- libs/api/Verifier.cpp | 11 ++-- libs/api/crab_verifier_wrapper.hpp | 1 + libs/api_common/api_common.cpp | 39 ++++++++++++++ libs/api_common/api_common.hpp | 8 +++ libs/api_common/ebpf_verifier_wrapper.hpp | 1 + libs/api_common/windows_platform_common.cpp | 2 +- libs/service/verifier_service.cpp | 6 +-- tests/end_to_end/netsh_test.cpp | 56 +++++++++++++-------- tests/unit/libbpf_test.cpp | 4 +- 10 files changed, 97 insertions(+), 33 deletions(-) diff --git a/external/ebpf-verifier b/external/ebpf-verifier index 41fbd5a378..e4750ee30e 160000 --- a/external/ebpf-verifier +++ b/external/ebpf-verifier @@ -1 +1 @@ -Subproject commit 41fbd5a378e80afbb45813a72a3353226c899fef +Subproject commit e4750ee30efe6b3634621735f63038d7d8d3ae05 diff --git a/libs/api/Verifier.cpp b/libs/api/Verifier.cpp index c579c45a0d..29543b9bbf 100644 --- a/libs/api/Verifier.cpp +++ b/libs/api/Verifier.cpp @@ -652,10 +652,10 @@ _ebpf_api_elf_verify_program_from_stream( ebpf_verifier_options_t verifier_options{}; verifier_options.assume_assertions = verbosity < EBPF_VERIFICATION_VERBOSITY_VERBOSE; verifier_options.cfg_opts.check_for_termination = true; - verifier_options.print_invariants = verbosity >= EBPF_VERIFICATION_VERBOSITY_INFORMATIONAL; - verifier_options.print_failures = true; + verifier_options.verbosity_opts.print_invariants = verbosity >= EBPF_VERIFICATION_VERBOSITY_INFORMATIONAL; + verifier_options.verbosity_opts.print_failures = true; verifier_options.mock_map_fds = true; - verifier_options.print_line_info = true; + verifier_options.verbosity_opts.print_line_info = true; if (!stream) { throw std::runtime_error(std::string("No such file or directory opening ") + stream_name); } @@ -683,9 +683,8 @@ _ebpf_api_elf_verify_program_from_stream( } auto& program = std::get(programOrError); - verifier_options.cfg_opts.simplify = false; - bool res = - ebpf_verify_program(output, program, raw_program.info, verifier_options, (ebpf_verifier_stats_t*)stats); + verifier_options.verbosity_opts.simplify = false; + bool res = ebpf_verify_program(output, program, raw_program.info, verifier_options, stats); if (!res) { error << "Verification failed"; *error_message = allocate_string(error.str()); diff --git a/libs/api/crab_verifier_wrapper.hpp b/libs/api/crab_verifier_wrapper.hpp index dea4ec8088..9e8969327f 100644 --- a/libs/api/crab_verifier_wrapper.hpp +++ b/libs/api/crab_verifier_wrapper.hpp @@ -5,6 +5,7 @@ #pragma warning(disable : 4100) // 'identifier' : unreferenced formal parameter #pragma warning(disable : 4244) // 'conversion' conversion from 'type1' to // 'type2', possible loss of data +#pragma warning(disable : 4267) // conversion from 'size_t' to 'int', possible loss of data #pragma warning(disable : 26451) // Arithmetic overflow #pragma warning(disable : 26450) // Arithmetic overflow #pragma warning(disable : 26450) // Arithmetic overflow diff --git a/libs/api_common/api_common.cpp b/libs/api_common/api_common.cpp index 4a4d2adc9e..a622e1103b 100644 --- a/libs/api_common/api_common.cpp +++ b/libs/api_common/api_common.cpp @@ -156,3 +156,42 @@ ebpf_clear_thread_local_storage() noexcept set_verification_in_progress(false); clean_up_sync_device_handle(); } + +// Returned value is true if the program passes verification. +bool +ebpf_verify_program( + std::ostream& os, + _In_ const InstructionSeq& prog, + _In_ const program_info& info, + _In_ const ebpf_verifier_options_t& options, + _Out_ ebpf_api_verifier_stats_t* stats) +{ + stats->total_unreachable = 0; + stats->total_warnings = 0; + stats->max_loop_count = 0; + + // Convert the instruction sequence to a control-flow graph. + try { + const cfg_t cfg = prepare_cfg(prog, info, options.cfg_opts); + auto invariants = analyze(cfg); + if (options.verbosity_opts.print_invariants) { + invariants.print_invariants(os, cfg); + } + bool pass; + if (options.verbosity_opts.print_failures) { + auto report = invariants.check_assertions(cfg); + thread_local_options.verbosity_opts.print_line_info = true; + report.print_warnings(os); + pass = report.verified(); + stats->total_warnings = (int)report.warning_set().size(); + stats->total_unreachable = (int)report.reachability_set().size(); + } else { + pass = invariants.verified(cfg); + } + stats->max_loop_count = invariants.max_loop_count(); + return pass; + } catch (UnmarshalError& e) { + os << "error: " << e.what() << std::endl; + return false; + } +} \ No newline at end of file diff --git a/libs/api_common/api_common.hpp b/libs/api_common/api_common.hpp index e0ca902383..9167ebc877 100644 --- a/libs/api_common/api_common.hpp +++ b/libs/api_common/api_common.hpp @@ -238,3 +238,11 @@ set_program_under_verification(ebpf_handle_t program); */ void ebpf_clear_thread_local_storage() noexcept; + +bool +ebpf_verify_program( + std::ostream& os, + _In_ const InstructionSeq& prog, + _In_ const program_info& info, + _In_ const ebpf_verifier_options_t& options, + _Out_ ebpf_api_verifier_stats_t* stats); \ No newline at end of file diff --git a/libs/api_common/ebpf_verifier_wrapper.hpp b/libs/api_common/ebpf_verifier_wrapper.hpp index 0f4b439f06..5eca8c54cd 100644 --- a/libs/api_common/ebpf_verifier_wrapper.hpp +++ b/libs/api_common/ebpf_verifier_wrapper.hpp @@ -5,6 +5,7 @@ #pragma warning(disable : 4100) // 'identifier' : unreferenced formal parameter #pragma warning(disable : 4244) // 'conversion' conversion from 'type1' to // 'type2', possible loss of data +#pragma warning(disable : 4267) // conversion from 'size_t' to 'int', possible loss of data #pragma warning(disable : 26451) // Arithmetic overflow #pragma warning(disable : 26450) // Arithmetic overflow #pragma warning(disable : 26439) // This kind of function may not diff --git a/libs/api_common/windows_platform_common.cpp b/libs/api_common/windows_platform_common.cpp index f4e6942f34..0e2dee76f8 100644 --- a/libs/api_common/windows_platform_common.cpp +++ b/libs/api_common/windows_platform_common.cpp @@ -775,7 +775,7 @@ clear_ebpf_provider_data() _Success_(return == EBPF_SUCCESS) ebpf_result_t get_program_type_info_from_tls(_Outptr_ const ebpf_program_info_t** info) { - const GUID* program_type = reinterpret_cast(global_program_info->type.platform_specific_data); + const GUID* program_type = reinterpret_cast(thread_local_program_info->type.platform_specific_data); ebpf_result_t result = EBPF_SUCCESS; _load_ebpf_provider_data(); diff --git a/libs/service/verifier_service.cpp b/libs/service/verifier_service.cpp index ff54d31d2a..f26553c785 100644 --- a/libs/service/verifier_service.cpp +++ b/libs/service/verifier_service.cpp @@ -26,14 +26,14 @@ _analyze(raw_program& raw_prog, const char** error_message, uint32_t* error_mess // First try optimized for the success case. ebpf_verifier_options_t options{}; - ebpf_verifier_stats_t stats; + ebpf_api_verifier_stats_t stats; options.cfg_opts.check_for_termination = true; bool res = ebpf_verify_program(std::cout, prog, raw_prog.info, options, &stats); if (!res) { // On failure, retry to get the more detailed error message. std::ostringstream oss; - options.cfg_opts.simplify = false; - options.print_failures = true; + options.verbosity_opts.simplify = false; + options.verbosity_opts.print_failures = true; // Until https://github.com/vbpf/ebpf-verifier/issues/643 is fixed, don't set options.assume_assertions to true. (void)ebpf_verify_program(oss, prog, raw_prog.info, options, &stats); diff --git a/tests/end_to_end/netsh_test.cpp b/tests/end_to_end/netsh_test.cpp index 6450b16858..1c45c472b1 100644 --- a/tests/end_to_end/netsh_test.cpp +++ b/tests/end_to_end/netsh_test.cpp @@ -195,7 +195,7 @@ TEST_CASE("show sections bpf.o .text", "[netsh][sections]") "arith32 : 0\n" "arith64 : 1\n" "assign : 1\n" - "basic_blocks : 2\n" + "basic_blocks : 4\n" "call_1 : 0\n" "call_mem : 0\n" "call_nomem : 0\n" @@ -204,7 +204,7 @@ TEST_CASE("show sections bpf.o .text", "[netsh][sections]") "load : 0\n" "load_store : 0\n" "map_in_map : 0\n" - "other : 1\n" + "other : 3\n" "packet_access: 0\n" "reallocate : 0\n" "store : 0\n" @@ -406,7 +406,6 @@ TEST_CASE("show verification bpf.o", "[netsh][verification]") REQUIRE(result == NO_ERROR); REQUIRE( output == "\n" - "\n" "Verification succeeded\n" "Program terminates within 0 loop iterations\n"); } @@ -421,7 +420,6 @@ TEST_CASE("show verification droppacket.o", "[netsh][verification]") REQUIRE(result == NO_ERROR); REQUIRE( output == "\n" - "\n" "Verification succeeded\n" "Program terminates within 0 loop iterations\n"); } @@ -443,6 +441,7 @@ TEST_CASE("show verification xdp_adjust_head_unsafe.o", "[netsh][verification]") "\n" "; ./tests/sample/unsafe/xdp_adjust_head_unsafe.c:43\n" "; ethernet_header->Type = 0x0800;\n" + "\n" "17: Upper bound must be at most packet_size (valid_access(r1.offset+12, width=2) for write)\n" "\n" "1 errors\n" @@ -466,9 +465,12 @@ TEST_CASE("show verification droppacket_unsafe.o", "[netsh][verification]") "\n" "; ./tests/sample/unsafe/droppacket_unsafe.c:42\n" "; if (ip_header->Protocol == IPPROTO_UDP) {\n" + "\n" "2: Upper bound must be at most packet_size (valid_access(r1.offset+9, width=1) for read)\n" + "\n" "; ./tests/sample/unsafe/droppacket_unsafe.c:43\n" "; if (ntohs(udp_header->length) <= sizeof(UDP_HEADER)) {\n" + "\n" "4: Upper bound must be at most packet_size (valid_access(r1.offset+24, width=2) for read)\n" "\n" "2 errors\n" @@ -487,19 +489,35 @@ TEST_CASE("show verification xdp_datasize_unsafe.o", "[netsh][verification]") output = strip_paths(output); // Perform a line by line comparison to detect any differences. - std::string expected_output = "Verification failed\n" - "\n" - "Verification report:\n" - "\n" - "; ./tests/sample/unsafe/xdp_datasize_unsafe.c:33\n" - "; if (next_header + sizeof(ETHERNET_HEADER) > (char*)ctx->data_end) {\n" - "4: Invalid type (r3.type in {number, ctx, stack, packet, shared})\n" - "; ./tests/sample/unsafe/xdp_datasize_unsafe.c:33\n" - "; if (next_header + sizeof(ETHERNET_HEADER) > (char*)ctx->data_end) {\n" - "4: Code is unreachable after 4\n" - "\n" - "1 errors\n" - "\n"; + std::string expected_output = + "Verification failed\n" + "\n" + "Verification report:\n" + "\n" + "; ./tests/sample/unsafe/xdp_datasize_unsafe.c:33\n" + "; if (next_header + sizeof(ETHERNET_HEADER) > (char*)ctx->data_end) {\n" + "\n" + "4: Invalid type (r3.type in {number, ctx, stack, packet, shared})\n" + "5: Invalid type (valid_access(r3.offset) for comparison/subtraction)\n" + "5: Invalid type (r3.type in {number, ctx, stack, packet, shared})\n" + "5: Cannot subtract pointers to different regions (r3.type == r1.type in {ctx, stack, packet})\n" + "\n" + "; ./tests/sample/unsafe/xdp_datasize_unsafe.c:39\n" + "; if (ethernet_header->Type != ntohs(ETHERNET_TYPE_IPV4) && ethernet_header->Type != " + "ntohs(ETHERNET_TYPE_IPV6)) {\n" + "\n" + "6: Invalid type (r2.type in {ctx, stack, packet, shared})\n" + "6: Invalid type (valid_access(r2.offset+12, width=2) for read)\n" + "8: Invalid type (r1.type == number)\n" + "10: Invalid type (r1.type == number)\n" + "\n" + "; ./tests/sample/unsafe/xdp_datasize_unsafe.c:44\n" + "; return rc;\n" + "\n" + "12: Invalid type (r0.type == number)\n" + "\n" + "9 errors\n" + "\n"; // Split both output and expected_output into lines. std::istringstream output_stream(output); @@ -528,10 +546,8 @@ TEST_CASE("show verification printk_unsafe.o", "[netsh][verification]") "\n" "; ./tests/sample/unsafe/printk_unsafe.c:22\n" "; bpf_printk(\"ctx: %u\", (uint64_t)ctx);\n" + "\n" "7: Invalid type (r3.type == number)\n" - "; ./tests/sample/unsafe/printk_unsafe.c:22\n" - "; bpf_printk(\"ctx: %u\", (uint64_t)ctx);\n" - "7: Code is unreachable after 7\n" "\n" "1 errors\n" "\n"; diff --git a/tests/unit/libbpf_test.cpp b/tests/unit/libbpf_test.cpp index 4811a4adc6..7460a566ec 100644 --- a/tests/unit/libbpf_test.cpp +++ b/tests/unit/libbpf_test.cpp @@ -220,7 +220,7 @@ TEST_CASE("invalid bpf_load_program", "[libbpf][deprecated]") REQUIRE(program_fd < 0); #if !defined(CONFIG_BPF_JIT_DISABLED) REQUIRE(errno == EACCES); - REQUIRE(strcmp(log_buffer, "\n0: Invalid type (r0.type == number)\n\n") == 0); + REQUIRE(strcmp(log_buffer, "0: Invalid type (r0.type == number)\n\n") == 0); #else REQUIRE(errno == ENOTSUP); #endif @@ -244,7 +244,7 @@ TEST_CASE("invalid bpf_prog_load", "[libbpf]") REQUIRE(program_fd < 0); #if !defined(CONFIG_BPF_JIT_DISABLED) REQUIRE(errno == EACCES); - REQUIRE(strcmp(log_buffer, "\n0: Invalid type (r0.type == number)\n\n") == 0); + REQUIRE(strcmp(log_buffer, "0: Invalid type (r0.type == number)\n\n") == 0); #else REQUIRE(errno == ENOTSUP); #endif