From 96226a7db9d4cb3e49a3891a201cc8a0bd07d7ef Mon Sep 17 00:00:00 2001 From: saxena-anurag <43585259+saxena-anurag@users.noreply.github.com> Date: Wed, 15 Jun 2022 08:59:39 -0700 Subject: [PATCH] Change default program type to EBPF_PROGRAM_TYPE_UNSPECIFIED (#1173) * fixes * update cmakelists * fix bpf2c tests * fix cilium tests * add tests * update bpf2c issue number * Apply suggestions from code review Co-authored-by: Dave Thaler * fix * fix * fix * cr comments * fix build break Co-authored-by: Dave Thaler Co-authored-by: Alan Jowett --- include/ebpf_api.h | 16 +++- include/ebpf_structs.h | 2 + libs/api/Verifier.cpp | 18 +++- libs/api/ebpf_api.cpp | 82 ++++++++++--------- libs/api/libbpf_program.cpp | 4 +- libs/api_common/CMakeLists.txt | 3 + libs/api_common/api_common.cpp | 16 ++++ libs/api_common/api_common.hpp | 6 ++ libs/api_common/api_common.vcxproj | 2 + libs/api_common/api_common.vcxproj.filters | 6 ++ libs/api_common/utilities.cpp | 53 ++++++++++++ libs/api_common/utilities.hpp | 13 +++ libs/api_common/windows_platform_common.cpp | 31 ++++++- libs/api_common/windows_program_type.h | 45 +++++++--- libs/ebpfnetsh/elf.cpp | 32 +++++++- libs/service/api_service.cpp | 1 + libs/service/verifier_service.cpp | 9 +- tests/bpf2c_tests/expected/bpf_dll.c | 2 +- tests/bpf2c_tests/expected/bpf_raw.c | 2 +- tests/bpf2c_tests/expected/bpf_sys.c | 2 +- .../expected/cgroup_sock_addr_dll.c | 2 +- .../expected/cgroup_sock_addr_raw.c | 2 +- .../expected/cgroup_sock_addr_sys.c | 2 +- tests/cilium/cilium_tests.cpp | 3 +- tests/end_to_end/end_to_end.cpp | 59 +++++++++++-- tests/end_to_end/netsh_test.cpp | 4 +- tests/end_to_end/test_helper.cpp | 3 + tests/libfuzzer/verifier/libfuzz_harness.cpp | 2 +- tests/unit/libbpf_test.cpp | 7 +- tools/bpf2c/bpf2c.cpp | 18 +++- .../encode_program_info.cpp | 3 +- 31 files changed, 362 insertions(+), 88 deletions(-) create mode 100644 libs/api_common/utilities.cpp create mode 100644 libs/api_common/utilities.hpp diff --git a/include/ebpf_api.h b/include/ebpf_api.h index 0b106d6d2..13525f653 100644 --- a/include/ebpf_api.h +++ b/include/ebpf_api.h @@ -102,6 +102,8 @@ extern "C" _Field_z_ const char* section_name; _Field_z_ const char* program_type_name; _Field_z_ const char* program_name; + ebpf_program_type_t program_type; + ebpf_attach_type_t expected_attach_type; size_t raw_data_size; _Field_size_(raw_data_size) char* raw_data; ebpf_stat_t* stats; @@ -153,6 +155,8 @@ extern "C" * @brief Verify that the program is safe to execute. * @param[in] file Name of ELF file containing eBPF program. * @param[in] section The name of the section to query. + * @param[in] program_type Optional program type. + * If NULL, the program type is derived from the section name. * @param[in] verbose Obtain additional info about the programs. * @param[out] report Points to a text section describing why the program * failed verification. @@ -162,8 +166,9 @@ extern "C" */ uint32_t ebpf_api_elf_verify_section_from_file( - const char* file, - const char* section, + _In_z_ const char* file, + _In_z_ const char* section, + _In_opt_ const ebpf_program_type_t* program_type, bool verbose, const char** report, const char** error_message, @@ -174,6 +179,8 @@ extern "C" * @param[in] data Memory containing the ELF file containing eBPF program. * @param[in] data_length Length of data. * @param[in] section The name of the section to query. + * @param[in] program_type Optional program type. + * If NULL, the program type is derived from the section name. * @param[in] verbose Obtain additional info about the programs. * @param[out] report Points to a text section describing why the program * failed verification. @@ -183,9 +190,10 @@ extern "C" */ uint32_t ebpf_api_elf_verify_section_from_memory( - const char* data, + _In_reads_(data_length) const char* data, size_t data_length, - const char* section, + _In_z_ const char* section, + _In_opt_ const ebpf_program_type_t* program_type, bool verbose, const char** report, const char** error_message, diff --git a/include/ebpf_structs.h b/include/ebpf_structs.h index 1ded8bded..c5954bd87 100644 --- a/include/ebpf_structs.h +++ b/include/ebpf_structs.h @@ -285,6 +285,8 @@ enum bpf_attach_type __MAX_BPF_ATTACH_TYPE, }; +typedef enum bpf_attach_type bpf_attach_type_t; + // Libbpf itself requires the following structs to be defined, but doesn't // care what fields they have. Applications such as bpftool on the other // hand depend on fields of specific names and types. diff --git a/libs/api/Verifier.cpp b/libs/api/Verifier.cpp index 56a096ace..5969a4856 100644 --- a/libs/api/Verifier.cpp +++ b/libs/api/Verifier.cpp @@ -528,27 +528,37 @@ _ebpf_api_elf_verify_section_from_stream( uint32_t ebpf_api_elf_verify_section_from_file( - const char* file, - const char* section, + _In_z_ const char* file, + _In_z_ const char* section, + _In_opt_ const ebpf_program_type_t* program_type, bool verbose, const char** report, const char** error_message, ebpf_api_verifier_stats_t* stats) { std::ifstream stream{file, std::ios::in | std::ios::binary}; + struct _thread_local_storage_cache tls_cache; + + set_global_program_and_attach_type(program_type, nullptr); + set_verification_in_progress(true); return _ebpf_api_elf_verify_section_from_stream(stream, file, section, verbose, report, error_message, stats); } uint32_t ebpf_api_elf_verify_section_from_memory( - const char* data, + _In_reads_(data_length) const char* data, size_t data_length, - const char* section, + _In_z_ const char* section, + _In_opt_ const ebpf_program_type_t* program_type, bool verbose, const char** report, const char** error_message, ebpf_api_verifier_stats_t* stats) { std::stringstream stream(std::string(data, data_length)); + struct _thread_local_storage_cache tls_cache; + + set_global_program_and_attach_type(program_type, nullptr); + set_verification_in_progress(true); return _ebpf_api_elf_verify_section_from_stream(stream, "memory", section, verbose, report, error_message, stats); } diff --git a/libs/api/ebpf_api.cpp b/libs/api/ebpf_api.cpp index baf4561e9..1e6b1205b 100644 --- a/libs/api/ebpf_api.cpp +++ b/libs/api/ebpf_api.cpp @@ -31,6 +31,7 @@ extern "C" { #include "ubpf.h" } +#include "utilities.hpp" #include "Verifier.h" #include "windows_platform_common.hpp" @@ -1578,11 +1579,8 @@ _initialize_ebpf_object_from_native_file( } program->handle = ebpf_handle_invalid; - - result = ebpf_get_program_type_by_name(info->program_type_name, &program->program_type, &program->attach_type); - if (result != EBPF_SUCCESS) { - goto Exit; - } + program->program_type = info->program_type; + program->attach_type = info->expected_attach_type; program->section_name = _strdup(info->section_name); if (program->section_name == nullptr) { @@ -1788,6 +1786,7 @@ typedef struct _ebpf_pe_context std::map section_names; std::map program_names; std::map section_program_types; + std::map section_attach_types; uintptr_t rdata_base; size_t rdata_size; const bounded_buffer* rdata_buffer; @@ -1960,6 +1959,13 @@ _ebpf_pe_get_section_names( program_type_guid_address < pe_context->data_base + pe_context->data_size); uintptr_t offset = program_type_guid_address - pe_context->data_base; pe_context->section_program_types[pe_section_name] = *(GUID*)(pe_context->data_buffer->buf + offset); + + uintptr_t attach_type_guid_address = (uintptr_t)program->expected_attach_type; + ebpf_assert( + attach_type_guid_address >= pe_context->data_base && + attach_type_guid_address < pe_context->data_base + pe_context->data_size); + offset = attach_type_guid_address - pe_context->data_base; + pe_context->section_attach_types[pe_section_name] = *(GUID*)(pe_context->data_buffer->buf + offset); } } @@ -1999,11 +2005,18 @@ _ebpf_pe_add_section( EBPF_LOG_EXIT(); return 1; } + memset(info, 0, sizeof(*info)); info->section_name = _strdup(elf_section_name.c_str()); info->program_name = _strdup(program_name.c_str()); - info->program_type_name = - _strdup(get_program_type_windows(pe_context->section_program_types[pe_section_name]).name.c_str()); + info->program_type = pe_context->section_program_types[pe_section_name]; + info->expected_attach_type = pe_context->section_attach_types[pe_section_name]; + info->program_type_name = ebpf_get_program_type_name(&pe_context->section_program_types[pe_section_name]); + if (info->program_type_name == nullptr) { + EBPF_LOG_EXIT(); + return 1; + } + info->program_type_name = _strdup(info->program_type_name); info->raw_data_size = section_header.Misc.VirtualSize; info->raw_data = (char*)malloc(section_header.Misc.VirtualSize); if (info->raw_data == nullptr || info->program_type_name == nullptr || info->section_name == nullptr) { @@ -2715,30 +2728,6 @@ Done: EBPF_RETURN_RESULT(result); } -std::wstring -_guid_to_wide_string(_In_ const GUID* guid) -{ - ebpf_assert(guid); - wchar_t guid_string[37] = {0}; - swprintf( - guid_string, - sizeof(guid_string) / sizeof(guid_string[0]), - L"%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x", - guid->Data1, - guid->Data2, - guid->Data3, - guid->Data4[0], - guid->Data4[1], - guid->Data4[2], - guid->Data4[3], - guid->Data4[4], - guid->Data4[5], - guid->Data4[6], - guid->Data4[7]); - - return std::wstring(guid_string); -} - static ebpf_result_t _ebpf_program_load_native( _In_z_ const char* file_name, @@ -2790,7 +2779,7 @@ _ebpf_program_load_native( try { // Create a driver service with a random name. - service_name = _guid_to_wide_string(&service_name_guid); + service_name = guid_to_wide_string(&service_name_guid); error = Platform::_create_service( service_name.c_str(), _get_wstring_from_string(file_name_string).c_str(), &service_handle); @@ -3197,18 +3186,29 @@ ebpf_result_t ebpf_get_program_type_by_name( _In_z_ const char* name, _Out_ ebpf_program_type_t* program_type, _Out_ ebpf_attach_type_t* expected_attach_type) { + ebpf_result_t result = EBPF_SUCCESS; + ebpf_program_type_t* program_type_uuid; EBPF_LOG_ENTRY(); ebpf_assert(name); ebpf_assert(program_type); ebpf_assert(expected_attach_type); - EbpfProgramType type = get_program_type_windows(name, name); - ebpf_program_type_t* program_type_uuid = (ebpf_program_type_t*)type.platform_specific_data; + try { + const EbpfProgramType& type = get_program_type_windows(name, name); + if (IsEqualGUID(*((ebpf_program_type_t*)type.platform_specific_data), EBPF_PROGRAM_TYPE_UNSPECIFIED)) { + result = EBPF_KEY_NOT_FOUND; + goto Exit; + } + program_type_uuid = (ebpf_program_type_t*)type.platform_specific_data; - *program_type = *program_type_uuid; - *expected_attach_type = *(get_attach_type_windows(name)); + *program_type = *program_type_uuid; + *expected_attach_type = *(get_attach_type_windows(name)); + } catch (...) { + result = EBPF_KEY_NOT_FOUND; + } - EBPF_RETURN_RESULT(EBPF_SUCCESS); +Exit: + EBPF_RETURN_RESULT(result); } _Ret_maybenull_z_ const char* @@ -3216,8 +3216,12 @@ ebpf_get_program_type_name(_In_ const ebpf_program_type_t* program_type) { EBPF_LOG_ENTRY(); ebpf_assert(program_type); - const EbpfProgramType& type = get_program_type_windows(*program_type); - EBPF_RETURN_POINTER(const char*, type.name.c_str()); + try { + const EbpfProgramType& type = get_program_type_windows(*program_type); + EBPF_RETURN_POINTER(const char*, type.name.c_str()); + } catch (...) { + return nullptr; + } } _Ret_maybenull_z_ const char* diff --git a/libs/api/libbpf_program.cpp b/libs/api/libbpf_program.cpp index 378498120..b1dda3d94 100644 --- a/libs/api/libbpf_program.cpp +++ b/libs/api/libbpf_program.cpp @@ -513,7 +513,8 @@ libbpf_prog_type_by_name(const char* name, enum bpf_prog_type* prog_type, enum b ebpf_attach_type_t expected_attach_type_uuid; ebpf_result_t result = ebpf_get_program_type_by_name(name, &program_type_uuid, &expected_attach_type_uuid); if (result != EBPF_SUCCESS) { - return libbpf_result_err(result); + ebpf_assert(result == EBPF_KEY_NOT_FOUND); + goto Exit; } // Convert UUIDs to enum values if they exist. @@ -529,6 +530,7 @@ libbpf_prog_type_by_name(const char* name, enum bpf_prog_type* prog_type, enum b return 0; } +Exit: errno = ESRCH; return -1; } diff --git a/libs/api_common/CMakeLists.txt b/libs/api_common/CMakeLists.txt index ec2da4906..1a7cb287f 100644 --- a/libs/api_common/CMakeLists.txt +++ b/libs/api_common/CMakeLists.txt @@ -16,6 +16,9 @@ add_library("api_common" STATIC device_helper.hpp device_helper.cpp + + utilities.hpp + utilities.cpp ) target_include_directories("api_common" PRIVATE diff --git a/libs/api_common/api_common.cpp b/libs/api_common/api_common.cpp index cdef14bf9..2f28b2cb7 100644 --- a/libs/api_common/api_common.cpp +++ b/libs/api_common/api_common.cpp @@ -18,6 +18,9 @@ thread_local static const ebpf_program_type_t* _global_program_type = nullptr; thread_local static const ebpf_attach_type_t* _global_attach_type = nullptr; +// Whether a program verification is in progress. +thread_local static bool _verification_in_progress = false; + const char* allocate_string(const std::string& string, uint32_t* length) noexcept { @@ -130,6 +133,18 @@ get_global_attach_type() return _global_attach_type; } +void +set_verification_in_progress(bool value) +{ + _verification_in_progress = value; +} + +bool +get_verification_in_progress() +{ + return _verification_in_progress; +} + void ebpf_clear_thread_local_storage() noexcept { @@ -137,4 +152,5 @@ ebpf_clear_thread_local_storage() noexcept clear_map_descriptors(); clear_program_info_cache(); set_program_under_verification(ebpf_handle_invalid); + set_verification_in_progress(false); } \ No newline at end of file diff --git a/libs/api_common/api_common.hpp b/libs/api_common/api_common.hpp index b03c00c28..c530dae7d 100644 --- a/libs/api_common/api_common.hpp +++ b/libs/api_common/api_common.hpp @@ -160,6 +160,12 @@ get_global_program_type(); const ebpf_attach_type_t* get_global_attach_type(); +void +set_verification_in_progress(bool value); + +bool +get_verification_in_progress(); + /** * @brief Save handle to program being verified in thread-local storage. * diff --git a/libs/api_common/api_common.vcxproj b/libs/api_common/api_common.vcxproj index a91bfae51..165cfb100 100644 --- a/libs/api_common/api_common.vcxproj +++ b/libs/api_common/api_common.vcxproj @@ -153,6 +153,7 @@ + @@ -161,6 +162,7 @@ + diff --git a/libs/api_common/api_common.vcxproj.filters b/libs/api_common/api_common.vcxproj.filters index fa4fda0ff..2a8da1706 100644 --- a/libs/api_common/api_common.vcxproj.filters +++ b/libs/api_common/api_common.vcxproj.filters @@ -31,6 +31,9 @@ Source Files + + Source Files + @@ -51,5 +54,8 @@ Header Files + + Header Files + \ No newline at end of file diff --git a/libs/api_common/utilities.cpp b/libs/api_common/utilities.cpp new file mode 100644 index 000000000..fb77dda16 --- /dev/null +++ b/libs/api_common/utilities.cpp @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation +// SPDX-License-Identifier: MIT + +#include "framework.h" +#include "utilities.hpp" + +std::wstring +guid_to_wide_string(_In_ const GUID* guid) +{ + ebpf_assert(guid); + wchar_t guid_string[37] = {0}; + swprintf( + guid_string, + sizeof(guid_string) / sizeof(guid_string[0]), + L"%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x", + guid->Data1, + guid->Data2, + guid->Data3, + guid->Data4[0], + guid->Data4[1], + guid->Data4[2], + guid->Data4[3], + guid->Data4[4], + guid->Data4[5], + guid->Data4[6], + guid->Data4[7]); + + return std::wstring(guid_string); +} + +std::string +guid_to_string(_In_ const GUID* guid) +{ + ebpf_assert(guid); + char guid_string[37] = {0}; + sprintf_s( + guid_string, + sizeof(guid_string) / sizeof(guid_string[0]), + "%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x", + guid->Data1, + guid->Data2, + guid->Data3, + guid->Data4[0], + guid->Data4[1], + guid->Data4[2], + guid->Data4[3], + guid->Data4[4], + guid->Data4[5], + guid->Data4[6], + guid->Data4[7]); + + return std::string(guid_string); +} \ No newline at end of file diff --git a/libs/api_common/utilities.hpp b/libs/api_common/utilities.hpp new file mode 100644 index 000000000..b2a733800 --- /dev/null +++ b/libs/api_common/utilities.hpp @@ -0,0 +1,13 @@ +// Copyright (c) Microsoft Corporation +// SPDX-License-Identifier: MIT + +#pragma once + +#include +#include + +std::wstring +guid_to_wide_string(_In_ const GUID* guid); + +std::string +guid_to_string(_In_ const GUID* guid); \ No newline at end of file diff --git a/libs/api_common/windows_platform_common.cpp b/libs/api_common/windows_platform_common.cpp index 26c8ab711..d6214d414 100644 --- a/libs/api_common/windows_platform_common.cpp +++ b/libs/api_common/windows_platform_common.cpp @@ -12,6 +12,7 @@ #include "map_descriptors.hpp" #include "platform.hpp" #include "spec_type_descriptors.hpp" +#include "utilities.hpp" #include "windows_program_type.h" #include "windows_platform.hpp" @@ -21,7 +22,7 @@ get_program_type_windows(const GUID& program_type) // TODO: (Issue #67) Make an IOCTL call to fetch the program context // info and then fill the EbpfProgramType struct. for (const EbpfProgramType& t : windows_program_types) { - if (t.platform_specific_data != 0) { + if (t.platform_specific_data != (uint64_t)&EBPF_PROGRAM_TYPE_UNSPECIFIED) { ebpf_program_type_t* program_type_uuid = (ebpf_program_type_t*)t.platform_specific_data; if (IsEqualGUID(*program_type_uuid, program_type)) { return t; @@ -29,7 +30,8 @@ get_program_type_windows(const GUID& program_type) } } - return windows_xdp_program_type; + auto guid_string = guid_to_string(&program_type); + throw std::runtime_error(std::string("ProgramType not found for GUID ") + guid_string); } EbpfProgramType @@ -42,7 +44,7 @@ get_program_type_windows(const std::string& section, const std::string&) // prefixes and corresponding program and attach types. for (const EbpfProgramType& t : windows_program_types) { if (program_type != nullptr) { - if (t.platform_specific_data != 0) { + if (t.platform_specific_data != (uint64_t)&EBPF_PROGRAM_TYPE_UNSPECIFIED) { ebpf_program_type_t* program_type_uuid = (ebpf_program_type_t*)t.platform_specific_data; if (IsEqualGUID(*program_type_uuid, *program_type)) { return t; @@ -56,7 +58,28 @@ get_program_type_windows(const std::string& section, const std::string&) } } - return windows_xdp_program_type; + // Note: Ideally this function should throw an exception whenever a matching ProgramType is not found, + // but that causes a problem in the following scenario: + // + // This function is called by verifier code in 2 cases: + // 1. When verifying the code + // 2. When parsing the ELF file and unmarshalling the code. + // For the second case mentioned above, if the ELF file contains an unknown section name (".text", for example), + // and this function is called while unmarshalling that section, throwing an exception here + // will fail the parsing of the ELF file. + // + // Hence this function returns ProgramType for EBPF_PROGRAM_TYPE_UNSPECIFIED when verification is not + // in progress, and throws an exception otherwise. + if (get_verification_in_progress()) { + if (program_type != nullptr) { + auto guid_string = guid_to_string(program_type); + throw std::runtime_error(std::string("ProgramType not found for GUID ") + guid_string); + } else { + throw std::runtime_error(std::string("ProgramType not found for section " + section)); + } + } + + return windows_unspecified_program_type; } #define BPF_MAP_TYPE(x) BPF_MAP_TYPE_##x, #x diff --git a/libs/api_common/windows_program_type.h b/libs/api_common/windows_program_type.h index 871e3a5ec..fb88db22d 100644 --- a/libs/api_common/windows_program_type.h +++ b/libs/api_common/windows_program_type.h @@ -87,12 +87,15 @@ const ebpf_context_descriptor_t g_sock_ops_context_descriptor = { const EbpfProgramType windows_sock_ops_program_type = { "sockops", &g_sock_ops_context_descriptor, (uint64_t)&EBPF_PROGRAM_TYPE_SOCK_OPS, {"sockops"}}; +const EbpfProgramType windows_unspecified_program_type = + PTYPE("unspec", {0}, (uint64_t)&EBPF_PROGRAM_TYPE_UNSPECIFIED, {}); + // // Global lists and vectors of program and attach types. // const std::vector windows_program_types = { - PTYPE("unspecified", {0}, 0, {}), + windows_unspecified_program_type, windows_xdp_program_type, windows_bind_program_type, windows_sock_addr_program_type, @@ -102,27 +105,49 @@ const std::vector windows_program_types = { typedef struct _ebpf_section_definition { _Field_z_ const char* section_prefix; - ebpf_program_type_t* prog_type; + ebpf_program_type_t* program_type; ebpf_attach_type_t* attach_type; + bpf_prog_type_t bpf_prog_type; + bpf_attach_type_t bpf_attach_type; } ebpf_section_definition_t; const std::vector windows_section_definitions = { // XDP. - {"xdp", &EBPF_PROGRAM_TYPE_XDP, &EBPF_ATTACH_TYPE_XDP}, + {"xdp", &EBPF_PROGRAM_TYPE_XDP, &EBPF_ATTACH_TYPE_XDP, BPF_PROG_TYPE_XDP, BPF_XDP}, // Bind. - {"bind", &EBPF_PROGRAM_TYPE_BIND, &EBPF_ATTACH_TYPE_BIND}, + {"bind", &EBPF_PROGRAM_TYPE_BIND, &EBPF_ATTACH_TYPE_BIND, BPF_PROG_TYPE_BIND, BPF_ATTACH_TYPE_BIND}, // socket connect v4. - {"cgroup/connect4", &EBPF_PROGRAM_TYPE_CGROUP_SOCK_ADDR, &EBPF_ATTACH_TYPE_CGROUP_INET4_CONNECT}, + {"cgroup/connect4", + &EBPF_PROGRAM_TYPE_CGROUP_SOCK_ADDR, + &EBPF_ATTACH_TYPE_CGROUP_INET4_CONNECT, + BPF_PROG_TYPE_CGROUP_SOCK_ADDR, + BPF_CGROUP_INET4_CONNECT}, // socket connect v6. - {"cgroup/connect4", &EBPF_PROGRAM_TYPE_CGROUP_SOCK_ADDR, &EBPF_ATTACH_TYPE_CGROUP_INET6_CONNECT}, + {"cgroup/connect6", + &EBPF_PROGRAM_TYPE_CGROUP_SOCK_ADDR, + &EBPF_ATTACH_TYPE_CGROUP_INET6_CONNECT, + BPF_PROG_TYPE_CGROUP_SOCK_ADDR, + BPF_CGROUP_INET6_CONNECT}, // socket recv/accept v4. - {"cgroup/recv_accept4", &EBPF_PROGRAM_TYPE_CGROUP_SOCK_ADDR, &EBPF_ATTACH_TYPE_CGROUP_INET4_RECV_ACCEPT}, + {"cgroup/recv_accept4", + &EBPF_PROGRAM_TYPE_CGROUP_SOCK_ADDR, + &EBPF_ATTACH_TYPE_CGROUP_INET4_RECV_ACCEPT, + BPF_PROG_TYPE_CGROUP_SOCK_ADDR, + BPF_CGROUP_INET4_RECV_ACCEPT}, // socket recv/accept v6. - {"cgroup/recv_accept6", &EBPF_PROGRAM_TYPE_CGROUP_SOCK_ADDR, &EBPF_ATTACH_TYPE_CGROUP_INET6_RECV_ACCEPT}, + {"cgroup/recv_accept6", + &EBPF_PROGRAM_TYPE_CGROUP_SOCK_ADDR, + &EBPF_ATTACH_TYPE_CGROUP_INET6_RECV_ACCEPT, + BPF_PROG_TYPE_CGROUP_SOCK_ADDR, + BPF_CGROUP_INET6_RECV_ACCEPT}, // sockops. - {"sockops", &EBPF_PROGRAM_TYPE_SOCK_OPS, &EBPF_ATTACH_TYPE_CGROUP_SOCK_OPS}, + {"sockops", + &EBPF_PROGRAM_TYPE_SOCK_OPS, + &EBPF_ATTACH_TYPE_CGROUP_SOCK_OPS, + BPF_PROG_TYPE_SOCK_OPS, + BPF_CGROUP_SOCK_OPS}, // Sample Extension. - {"sample_ext", &EBPF_PROGRAM_TYPE_SAMPLE, &EBPF_ATTACH_TYPE_SAMPLE}, + {"sample_ext", &EBPF_PROGRAM_TYPE_SAMPLE, &EBPF_ATTACH_TYPE_SAMPLE, BPF_PROG_TYPE_SAMPLE, BPF_ATTACH_TYPE_SAMPLE}, }; struct ebpf_attach_type_compare diff --git a/libs/ebpfnetsh/elf.cpp b/libs/ebpfnetsh/elf.cpp index 1498fdf8b..0277ee524 100644 --- a/libs/ebpfnetsh/elf.cpp +++ b/libs/ebpfnetsh/elf.cpp @@ -248,9 +248,39 @@ handle_ebpf_show_verification( const char* report; const char* error_message; ebpf_api_verifier_stats_t stats; + ebpf_program_type_t program_type; + ebpf_attach_type_t attach_type; + + if (section == "") { + // If no section name was provided, fetch the first section name. + ebpf_section_info_t* section_data = nullptr; + ebpf_result_t result = + ebpf_enumerate_sections(filename.c_str(), level == VL_VERBOSE, §ion_data, &error_message); + if (result != ERROR_SUCCESS || section_data == nullptr) { + if (error_message) { + std::cerr << error_message << std::endl; + } else { + std::cerr << "\nNo section(s) found" << std::endl; + } + ebpf_free_string(error_message); + ebpf_free_sections(section_data); + return ERROR_SUPPRESS_OUTPUT; + } + + section = section_data->section_name; + ebpf_free_string(error_message); + ebpf_free_sections(section_data); + } + + // TODO: Issue #1170. + // Workaround: Check the program type for the section name and default to XDP + // if getting program type fails. + if (ebpf_get_program_type_by_name(section.c_str(), &program_type, &attach_type) != EBPF_SUCCESS) { + program_type = EBPF_PROGRAM_TYPE_XDP; + } status = ebpf_api_elf_verify_section_from_file( - filename.c_str(), section.c_str(), level == VL_VERBOSE, &report, &error_message, &stats); + filename.c_str(), section.c_str(), &program_type, level == VL_VERBOSE, &report, &error_message, &stats); if (status == ERROR_SUCCESS) { std::cout << report; std::cout << "\nProgram terminates within " << stats.max_instruction_count << " instructions\n"; diff --git a/libs/service/api_service.cpp b/libs/service/api_service.cpp index 3a9623b2f..92af0286c 100644 --- a/libs/service/api_service.cpp +++ b/libs/service/api_service.cpp @@ -274,6 +274,7 @@ ebpf_verify_and_load_program( } // Verify the program. + set_verification_in_progress(true); result = verify_byte_code(program_type, instructions, instruction_count, error_message, error_message_size); if (result != EBPF_SUCCESS) { goto Exit; diff --git a/libs/service/verifier_service.cpp b/libs/service/verifier_service.cpp index 2e6a54f6b..174de4c56 100644 --- a/libs/service/verifier_service.cpp +++ b/libs/service/verifier_service.cpp @@ -49,12 +49,19 @@ verify_byte_code( _Outptr_result_maybenull_z_ const char** error_message, _Out_ uint32_t* error_message_size) { + std::ostringstream error; const ebpf_platform_t* platform = &g_ebpf_platform_windows_service; std::vector instructions{instruction_array, instruction_array + instruction_count}; program_info info{platform}; std::string section; std::string file; - info.type = get_program_type_windows(*program_type); + try { + info.type = get_program_type_windows(*program_type); + } catch (std::runtime_error e) { + error << "error: " << e.what(); + *error_message = allocate_string(error.str()); + return EBPF_VERIFICATION_FAILED; + } raw_program raw_prog{file, section, instructions, info}; diff --git a/tests/bpf2c_tests/expected/bpf_dll.c b/tests/bpf2c_tests/expected/bpf_dll.c index de54b1624..5ea51668a 100644 --- a/tests/bpf2c_tests/expected/bpf_dll.c +++ b/tests/bpf2c_tests/expected/bpf_dll.c @@ -58,7 +58,7 @@ _get_maps(_Outptr_result_buffer_maybenull_(*count) map_entry_t** maps, _Out_ siz } static GUID func_program_type_guid = {0xf1832a85, 0x85d5, 0x45b0, {0x98, 0xa0, 0x70, 0x69, 0xd6, 0x30, 0x13, 0xb0}}; -static GUID func_attach_type_guid = {0x00000000, 0x0000, 0x0000, {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}; +static GUID func_attach_type_guid = {0x85e0d8ef, 0x579e, 0x4931, {0xb0, 0x72, 0x8e, 0xe2, 0x26, 0xbb, 0x2e, 0x9d}}; #pragma code_seg(push, ".text") static uint64_t func(void* context) diff --git a/tests/bpf2c_tests/expected/bpf_raw.c b/tests/bpf2c_tests/expected/bpf_raw.c index 4e1a3bf50..606e556de 100644 --- a/tests/bpf2c_tests/expected/bpf_raw.c +++ b/tests/bpf2c_tests/expected/bpf_raw.c @@ -20,7 +20,7 @@ _get_maps(_Outptr_result_buffer_maybenull_(*count) map_entry_t** maps, _Out_ siz } static GUID func_program_type_guid = {0xf1832a85, 0x85d5, 0x45b0, {0x98, 0xa0, 0x70, 0x69, 0xd6, 0x30, 0x13, 0xb0}}; -static GUID func_attach_type_guid = {0x00000000, 0x0000, 0x0000, {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}; +static GUID func_attach_type_guid = {0x85e0d8ef, 0x579e, 0x4931, {0xb0, 0x72, 0x8e, 0xe2, 0x26, 0xbb, 0x2e, 0x9d}}; #pragma code_seg(push, ".text") static uint64_t func(void* context) diff --git a/tests/bpf2c_tests/expected/bpf_sys.c b/tests/bpf2c_tests/expected/bpf_sys.c index b18ea9961..8a2700395 100644 --- a/tests/bpf2c_tests/expected/bpf_sys.c +++ b/tests/bpf2c_tests/expected/bpf_sys.c @@ -187,7 +187,7 @@ _get_maps(_Outptr_result_buffer_maybenull_(*count) map_entry_t** maps, _Out_ siz } static GUID func_program_type_guid = {0xf1832a85, 0x85d5, 0x45b0, {0x98, 0xa0, 0x70, 0x69, 0xd6, 0x30, 0x13, 0xb0}}; -static GUID func_attach_type_guid = {0x00000000, 0x0000, 0x0000, {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}; +static GUID func_attach_type_guid = {0x85e0d8ef, 0x579e, 0x4931, {0xb0, 0x72, 0x8e, 0xe2, 0x26, 0xbb, 0x2e, 0x9d}}; #pragma code_seg(push, ".text") static uint64_t func(void* context) diff --git a/tests/bpf2c_tests/expected/cgroup_sock_addr_dll.c b/tests/bpf2c_tests/expected/cgroup_sock_addr_dll.c index 57e61ecbf..578de1be2 100644 --- a/tests/bpf2c_tests/expected/cgroup_sock_addr_dll.c +++ b/tests/bpf2c_tests/expected/cgroup_sock_addr_dll.c @@ -235,7 +235,7 @@ static helper_function_entry_t authorize_connect6_helpers[] = { static GUID authorize_connect6_program_type_guid = { 0x92ec8e39, 0xaeec, 0x11ec, {0x9a, 0x30, 0x18, 0x60, 0x24, 0x89, 0xbe, 0xee}}; static GUID authorize_connect6_attach_type_guid = { - 0x00000000, 0x0000, 0x0000, {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}; + 0xa82e37b2, 0xaee7, 0x11ec, {0x9a, 0x30, 0x18, 0x60, 0x24, 0x89, 0xbe, 0xee}}; static uint16_t authorize_connect6_maps[] = { 1, }; diff --git a/tests/bpf2c_tests/expected/cgroup_sock_addr_raw.c b/tests/bpf2c_tests/expected/cgroup_sock_addr_raw.c index 657c59b80..820b1bd5b 100644 --- a/tests/bpf2c_tests/expected/cgroup_sock_addr_raw.c +++ b/tests/bpf2c_tests/expected/cgroup_sock_addr_raw.c @@ -197,7 +197,7 @@ static helper_function_entry_t authorize_connect6_helpers[] = { static GUID authorize_connect6_program_type_guid = { 0x92ec8e39, 0xaeec, 0x11ec, {0x9a, 0x30, 0x18, 0x60, 0x24, 0x89, 0xbe, 0xee}}; static GUID authorize_connect6_attach_type_guid = { - 0x00000000, 0x0000, 0x0000, {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}; + 0xa82e37b2, 0xaee7, 0x11ec, {0x9a, 0x30, 0x18, 0x60, 0x24, 0x89, 0xbe, 0xee}}; static uint16_t authorize_connect6_maps[] = { 1, }; diff --git a/tests/bpf2c_tests/expected/cgroup_sock_addr_sys.c b/tests/bpf2c_tests/expected/cgroup_sock_addr_sys.c index 8b111b73b..80bf1044c 100644 --- a/tests/bpf2c_tests/expected/cgroup_sock_addr_sys.c +++ b/tests/bpf2c_tests/expected/cgroup_sock_addr_sys.c @@ -364,7 +364,7 @@ static helper_function_entry_t authorize_connect6_helpers[] = { static GUID authorize_connect6_program_type_guid = { 0x92ec8e39, 0xaeec, 0x11ec, {0x9a, 0x30, 0x18, 0x60, 0x24, 0x89, 0xbe, 0xee}}; static GUID authorize_connect6_attach_type_guid = { - 0x00000000, 0x0000, 0x0000, {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}; + 0xa82e37b2, 0xaee7, 0x11ec, {0x9a, 0x30, 0x18, 0x60, 0x24, 0x89, 0xbe, 0xee}}; static uint16_t authorize_connect6_maps[] = { 1, }; diff --git a/tests/cilium/cilium_tests.cpp b/tests/cilium/cilium_tests.cpp index 40b71776c..4a647f8b5 100644 --- a/tests/cilium/cilium_tests.cpp +++ b/tests/cilium/cilium_tests.cpp @@ -37,7 +37,8 @@ verify_program(_In_z_ const char* file, uint32_t expected_section_count) const char* log_buffer = nullptr; const char* report = nullptr; REQUIRE( - (result = ebpf_api_elf_verify_section_from_file(file, section_name, false, &report, &log_buffer, &stats), + (result = ebpf_api_elf_verify_section_from_file( + file, section_name, &EBPF_PROGRAM_TYPE_XDP, false, &report, &log_buffer, &stats), ebpf_free_string(log_buffer), log_buffer = nullptr, result == 0)); diff --git a/tests/end_to_end/end_to_end.cpp b/tests/end_to_end/end_to_end.cpp index 0690d712c..38053f65d 100644 --- a/tests/end_to_end/end_to_end.cpp +++ b/tests/end_to_end/end_to_end.cpp @@ -10,6 +10,7 @@ #include #include // Must come after Winsock2.h +#include "api_common.hpp" #include "bpf2c.h" #include "bpf/bpf.h" #include "bpf/libbpf.h" @@ -836,7 +837,7 @@ map_test(ebpf_execution_type_t execution_type) const char* file_name = (execution_type == EBPF_EXECUTION_NATIVE ? "map_um.dll" : "map.o"); - result = ebpf_program_load(file_name, BPF_PROG_TYPE_UNSPEC, execution_type, &object, &program_fd, &error_message); + result = ebpf_program_load(file_name, BPF_PROG_TYPE_XDP, execution_type, &object, &program_fd, &error_message); if (error_message) { printf("ebpf_program_load failed with %s\n", error_message); @@ -903,7 +904,7 @@ TEST_CASE("verify section", "[end_to_end]") ebpf_api_verifier_stats_t stats; REQUIRE( (result = ebpf_api_elf_verify_section_from_file( - SAMPLE_PATH "droppacket.o", "xdp", false, &report, &error_message, &stats), + SAMPLE_PATH "droppacket.o", "xdp", nullptr, false, &report, &error_message, &stats), ebpf_free_string(error_message), error_message = nullptr, result == 0)); @@ -911,6 +912,24 @@ TEST_CASE("verify section", "[end_to_end]") ebpf_free_string(report); } +TEST_CASE("verify section with invalid program type", "[end_to_end]") +{ + _test_helper_end_to_end test_helper; + + const char* error_message = nullptr; + const char* report = nullptr; + uint32_t result; + program_info_provider_t xdp_program_info(EBPF_PROGRAM_TYPE_BIND); + + ebpf_api_verifier_stats_t stats; + result = ebpf_api_elf_verify_section_from_file( + SAMPLE_PATH "droppacket.o", "xdp", &EBPF_PROGRAM_TYPE_UNSPECIFIED, false, &report, &error_message, &stats); + + REQUIRE(result == 1); + REQUIRE(error_message != nullptr); + ebpf_free_string(error_message); +} + void verify_bad_section(const char* path, const std::string& expected_error_message) { @@ -920,7 +939,7 @@ verify_bad_section(const char* path, const std::string& expected_error_message) uint32_t result; program_info_provider_t xdp_program_info(EBPF_PROGRAM_TYPE_XDP); ebpf_api_verifier_stats_t stats; - result = ebpf_api_elf_verify_section_from_file(path, "xdp", false, &report, &error_message, &stats); + result = ebpf_api_elf_verify_section_from_file(path, "xdp", nullptr, false, &report, &error_message, &stats); REQUIRE(result != 0); REQUIRE(report == nullptr); REQUIRE(std::string(error_message) == expected_error_message); @@ -1029,7 +1048,7 @@ TEST_CASE("verify_test0", "[sample_extension]") ebpf_api_verifier_stats_t stats; REQUIRE( (result = ebpf_api_elf_verify_section_from_file( - SAMPLE_PATH "test_sample_ebpf.o", "sample_ext", false, &report, &error_message, &stats), + SAMPLE_PATH "test_sample_ebpf.o", "sample_ext", nullptr, false, &report, &error_message, &stats), ebpf_free_string(error_message), error_message = nullptr, result == 0)); @@ -1050,7 +1069,7 @@ TEST_CASE("verify_test1", "[sample_extension]") REQUIRE( (result = ebpf_api_elf_verify_section_from_file( - SAMPLE_PATH "test_sample_ebpf.o", "sample_ext/utility", false, &report, &error_message, &stats), + SAMPLE_PATH "test_sample_ebpf.o", "sample_ext/utility", nullptr, false, &report, &error_message, &stats), ebpf_free_string(error_message), error_message = nullptr, result == 0)); @@ -2331,3 +2350,33 @@ TEST_CASE("load_native_program_invalid4", "[end-to-end]") _load_invalid_program("empty_um.dll", EBPF_EXECUTION_NATIVE, -EINVAL); } #endif + +TEST_CASE("ebpf_get_program_type_by_name invalid name", "[end-to-end]") +{ + _test_helper_end_to_end test_helper; + ebpf_program_type_t program_type; + ebpf_attach_type_t attach_type; + + ebpf_result_t result = ebpf_get_program_type_by_name("invalid_name", &program_type, &attach_type); + REQUIRE(result == EBPF_KEY_NOT_FOUND); + + // Now set verification in progress and try again. + set_verification_in_progress(true); + result = ebpf_get_program_type_by_name("invalid_name", &program_type, &attach_type); + REQUIRE(result == EBPF_KEY_NOT_FOUND); +} + +TEST_CASE("ebpf_get_program_type_name invalid types", "[end-to-end]") +{ + _test_helper_end_to_end test_helper; + ebpf_program_type_t program_type = EBPF_PROGRAM_TYPE_UNSPECIFIED; + + // First try with EBPF_PROGRAM_TYPE_UNSPECIFIED. + const char* name1 = ebpf_get_program_type_name(&program_type); + REQUIRE(name1 == nullptr); + + // Try with a random program type GUID. + REQUIRE(UuidCreate(&program_type) == RPC_S_OK); + const char* name2 = ebpf_get_program_type_name(&program_type); + REQUIRE(name2 == nullptr); +} diff --git a/tests/end_to_end/netsh_test.cpp b/tests/end_to_end/netsh_test.cpp index 02ecb0aa3..1867d7dd4 100644 --- a/tests/end_to_end/netsh_test.cpp +++ b/tests/end_to_end/netsh_test.cpp @@ -97,7 +97,7 @@ TEST_CASE("show sections bpf.o", "[netsh][sections]") " Size\n" " Section Type (bytes)\n" "==================== ========= =======\n" - " .text xdp 16\n" + " .text unspec 16\n" "\n" " Key Value Max\n" " Map Type Size Size Entries Name\n" @@ -113,7 +113,7 @@ TEST_CASE("show sections bpf.o .text", "[netsh][sections]") REQUIRE( output == "\n" "Section : .text\n" - "Program Type : xdp\n" + "Program Type : unspec\n" "Size : 16 bytes\n" "Instructions : 2\n" "adjust_head : 0\n" diff --git a/tests/end_to_end/test_helper.cpp b/tests/end_to_end/test_helper.cpp index 852469db3..4bb0ff17e 100644 --- a/tests/end_to_end/test_helper.cpp +++ b/tests/end_to_end/test_helper.cpp @@ -8,6 +8,7 @@ using namespace std::chrono_literals; #include "bpf/bpf.h" #include "catch_wrapper.hpp" +#include "api_common.hpp" #include "api_internal.h" #include "bpf2c.h" #include "ebpf_async.h" @@ -566,6 +567,8 @@ _test_helper_end_to_end::~_test_helper_end_to_end() // Change back to original value. _ebpf_platform_is_preemptible = true; + + set_verification_in_progress(false); } _test_helper_libbpf::_test_helper_libbpf() diff --git a/tests/libfuzzer/verifier/libfuzz_harness.cpp b/tests/libfuzzer/verifier/libfuzz_harness.cpp index 3a36404e4..1567cfb26 100644 --- a/tests/libfuzzer/verifier/libfuzz_harness.cpp +++ b/tests/libfuzzer/verifier/libfuzz_harness.cpp @@ -14,7 +14,7 @@ FUZZ_EXPORT int __cdecl LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) const char* report = nullptr; const char* error_message = nullptr; ebpf_api_elf_verify_section_from_memory( - reinterpret_cast(data), size, "", false, &report, &error_message, nullptr); + reinterpret_cast(data), size, "", nullptr, false, &report, &error_message, nullptr); free(const_cast(report)); free(const_cast(error_message)); } catch (std::runtime_error&) { diff --git a/tests/unit/libbpf_test.cpp b/tests/unit/libbpf_test.cpp index 6e4ae6e0d..23cc79278 100644 --- a/tests/unit/libbpf_test.cpp +++ b/tests/unit/libbpf_test.cpp @@ -1919,10 +1919,9 @@ TEST_CASE("libbpf_prog_type_by_name", "[libbpf]") REQUIRE(prog_type == BPF_PROG_TYPE_BIND); REQUIRE(expected_attach_type == BPF_ATTACH_TYPE_UNSPEC); - // Try something that will fall back to the default. - REQUIRE(libbpf_prog_type_by_name("default", &prog_type, &expected_attach_type) == 0); - REQUIRE(prog_type == BPF_PROG_TYPE_XDP); - REQUIRE(expected_attach_type == BPF_XDP); + // Try a random name. This should fail. + REQUIRE(libbpf_prog_type_by_name("default", &prog_type, &expected_attach_type) == -1); + REQUIRE(errno == ESRCH); } TEST_CASE("libbpf_get_error", "[libbpf]") diff --git a/tools/bpf2c/bpf2c.cpp b/tools/bpf2c/bpf2c.cpp index e3cf5a099..45e524af7 100644 --- a/tools/bpf2c/bpf2c.cpp +++ b/tools/bpf2c/bpf2c.cpp @@ -241,15 +241,25 @@ main(int argc, char** argv) for (const auto& section : sections) { ebpf_program_type_t program_type; ebpf_attach_type_t attach_type; + // TODO: Issue #1172 + // Workaround: If querying the program and attach type fails, default it to XDP until Issue #1172 + // is fixed. if (ebpf_get_program_type_by_name(section.c_str(), &program_type, &attach_type) != EBPF_SUCCESS) { - throw std::runtime_error(std::string("Cannot get program / attach type for section ") + section); + program_type = EBPF_PROGRAM_TYPE_XDP; + attach_type = EBPF_ATTACH_TYPE_XDP; } const char* report = nullptr; const char* error_message = nullptr; ebpf_api_verifier_stats_t stats; - if (verify_programs && - ebpf_api_elf_verify_section_from_memory( - data.c_str(), data.size(), section.c_str(), false, &report, &error_message, &stats) != 0) { + if (verify_programs && ebpf_api_elf_verify_section_from_memory( + data.c_str(), + data.size(), + section.c_str(), + &program_type, + false, + &report, + &error_message, + &stats) != 0) { report = ((report == nullptr) ? "" : report); throw std::runtime_error( std::string("Verification failed for ") + section + std::string(" with error ") + diff --git a/tools/encode_program_info/encode_program_info.cpp b/tools/encode_program_info/encode_program_info.cpp index 0a896bf5a..244cac820 100644 --- a/tools/encode_program_info/encode_program_info.cpp +++ b/tools/encode_program_info/encode_program_info.cpp @@ -78,7 +78,8 @@ int main() { for (const auto& program_type : windows_program_types) { - if (program_type.platform_specific_data == 0) { + if (IsEqualGUID( + *(reinterpret_cast(program_type.platform_specific_data)), EBPF_PROGRAM_TYPE_UNSPECIFIED)) { continue; } _encode_program_info(program_type);