From c97b2a7782e9f450376c84ca175fa3d0766f8753 Mon Sep 17 00:00:00 2001 From: Dave Thaler Date: Mon, 15 Aug 2022 09:30:11 -0700 Subject: [PATCH] Fix handling of long map names (#1328) Fixes #1312 Signed-off-by: Dave Thaler Signed-off-by: Dave Thaler --- libs/api/ebpf_api.cpp | 22 ++++++++++++++-------- tests/end_to_end/end_to_end.cpp | 27 +++++++++++++++++++++++++++ tests/sample/bad_map_name.c | 26 ++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 tests/sample/bad_map_name.c diff --git a/libs/api/ebpf_api.cpp b/libs/api/ebpf_api.cpp index 5c4d96d0e..56b71c544 100644 --- a/libs/api/ebpf_api.cpp +++ b/libs/api/ebpf_api.cpp @@ -1848,13 +1848,13 @@ typedef struct _ebpf_pe_context const bounded_buffer* data_buffer; } ebpf_pe_context_t; -static int +static int // Returns 0 on success, 1 on error. _ebpf_pe_get_map_definitions( - void* context, - const VA& va, - const std::string& section_name, - const image_section_header& section_header, - const bounded_buffer* buffer) noexcept + _Inout_ void* context, + _In_ const VA& va, + _In_ const std::string& section_name, + _In_ const image_section_header& section_header, + _In_ const bounded_buffer* buffer) noexcept { EBPF_LOG_ENTRY(); UNREFERENCED_PARAMETER(va); @@ -1863,8 +1863,8 @@ _ebpf_pe_get_map_definitions( ebpf_map_t* map = nullptr; ebpf_pe_context_t* pe_context = (ebpf_pe_context_t*)context; if (section_name == "maps") { - // bpf2c generates a section that has map names as strings at the - // start of the section. Skip over them looking for the map_entry_t + // bpf2c generates a section that has map names shorter than sizeof(map_entry_t) + // at the start of the section. Skip over them looking for the map_entry_t // which starts with an 8-byte-aligned NULL pointer where the previous // byte (if any) is also 00, and the following 8 bytes are non-NULL. uint32_t map_offset = 0; @@ -1879,6 +1879,12 @@ _ebpf_pe_get_map_definitions( for (int map_index = 0; map_offset + sizeof(map_entry_t) <= section_header.Misc.VirtualSize; map_offset += sizeof(map_entry_t), map_index++) { map_entry_t* entry = (map_entry_t*)(buffer->buf + map_offset); + if (entry->address != nullptr) { + // bpf2c generates a section that has map names longer than sizeof(map_entry_t) + // at the end of the section. This entry seems to be a map name string, so we've + // reached the end of the maps. + break; + } map = (ebpf_map_t*)calloc(1, sizeof(ebpf_map_t)); if (map == nullptr) { diff --git a/tests/end_to_end/end_to_end.cpp b/tests/end_to_end/end_to_end.cpp index 82b9644b0..1bd51ef99 100644 --- a/tests/end_to_end/end_to_end.cpp +++ b/tests/end_to_end/end_to_end.cpp @@ -492,6 +492,32 @@ divide_by_zero_test_um(ebpf_execution_type_t execution_type) bpf_object__close(object); } +void +bad_map_name_um(ebpf_execution_type_t execution_type) +{ + _test_helper_end_to_end test_helper; + + int result; + const char* error_message = nullptr; + bpf_object* object = nullptr; + fd_t program_fd; + + single_instance_hook_t hook(EBPF_PROGRAM_TYPE_XDP, EBPF_ATTACH_TYPE_XDP); + program_info_provider_t xdp_program_info(EBPF_PROGRAM_TYPE_XDP); + + const char* file_name = (execution_type == EBPF_EXECUTION_NATIVE ? "bad_map_name_um.dll" : "bad_map_name.o"); + + result = ebpf_program_load(file_name, BPF_PROG_TYPE_UNSPEC, execution_type, &object, &program_fd, &error_message); + + if (error_message) { + printf("ebpf_program_load failed with %s\n", error_message); + free((void*)error_message); + } + REQUIRE(result == -EINVAL); + + bpf_object__close(object); +} + typedef struct _process_entry { uint32_t count; @@ -873,6 +899,7 @@ DECLARE_ALL_TEST_CASES("bindmonitor-tailcall", "[end_to_end]", bindmonitor_tailc DECLARE_ALL_TEST_CASES("bindmonitor-ringbuf", "[end_to_end]", bindmonitor_ring_buffer_test); DECLARE_ALL_TEST_CASES("utility-helpers", "[end_to_end]", _utility_helper_functions_test); DECLARE_ALL_TEST_CASES("map", "[end_to_end]", map_test); +DECLARE_ALL_TEST_CASES("bad_map_name", "[end_to_end]", bad_map_name_um); TEST_CASE("enum section", "[end_to_end]") { diff --git a/tests/sample/bad_map_name.c b/tests/sample/bad_map_name.c new file mode 100644 index 000000000..94515511b --- /dev/null +++ b/tests/sample/bad_map_name.c @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation +// SPDX-License-Identifier: MIT + +// Whenever this sample program changes, bpf2c_tests will fail unless the +// expected files in tests\bpf2c_tests\expected are updated. The following +// script can be used to regenerate the expected files: +// generate_expected_bpf2c_output.ps1 +// +// Usage: +// .\scripts\generate_expected_bpf2c_output.ps1 +// Example: +// .\scripts\generate_expected_bpf2c_output.ps1 .\x64\Debug\ + +#include "bpf_helpers.h" + +SEC("maps") +struct bpf_map_def this_map_has_a_name_that_is_longer_than_what_the_ebofcore_driver_can_support = { + .type = BPF_MAP_TYPE_HASH, .key_size = sizeof(uint32_t), .value_size = sizeof(uint32_t), .max_entries = 1}; + +SEC("xdp_prog") int lookup(struct xdp_md* ctx) +{ + uint32_t key = 0; + void* value = + bpf_map_lookup_elem(&this_map_has_a_name_that_is_longer_than_what_the_ebofcore_driver_can_support, &key); + return (value == NULL); +}