From 2c6d842fdacdc346055dc25de908e3245cce75b9 Mon Sep 17 00:00:00 2001 From: Dave Thaler Date: Mon, 10 Mar 2025 09:16:32 -0700 Subject: [PATCH 1/6] Canonicalize pin paths using filesystem canonicalization rules Signed-off-by: Dave Thaler --- docs/PinPaths.md | 98 ++++++++++++++++++++++++++++ include/ebpf_api.h | 2 +- libs/api/ebpf_api.cpp | 33 ++++++++-- libs/execution_context/ebpf_native.c | 21 +++--- libs/shared/ebpf_shared_framework.h | 13 ++++ libs/shared/shared_common.c | 77 ++++++++++++++++++++++ tests/end_to_end/end_to_end.cpp | 98 ++++++++++++++++++++++++++-- tests/end_to_end/netsh_test.cpp | 36 +++++----- tests/libs/common/common_tests.cpp | 10 ++- 9 files changed, 339 insertions(+), 49 deletions(-) create mode 100644 docs/PinPaths.md diff --git a/docs/PinPaths.md b/docs/PinPaths.md new file mode 100644 index 0000000000..43fb3fabca --- /dev/null +++ b/docs/PinPaths.md @@ -0,0 +1,98 @@ +# Pin path design + +On Linux, objects can be pinned to a filesystem path. As a result: +* Pins can be enumerated or removed via normal filesystem APIs/utilities +* Pin paths are case-sensitive +* The path component separator is a forward slash ('/') +* Pin paths begin with "/sys/fs/bpf/" and relative paths like "mymap" are relative to that prefix. +* ".." and "." components are resolved. + +On Windows, pin paths are not currently part of the filesystem. As a result, +pins cannot be removed via normal filesystem deletion APIs/utilities and instead +ebpf_get_next_pinned_object_path() and ebpf_object_unpin() are exposed +for enumeration and unpinning, respectively. + +This leaves open the question of what syntax(es) we support on Windows for pin paths. + +## Criteria for Evaluation + +The following are criteria for evaluating each option: + +1. Slashes: Paths must be accepted using either '/' or '\\' and be canonicalized. + +1. Kernel: Canonicalization must be the same in kernel-mode (for native) and user-mode (for JIT). + +1. LocalFile: A pin path should not collide with the path of another file in the Windows filesystem. + +1. RemoteFile: Avoid confusion with remote paths. + +1. FileAPIs: Paths should work with Windows file APIs in the future once a BPF filesystem is implemented. + +1. CaseSensitive: Paths should be case-sensitive. + +## Options + +### Native (Actual file system path) + +Example: C:\ebpf\global\my\pin\path + +In this option, querying the path of a pinned object would give an actual file system path. +This might be confusing if the path conflicts with an actual path where there may be files, +so additional care would be needed to fail such a pin. + +Furthermore, an actual file system path would be case-insensitive rather than case-sensitive. +Another implementation challenge is that the logic for forming pin paths for native programs +is in the kernel (to minimize security concerns), and there isn't an easy way to get the +system drive, and picking "C:" would be fairly arbitrary. + +### Mnt (Actual file system path in Linux-style syntax) + +Example: /mnt/c/ebpf/global/my/pin/path + +Like the previous option, the current or system drive letter is still needed, so this has +the same issues as the previous option. + +### Virtual (Windows file system path with another virtual drive for BPF) + +Example: BPF:\my\pin\path + +In this option, there would be no collisions with actual files, and the path can be case +sensitive if the "BPF:" driver declares such. Similarly, ".." resolution would work if +the driver implements it as such. + +As for portability, canonicalization could ensure that Linux style paths like +"/sys/fs/bpf/my/pin/path" would be canonicalized to "BPF:\my\pin\path" allowing +portability in that respect. But querying the pin path on an object + + +### Linux (Linux-like file system path) + +Example: /sys/fs/bpf/my/pin/path + +In this option, the path would look the same as on Linux where +the canonical form uses forward slash rather than backslash. +It cannot be used with other Windows file system APIs in the future. + +### UNC (UNC path) + +Example: \\BPF\my\pin\path + +In this option, "BPF" would be a special "host" like "localhost" or "?". +However, it may be confusing if there is another host with the name "bpf", +or at least lead people to believe there might be. + +## Evaluation Matrix + +The matrix below shows how each option evaluates using the +criteria discussed above. "Y" means the option is good, +"N" means the option is bad, and "-" means it would be somewhere +in between good and bad. + +| Criteria | Native | Mnt | Virtual | Linux | UNC | +| ------------- | ------ | --- | ------- | ----- | --- | +| Slashes | Y | Y | Y | Y | Y | +| Kernel | - | - | Y | Y | Y | +| LocalFile | - | - | Y | Y | Y | +| RemoteFile | Y | Y | Y | Y | N | +| FileAPIs | Y | N | Y | N | Y | +| CaseSensitive | N | N | Y | Y | Y | diff --git a/include/ebpf_api.h b/include/ebpf_api.h index 3316248ada..943e3426f7 100644 --- a/include/ebpf_api.h +++ b/include/ebpf_api.h @@ -603,7 +603,7 @@ extern "C" /** * @brief Retrieve the next pinned path of an eBPF object. * - * @param[in] start_path Path to look for an entry greater than or NULL. + * @param[in] start_path Path to look for an entry greater than. * @param[out] next_path Returns the next path in lexicographical order, if one exists. * @param[in] next_path_len Length of the next path buffer. * @param[in, out] type On input, the type of object to retrieve or EBPF_OBJECT_UNKNOWN. diff --git a/libs/api/ebpf_api.cpp b/libs/api/ebpf_api.cpp index 1fff58a07f..7b7f82877d 100644 --- a/libs/api/ebpf_api.cpp +++ b/libs/api/ebpf_api.cpp @@ -1273,6 +1273,12 @@ ebpf_object_pin(fd_t fd, _In_z_ const char* path) NO_EXCEPT_TRY ebpf_handle_t handle; ebpf_assert(path); + char canonical_path[MAX_PATH]; + uint32_t return_value = ebpf_canonicalize_path(canonical_path, sizeof(canonical_path), path); + if (return_value != ERROR_SUCCESS) { + EBPF_RETURN_RESULT(EBPF_INVALID_ARGUMENT); + } + if (fd <= 0) { EBPF_RETURN_RESULT(EBPF_INVALID_ARGUMENT); } @@ -1282,14 +1288,14 @@ ebpf_object_pin(fd_t fd, _In_z_ const char* path) NO_EXCEPT_TRY EBPF_RETURN_RESULT(EBPF_INVALID_FD); } - auto path_length = strlen(path); + auto path_length = strlen(canonical_path); ebpf_protocol_buffer_t request_buffer(offsetof(ebpf_operation_update_pinning_request_t, path) + path_length); auto request = reinterpret_cast(request_buffer.data()); request->header.id = ebpf_operation_id_t::EBPF_OPERATION_UPDATE_PINNING; request->header.length = static_cast(request_buffer.size()); request->handle = handle; - std::copy(path, path + path_length, request->path); + std::copy(canonical_path, canonical_path + path_length, request->path); result = win32_error_code_to_ebpf_result(invoke_ioctl(request_buffer)); EBPF_RETURN_RESULT(result); @@ -1301,14 +1307,21 @@ ebpf_object_unpin(_In_z_ const char* path) NO_EXCEPT_TRY { EBPF_LOG_ENTRY(); ebpf_assert(path); - auto path_length = strlen(path); + char canonical_path[MAX_PATH]; + uint32_t return_value = ebpf_canonicalize_path(canonical_path, sizeof(canonical_path), path); + if (return_value != ERROR_SUCCESS) { + ebpf_result_t result = win32_error_code_to_ebpf_result(return_value); + EBPF_RETURN_RESULT(result); + } + + auto path_length = strlen(canonical_path); ebpf_protocol_buffer_t request_buffer(offsetof(ebpf_operation_update_pinning_request_t, path) + path_length); auto request = reinterpret_cast(request_buffer.data()); request->header.id = ebpf_operation_id_t::EBPF_OPERATION_UPDATE_PINNING; request->header.length = static_cast(request_buffer.size()); request->handle = UINT64_MAX; - std::copy(path, path + path_length, request->path); + std::copy(canonical_path, canonical_path + path_length, request->path); EBPF_RETURN_RESULT(win32_error_code_to_ebpf_result(invoke_ioctl(request_buffer))); } CATCH_NO_MEMORY_EBPF_RESULT @@ -1401,16 +1414,22 @@ ebpf_object_get(_In_z_ const char* path, _Out_ fd_t* fd) NO_EXCEPT_TRY { EBPF_LOG_ENTRY(); ebpf_assert(fd); + ebpf_assert(path); - size_t path_length = strlen(path); + char canonical_path[MAX_PATH]; + uint32_t return_value = ebpf_canonicalize_path(canonical_path, sizeof(canonical_path), path); + if (return_value != ERROR_SUCCESS) { + EBPF_RETURN_RESULT(EBPF_INVALID_ARGUMENT); + } + + size_t path_length = strlen(canonical_path); ebpf_protocol_buffer_t request_buffer(offsetof(ebpf_operation_get_pinned_object_request_t, path) + path_length); auto request = reinterpret_cast(request_buffer.data()); ebpf_operation_get_pinned_object_reply_t reply; - ebpf_assert(path); request->header.id = ebpf_operation_id_t::EBPF_OPERATION_GET_PINNED_OBJECT; request->header.length = static_cast(request_buffer.size()); - std::copy(path, path + path_length, request->path); + std::copy(canonical_path, canonical_path + path_length, request->path); auto result = invoke_ioctl(request_buffer, reply); if (result != ERROR_SUCCESS) { *fd = (fd_t)ebpf_fd_invalid; diff --git a/libs/execution_context/ebpf_native.c b/libs/execution_context/ebpf_native.c index 58eb7c42d7..bb32313c63 100644 --- a/libs/execution_context/ebpf_native.c +++ b/libs/execution_context/ebpf_native.c @@ -14,7 +14,6 @@ #include -#define DEFAULT_PIN_ROOT_PATH "/ebpf/global" #define EBPF_MAX_PIN_PATH_LENGTH 256 static const uint32_t _ebpf_native_marker = 'entv'; @@ -1052,29 +1051,25 @@ _ebpf_native_initialize_maps( if (entry->definition.pinning == LIBBPF_PIN_BY_NAME) { // Construct the pin path. - size_t prefix_length = strnlen(DEFAULT_PIN_ROOT_PATH, EBPF_MAX_PIN_PATH_LENGTH); - size_t name_length = strnlen_s(entry->name, BPF_OBJ_NAME_LEN); - if (name_length == 0 || name_length >= BPF_OBJ_NAME_LEN || - prefix_length + name_length + 1 >= EBPF_MAX_PIN_PATH_LENGTH) { + char canonical_path[EBPF_MAX_PIN_PATH_LENGTH]; + result = ebpf_canonicalize_path(canonical_path, sizeof(canonical_path), entry->name); + if (result != EBPF_SUCCESS) { EBPF_LOG_MESSAGE_GUID( EBPF_TRACELOG_LEVEL_ERROR, EBPF_TRACELOG_KEYWORD_NATIVE, - "_ebpf_native_initialize_maps: map pin path too long", + "_ebpf_native_initialize_maps: invalid map pin path", module_id); - result = EBPF_INVALID_ARGUMENT; goto Done; } - native_maps[i].pin_path.value = - ebpf_allocate_with_tag(prefix_length + name_length + 1, EBPF_POOL_TAG_NATIVE); + size_t length = strlen(canonical_path); + native_maps[i].pin_path.value = ebpf_allocate_with_tag(length, EBPF_POOL_TAG_NATIVE); if (native_maps[i].pin_path.value == NULL) { result = EBPF_NO_MEMORY; goto Done; } - native_maps[i].pin_path.length = prefix_length + name_length + 1; - memcpy(native_maps[i].pin_path.value, DEFAULT_PIN_ROOT_PATH, prefix_length); - memcpy(native_maps[i].pin_path.value + prefix_length, "/", 1); - memcpy(native_maps[i].pin_path.value + prefix_length + 1, entry->name, name_length); + native_maps[i].pin_path.length = length; + memcpy(native_maps[i].pin_path.value, canonical_path, length); } else { native_maps[i].pin_path.value = NULL; native_maps[i].pin_path.length = 0; diff --git a/libs/shared/ebpf_shared_framework.h b/libs/shared/ebpf_shared_framework.h index 21960256e7..d2ddd1f04f 100644 --- a/libs/shared/ebpf_shared_framework.h +++ b/libs/shared/ebpf_shared_framework.h @@ -219,4 +219,17 @@ ebpf_result_t ebpf_duplicate_program_data( _In_ const ebpf_program_data_t* program_data, _Outptr_ ebpf_program_data_t** new_program_data); +/** + * @brief Canonicalize a path using filesystem canonicalization rules. + * + * @param[out] output Buffer in which to write canonicalized path. + * @param[in] output_size Size of output buffer. + * @param[out] error_code Zero on success, non-zero Win32 error code on failure. + * + * @retval EBPF_SUCCESS The operation was successful. + * @retval EBPF_INVALID_ARGUMENT The input path was invalid. + */ +ebpf_result_t +ebpf_canonicalize_path(_Out_writes_(output_size) char* output, size_t output_size, _In_z_ const char* input); + CXPLAT_EXTERN_C_END diff --git a/libs/shared/shared_common.c b/libs/shared/shared_common.c index 24ed2baa8f..fa845b3dc9 100644 --- a/libs/shared/shared_common.c +++ b/libs/shared/shared_common.c @@ -648,3 +648,80 @@ ebpf_duplicate_program_data( EBPF_RETURN_RESULT(result); } + +ebpf_result_t +ebpf_canonicalize_path(_Out_writes_(output_size) char* output, size_t output_size, _In_z_ const char* input) +{ + const int PREFIX_SIZE = 5; // Length of "BPF:\\". + strcpy_s(output, output_size, "BPF:\\"); + + // Add the BPF: prefix if not already present. + if (_strnicmp(input, "BPF:\\", PREFIX_SIZE) == 0 || _strnicmp(input, "BPF:/", PREFIX_SIZE) == 0) { + strcpy_s(output + PREFIX_SIZE, output_size - PREFIX_SIZE, input + PREFIX_SIZE); + } else { + strcpy_s(output + PREFIX_SIZE, output_size - PREFIX_SIZE, input); + } + + for (int i = PREFIX_SIZE; output[i] != 0; i++) { + if (output[i] == '/') { + // Convert slashes to backslashes. + output[i] = '\\'; + } else if (output[i] == ':') { + // Disallow ':' in mid-path. + return EBPF_INVALID_ARGUMENT; + } + } + + // Remove other prefix aliases from the beginning. + if (_strnicmp(output + PREFIX_SIZE, "\\ebpf\\global\\", 13) == 0) { + char* next = output + PREFIX_SIZE + 13; + memmove(output + PREFIX_SIZE, next, strlen(next) + 1); + } else if (strncmp(output + PREFIX_SIZE, "\\sys\\fs\\bpf\\", 12) == 0) { + char* next = output + PREFIX_SIZE + 12; + memmove(output + PREFIX_SIZE, next, strlen(next) + 1); + } + + for (int i = PREFIX_SIZE - 1; output[i] != 0;) { + if (strncmp(output + i, "\\\\", 2) == 0) { + char* next = output + i + 2; + memmove(output + i + 1, next, strlen(next) + 1); + + // Stay at this position for the next loop iteration. + } else if (strncmp(output + i, "\\.\\", 3) == 0) { + char* next = output + i + 3; + memmove(output + i + 1, next, strlen(next) + 1); + } else if (strncmp(output + i, "\\..\\", 4) == 0) { + int previous_index = i - 1; + for (; previous_index >= 4; previous_index--) { + if (output[previous_index] == '\\') { + // Back up to the previous separator. + char* next = output + i + 4; + memmove(output + previous_index + 1, next, strlen(next) + 1); + i = previous_index; + break; + } + } + if (previous_index < PREFIX_SIZE - 1) { + return EBPF_INVALID_ARGUMENT; + } + } else if (strcmp(output + i, "\\..") == 0) { + int previous_index = i - 1; + for (; previous_index >= PREFIX_SIZE - 1; previous_index--) { + if (output[previous_index] == '\\') { + // Terminate the string at the previous separator. + output[previous_index] = 0; + i = previous_index; + break; + } + } + if (previous_index < PREFIX_SIZE - 1) { + return EBPF_INVALID_ARGUMENT; + } + } else { + // Advance to the next character in the path. + i++; + } + } + + return EBPF_SUCCESS; +} diff --git a/tests/end_to_end/end_to_end.cpp b/tests/end_to_end/end_to_end.cpp index 62dba2b44a..f4f9e01812 100644 --- a/tests/end_to_end/end_to_end.cpp +++ b/tests/end_to_end/end_to_end.cpp @@ -1546,8 +1546,8 @@ TEST_CASE("map_pinning_test", "[end_to_end]") single_instance_hook_t hook(EBPF_PROGRAM_TYPE_BIND, EBPF_ATTACH_TYPE_BIND); REQUIRE(hook.initialize() == EBPF_SUCCESS); - std::string process_maps_name = "bindmonitor::process_map"; - std::string limit_maps_name = "bindmonitor::limits_map"; + std::string process_maps_name = "bindmonitor/process_map"; + std::string limit_maps_name = "bindmonitor/limits_map"; REQUIRE(bpf_object__find_map_by_name(unique_object.get(), "process_map") != nullptr); REQUIRE(bpf_object__find_map_by_name(unique_object.get(), "limits_map") != nullptr); @@ -1673,6 +1673,88 @@ TEST_CASE("pinned_map_enum", "[end_to_end]") ebpf_test_pinned_map_enum(); } +static void +_verify_canonical_path(int fd, _In_z_ const char* original_path, _In_z_ const char* canonical_path) +{ + // Pin the fd to the original path. + REQUIRE(ebpf_object_pin(fd, original_path) == EBPF_SUCCESS); + + // Look up id for the fd. + bpf_prog_info info = {}; + uint32_t info_size = sizeof(info); + int result = bpf_obj_get_info_by_fd(fd, &info, &info_size); + REQUIRE(result == 0); + ebpf_id_t id = info.id; + + // TODO(#4273): Verify it has exactly one path pinned. + // REQUIRE(info.pinned_path_count == 1); + + // Look up the actual path pinned. + ebpf_object_type_t object_type = EBPF_OBJECT_UNKNOWN; + char path[EBPF_MAX_PIN_PATH_LENGTH] = ""; + while (ebpf_get_next_pinned_object_path(path, path, sizeof(path), &object_type) == EBPF_SUCCESS) { + int fd2 = bpf_obj_get(path); + if (fd2 < 0) { + continue; + } + if ((bpf_obj_get_info_by_fd(fd2, &info, &info_size) == 0) && (info.id == id)) { + // Verify the path is what we expect. + REQUIRE(strcmp(path, canonical_path) == 0); + } + Platform::_close(fd2); + } + + // Verify we can unpin it by the original path. + REQUIRE(ebpf_object_unpin(original_path) == EBPF_SUCCESS); +} + +TEST_CASE("pin path canonicalization", "[end_to_end][pinning]") +{ + _test_helper_end_to_end test_helper; + test_helper.initialize(); + + fd_t map_fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, "test_map", sizeof(uint32_t), sizeof(uint32_t), 1, nullptr); + REQUIRE(map_fd > 0); + + // Verify pin path canonicalization. + _verify_canonical_path(map_fd, "BPF:\\.\\my\\pin\\path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "BPF:/./my/pin/path", "BPF:\\my\\pin\\path"); + + // Try Linux-style absolute paths. + _verify_canonical_path(map_fd, "/sys/fs/bpf/my/pin/path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "/sys/fs/bpf/My/Pin/Path", "BPF:\\My\\Pin\\Path"); + + // Try Linux-style relative paths. + _verify_canonical_path(map_fd, "my/pin/path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "my/pin/./path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "my/pin//path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "my/pin/./././path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "my/pin/oops/../path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "my/pin/oops/again/../../path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "My/Pin/Path", "BPF:\\My\\Pin\\Path"); + + // Try Windows-style relative paths. + _verify_canonical_path(map_fd, "my\\pin\\path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "my\\pin\\.\\path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "my\\pin\\oops\\..\\path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "my\\pin\\path\\oops\\..", "BPF:\\my\\pin\\path"); + + // Try a Windows-style absolute path without the device. + _verify_canonical_path(map_fd, "\\my\\pin\\path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "\\sys\\fs\\bpf\\my\\pin\\path", "BPF:\\my\\pin\\path"); + + // Try a legacy eBPF-for-Windows path. + _verify_canonical_path(map_fd, "/ebpf/global/my/pin/path", "BPF:\\my\\pin\\path"); + + // Verify invalid paths fail. + REQUIRE(ebpf_object_pin(map_fd, "..") == EBPF_INVALID_ARGUMENT); + REQUIRE(ebpf_object_pin(map_fd, "/..") == EBPF_INVALID_ARGUMENT); + REQUIRE(ebpf_object_pin(map_fd, "/mypinpath/../..") == EBPF_INVALID_ARGUMENT); + REQUIRE(ebpf_object_pin(map_fd, "C:\\") == EBPF_INVALID_ARGUMENT); + + Platform::_close(map_fd); +} + TEST_CASE("ebpf_get_next_pinned_object_path", "[end_to_end][pinning]") { _test_helper_end_to_end test_helper; @@ -1706,12 +1788,14 @@ TEST_CASE("ebpf_get_next_pinned_object_path", "[end_to_end][pinning]") REQUIRE(map_fd > 0); // Pin the program and map multiple times with a shared prefix. - const char* prefix = "/ebpf/test/"; + // We use canonical paths here so we can verify that what is + // enumerated is identical to the paths we pinned. + const char* prefix = "BPF:\\"; const char* paths[] = { - "/ebpf/test/map1", - "/ebpf/test/map2", - "/ebpf/test/program1", - "/ebpf/test/program2", + "BPF:\\map1", + "BPF:\\map2", + "BPF:\\program1", + "BPF:\\program2", }; const ebpf_object_type_t object_types[] = { EBPF_OBJECT_MAP, diff --git a/tests/end_to_end/netsh_test.cpp b/tests/end_to_end/netsh_test.cpp index 5d5fb7401a..fcd88498f6 100644 --- a/tests/end_to_end/netsh_test.cpp +++ b/tests/end_to_end/netsh_test.cpp @@ -589,7 +589,7 @@ TEST_CASE("pin first program", "[netsh][programs]") " 6 0 0 JIT sample callee\n"); output = _run_netsh_command(handle_ebpf_delete_program, L"5", nullptr, nullptr, &result); - REQUIRE(output == "Unpinned 5 from mypinpath\n"); + REQUIRE(output == "Unpinned 5 from BPF:\\mypinpath\n"); REQUIRE(result == NO_ERROR); verify_no_programs_exist(); @@ -618,11 +618,11 @@ TEST_CASE("pin all programs", "[netsh][programs]") " 6 1 0 JIT sample callee\n"); output = _run_netsh_command(handle_ebpf_delete_program, L"5", nullptr, nullptr, &result); - REQUIRE(output == "Unpinned 5 from mypinpath/sample_ext\n"); + REQUIRE(output == "Unpinned 5 from BPF:\\mypinpath\\sample_ext\n"); REQUIRE(result == NO_ERROR); output = _run_netsh_command(handle_ebpf_delete_program, L"6", nullptr, nullptr, &result); - REQUIRE(output == "Unpinned 6 from mypinpath/sample_ext_0\n"); + REQUIRE(output == "Unpinned 6 from BPF:\\mypinpath\\sample_ext_0\n"); REQUIRE(result == NO_ERROR); verify_no_programs_exist(); @@ -714,7 +714,7 @@ TEST_CASE("show programs", "[netsh][programs]") "# links : 0\n"); output = _run_netsh_command(handle_ebpf_delete_program, L"5", nullptr, nullptr, &result); - REQUIRE(output == "Unpinned 5 from mypinname\n"); + REQUIRE(output == "Unpinned 5 from BPF:\\mypinname\n"); REQUIRE(result == NO_ERROR); verify_no_programs_exist(); @@ -796,7 +796,7 @@ TEST_CASE("show maps", "[netsh][maps]") output = _run_netsh_command(handle_ebpf_delete_program, L"5", nullptr, nullptr, &result); REQUIRE(result == NO_ERROR); - REQUIRE(output == "Unpinned 5 from lookup\n"); + REQUIRE(output == "Unpinned 5 from BPF:\\lookup\n"); verify_no_programs_exist(); ebpf_epoch_synchronize(); @@ -862,15 +862,15 @@ TEST_CASE("show pins", "[netsh][pins]") output == "\n" " ID Type Path\n" "======= ======= ==============\n" - " 5 Program mypinpath/sample_ext\n" - " 6 Program mypinpath/sample_ext_0\n"); + " 5 Program BPF:\\mypinpath\\sample_ext\n" + " 6 Program BPF:\\mypinpath\\sample_ext_0\n"); output = _run_netsh_command(handle_ebpf_delete_program, L"5", nullptr, nullptr, &result); - REQUIRE(output == "Unpinned 5 from mypinpath/sample_ext\n"); + REQUIRE(output == "Unpinned 5 from BPF:\\mypinpath\\sample_ext\n"); REQUIRE(result == NO_ERROR); output = _run_netsh_command(handle_ebpf_delete_program, L"6", nullptr, nullptr, &result); - REQUIRE(output == "Unpinned 6 from mypinpath/sample_ext_0\n"); + REQUIRE(output == "Unpinned 6 from BPF:\\mypinpath\\sample_ext_0\n"); REQUIRE(result == NO_ERROR); verify_no_programs_exist(); @@ -899,7 +899,7 @@ TEST_CASE("delete pinned program", "[netsh][programs]") // Verify we can delete a pinned program. output = _run_netsh_command(handle_ebpf_delete_program, L"5", nullptr, nullptr, &result); - REQUIRE(output == "Unpinned 5 from mypinname\nUnpinned 5 from mypinname2\n"); + REQUIRE(output == "Unpinned 5 from BPF:\\mypinname\nUnpinned 5 from BPF:\\mypinname2\n"); REQUIRE(result == NO_ERROR); // Verify the program ID doesn't exist any more. @@ -925,7 +925,7 @@ TEST_CASE("unpin program", "[netsh][programs]") // Verify we can delete the unpinned program. output = _run_netsh_command(handle_ebpf_delete_program, L"5", nullptr, nullptr, &result); - REQUIRE(output == "Unpinned 5 from mypinname\n"); + REQUIRE(output == "Unpinned 5 from BPF:\\mypinname\n"); REQUIRE(result == NO_ERROR); // Verify the program ID doesn't exist any more. @@ -947,7 +947,7 @@ TEST_CASE("xdp interface parameter", "[netsh][programs]") REQUIRE(result == NO_ERROR); output = _run_netsh_command(handle_ebpf_delete_program, L"5", nullptr, nullptr, &result); REQUIRE(result == NO_ERROR); - REQUIRE(output == "Unpinned 5 from mypinpath\n"); + REQUIRE(output == "Unpinned 5 from BPF:\\mypinpath\n"); verify_no_programs_exist(); // Load program with pinpath and loopback interface name. @@ -957,7 +957,7 @@ TEST_CASE("xdp interface parameter", "[netsh][programs]") REQUIRE(result == NO_ERROR); output = _run_netsh_command(handle_ebpf_delete_program, L"10", nullptr, nullptr, &result); REQUIRE(result == NO_ERROR); - REQUIRE(output == "Unpinned 10 from mypinpath\n"); + REQUIRE(output == "Unpinned 10 from BPF:\\mypinpath\n"); verify_no_programs_exist(); // Load program with loopback interface index. @@ -966,7 +966,7 @@ TEST_CASE("xdp interface parameter", "[netsh][programs]") REQUIRE(result == NO_ERROR); output = _run_netsh_command(handle_ebpf_delete_program, L"15", nullptr, nullptr, &result); REQUIRE(result == NO_ERROR); - REQUIRE(output == "Unpinned 15 from DropPacket\n"); + REQUIRE(output == "Unpinned 15 from BPF:\\DropPacket\n"); verify_no_programs_exist(); // (Negative) Load program with incorrect interface name. @@ -1027,7 +1027,7 @@ TEST_CASE("cgroup_sock_addr compartment parameter", "[netsh][programs]") REQUIRE(result == NO_ERROR); output = _run_netsh_command(handle_ebpf_delete_program, L"6", nullptr, nullptr, &result); REQUIRE(result == NO_ERROR); - REQUIRE(output == "Unpinned 6 from mypinpath\n"); + REQUIRE(output == "Unpinned 6 from BPF:\\mypinpath\n"); verify_no_programs_exist(); // (Negative) Load program with incorrect compartment id. @@ -1099,8 +1099,8 @@ TEST_CASE("pin/unpin program", "[netsh][pin]") "\n" " ID Type Path\n" "======= ======= ==============\n" - " {0} Program bindmonitor\n" - " {0} Program bindmonitorpin\n", + " {0} Program BPF:\\bindmonitor\n" + " {0} Program BPF:\\bindmonitorpin\n", id)); _run_netsh_command(handle_ebpf_unpin_program, sid.c_str(), L"random", nullptr, &result); @@ -1115,7 +1115,7 @@ TEST_CASE("pin/unpin program", "[netsh][pin]") "\n" " ID Type Path\n" "======= ======= ==============\n" - " {} Program bindmonitor\n", + " {} Program BPF:\\bindmonitor\n", id)); _run_netsh_command(handle_ebpf_delete_program, sid.c_str(), nullptr, nullptr, &result); diff --git a/tests/libs/common/common_tests.cpp b/tests/libs/common/common_tests.cpp index 7fab2f90ee..a93313004d 100644 --- a/tests/libs/common/common_tests.cpp +++ b/tests/libs/common/common_tests.cpp @@ -61,9 +61,13 @@ ebpf_test_pinned_map_enum() for (int i = 0; i < pinned_map_count; i++) { bool matched = false; std::string pin_path = pin_path_prefix + std::to_string(i); - REQUIRE(( - matched = - (static_cast(pin_path.size()) == strnlen_s(map_info[i].pin_path, EBPF_MAX_PIN_PATH_LENGTH)))); + + // The canonical file path actually pinned should have the "BPF:" prefix, + // so should be 4 longer than the non-canonical path. + REQUIRE( + (matched = + (static_cast(pin_path.size() + 4) == + strnlen_s(map_info[i].pin_path, EBPF_MAX_PIN_PATH_LENGTH)))); std::string temp(map_info[i].pin_path); results[pin_path] = temp; From bd6220c8b736a01dcbf51fabd25775ed694fa48e Mon Sep 17 00:00:00 2001 From: Dave Thaler Date: Tue, 18 Mar 2025 01:45:11 -0700 Subject: [PATCH 2/6] Update some bpftool_tests Signed-off-by: Dave Thaler --- tests/bpftool_tests/bpftool_tests.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/bpftool_tests/bpftool_tests.cpp b/tests/bpftool_tests/bpftool_tests.cpp index 29283abc08..b740c0225a 100644 --- a/tests/bpftool_tests/bpftool_tests.cpp +++ b/tests/bpftool_tests/bpftool_tests.cpp @@ -104,8 +104,8 @@ TEST_CASE("prog load map_in_map", "[prog][load]") std::string expected_output = "\n\n ID Type Path\n"; expected_output += "======= ======= ==============\n"; - expected_output += std::format("{:>7s} Program map_in_map\n", id); - expected_output += std::format("{:>7s} Program pin2\n", id); + expected_output += std::format("{:>7s} Program BPF:\\map_in_map\n", id); + expected_output += std::format("{:>7s} Program BPF:\\pin2\n", id); output = run_command("netsh ebpf show pins", &result); REQUIRE(output == expected_output); @@ -114,9 +114,9 @@ TEST_CASE("prog load map_in_map", "[prog][load]") output = run_command(("netsh ebpf delete prog " + id).c_str(), &result); REQUIRE( output == "\nUnpinned " + id + - " from map_in_map\n" + " from BPF:\\map_in_map\n" "Unpinned " + - id + " from pin2\n"); + id + " from BPF:\\pin2\n"); REQUIRE(result == 0); output = run_command("bpftool prog show", &result); @@ -154,7 +154,7 @@ TEST_CASE("prog attach by interface alias", "[prog][load]") &result); REQUIRE(result == 0); output = run_command(("netsh ebpf delete prog " + id).c_str(), &result); - REQUIRE(output == "\nUnpinned " + id + " from droppacket\n"); + REQUIRE(output == "\nUnpinned " + id + " from BPF:\\droppacket\n"); REQUIRE(result == 0); output = run_command("bpftool prog show", &result); @@ -183,14 +183,14 @@ TEST_CASE("map create", "[map]") "Found 2 elements\n"); REQUIRE(status == 0); - REQUIRE(ebpf_object_unpin("FileName") == EBPF_SUCCESS); + REQUIRE(ebpf_object_unpin("BPF:\\FileName") == EBPF_SUCCESS); } TEST_CASE("map show pinned", "[map]") { int status; std::string output = - run_command("bpftool map create \\\\test_map type hash key 4 value 4 entries 10 name testing", &status); + run_command("bpftool map create test_map type hash key 4 value 4 entries 10 name testing", &status); REQUIRE(output == ""); REQUIRE(status == 0); @@ -199,11 +199,11 @@ TEST_CASE("map show pinned", "[map]") std::string id = std::to_string(atoi(output.c_str())); REQUIRE(output == id + ": hash name testing flags 0x0\n\tkey 4B value 4B max_entries 10\n"); - output = run_command("bpftool map show pinned \\\\test_map", &status); + output = run_command("bpftool map show pinned BPF:\\test_map", &status); REQUIRE(status == 0); REQUIRE(output == id + ": hash name testing flags 0x0\n\tkey 4B value 4B max_entries 10\n"); - REQUIRE(ebpf_object_unpin("\\\\test_map") == EBPF_SUCCESS); + REQUIRE(ebpf_object_unpin("BPF:\\test_map") == EBPF_SUCCESS); } TEST_CASE("prog show id 1", "[prog][show]") @@ -262,7 +262,7 @@ TEST_CASE("prog prog run", "[prog][load]") REQUIRE(output.find("Return value: 42, duration (average): ") != std::string::npos); output = run_command(("netsh ebpf delete prog " + id).c_str(), &result); - REQUIRE(output == "\nUnpinned " + id + " from test_sample_ebpf\n"); + REQUIRE(output == "\nUnpinned " + id + " from BPF:\\test_sample_ebpf\n"); REQUIRE(result == 0); output = run_command("bpftool prog show", &result); From 6abb6c8f611ae6d905aca9c5dc854ec98f0cd414 Mon Sep 17 00:00:00 2001 From: Dave Thaler Date: Tue, 18 Mar 2025 08:53:52 -0700 Subject: [PATCH 3/6] Add decision to PinPaths.md Signed-off-by: Dave Thaler --- docs/PinPaths.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/PinPaths.md b/docs/PinPaths.md index 43fb3fabca..f939a620b4 100644 --- a/docs/PinPaths.md +++ b/docs/PinPaths.md @@ -96,3 +96,13 @@ in between good and bad. | RemoteFile | Y | Y | Y | Y | N | | FileAPIs | Y | N | Y | N | Y | | CaseSensitive | N | N | Y | Y | Y | + +## Decision + +Based on the evaluation matrix, the Virtual form +("BPF:\my\pin\path") will be the canonical form. + +In addition, for portability, the Linux form ("/sys/fs/bpf/my/pin/path") +will be accepted as valid, as will the older eBPF for Windows +path ("/ebpf/global/my/pin/path"). These will be canonicalized +to "BPF:\my\pin\path" internally. From 144b330459a6801adc0ed6435416892102434717 Mon Sep 17 00:00:00 2001 From: Dave Thaler Date: Mon, 31 Mar 2025 08:20:30 -0700 Subject: [PATCH 4/6] Address PR feedback Signed-off-by: Dave Thaler --- docs/PinPaths.md | 21 +++++++++++++++------ libs/shared/shared_common.c | 3 --- tests/end_to_end/end_to_end.cpp | 6 +++--- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/docs/PinPaths.md b/docs/PinPaths.md index f939a620b4..2b29c893d8 100644 --- a/docs/PinPaths.md +++ b/docs/PinPaths.md @@ -4,7 +4,8 @@ On Linux, objects can be pinned to a filesystem path. As a result: * Pins can be enumerated or removed via normal filesystem APIs/utilities * Pin paths are case-sensitive * The path component separator is a forward slash ('/') -* Pin paths begin with "/sys/fs/bpf/" and relative paths like "mymap" are relative to that prefix. +* Pin paths typically begin with "/sys/fs/bpf/", but could begin with other + paths. Relative paths like "mymap" are then relative to that prefix. * ".." and "." components are resolved. On Windows, pin paths are not currently part of the filesystem. As a result, @@ -52,18 +53,20 @@ Example: /mnt/c/ebpf/global/my/pin/path Like the previous option, the current or system drive letter is still needed, so this has the same issues as the previous option. -### Virtual (Windows file system path with another virtual drive for BPF) +### Virtual (Windows device path with another virtual drive for BPF) Example: BPF:\my\pin\path +Note that this is a device path not a "file system" path per se. + In this option, there would be no collisions with actual files, and the path can be case -sensitive if the "BPF:" driver declares such. Similarly, ".." resolution would work if +sensitive if the "BPF:" driver declares it as such. Similarly, ".." resolution would work if the driver implements it as such. As for portability, canonicalization could ensure that Linux style paths like -"/sys/fs/bpf/my/pin/path" would be canonicalized to "BPF:\my\pin\path" allowing -portability in that respect. But querying the pin path on an object - +"/sys/fs/bpf/my/pin/path" would be canonicalized to "BPF:\sys\fs\bpf\my\pin\path" allowing +portability in that respect. Querying the pin path using a Windows-specific +API on an object would show the canonical path. ### Linux (Linux-like file system path) @@ -81,6 +84,12 @@ In this option, "BPF" would be a special "host" like "localhost" or "?". However, it may be confusing if there is another host with the name "bpf", or at least lead people to believe there might be. +### Others + +There are also "\\?\" and "\\.\" paths on Windows, but these don't +appear to provide any value that one or more of the other options don't +already provide so are omitted from the rest of the evaluation. + ## Evaluation Matrix The matrix below shows how each option evaluates using the diff --git a/libs/shared/shared_common.c b/libs/shared/shared_common.c index fa845b3dc9..3418261c7d 100644 --- a/libs/shared/shared_common.c +++ b/libs/shared/shared_common.c @@ -676,9 +676,6 @@ ebpf_canonicalize_path(_Out_writes_(output_size) char* output, size_t output_siz if (_strnicmp(output + PREFIX_SIZE, "\\ebpf\\global\\", 13) == 0) { char* next = output + PREFIX_SIZE + 13; memmove(output + PREFIX_SIZE, next, strlen(next) + 1); - } else if (strncmp(output + PREFIX_SIZE, "\\sys\\fs\\bpf\\", 12) == 0) { - char* next = output + PREFIX_SIZE + 12; - memmove(output + PREFIX_SIZE, next, strlen(next) + 1); } for (int i = PREFIX_SIZE - 1; output[i] != 0;) { diff --git a/tests/end_to_end/end_to_end.cpp b/tests/end_to_end/end_to_end.cpp index f4f9e01812..980ad0bee3 100644 --- a/tests/end_to_end/end_to_end.cpp +++ b/tests/end_to_end/end_to_end.cpp @@ -1721,8 +1721,8 @@ TEST_CASE("pin path canonicalization", "[end_to_end][pinning]") _verify_canonical_path(map_fd, "BPF:/./my/pin/path", "BPF:\\my\\pin\\path"); // Try Linux-style absolute paths. - _verify_canonical_path(map_fd, "/sys/fs/bpf/my/pin/path", "BPF:\\my\\pin\\path"); - _verify_canonical_path(map_fd, "/sys/fs/bpf/My/Pin/Path", "BPF:\\My\\Pin\\Path"); + _verify_canonical_path(map_fd, "/sys/fs/bpf/my/pin/path", "BPF:\\sys\\fs\\bpf\\my\\pin\\path"); + _verify_canonical_path(map_fd, "/sys/fs/bpf/My/Pin/Path", "BPF:\\sys\\fs\\bpf\\My\\Pin\\Path"); // Try Linux-style relative paths. _verify_canonical_path(map_fd, "my/pin/path", "BPF:\\my\\pin\\path"); @@ -1741,7 +1741,7 @@ TEST_CASE("pin path canonicalization", "[end_to_end][pinning]") // Try a Windows-style absolute path without the device. _verify_canonical_path(map_fd, "\\my\\pin\\path", "BPF:\\my\\pin\\path"); - _verify_canonical_path(map_fd, "\\sys\\fs\\bpf\\my\\pin\\path", "BPF:\\my\\pin\\path"); + _verify_canonical_path(map_fd, "\\sys\\fs\\bpf\\my\\pin\\path", "BPF:\\sys\\fs\\bpf\\my\\pin\\path"); // Try a legacy eBPF-for-Windows path. _verify_canonical_path(map_fd, "/ebpf/global/my/pin/path", "BPF:\\my\\pin\\path"); From f1dd483186c81efd8e133f2e887445274b4450f0 Mon Sep 17 00:00:00 2001 From: Dave Thaler Date: Tue, 1 Apr 2025 17:04:34 -0700 Subject: [PATCH 5/6] Address PR feedback Signed-off-by: Dave Thaler --- libs/shared/shared_common.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/libs/shared/shared_common.c b/libs/shared/shared_common.c index 3418261c7d..96719640d0 100644 --- a/libs/shared/shared_common.c +++ b/libs/shared/shared_common.c @@ -652,17 +652,23 @@ ebpf_duplicate_program_data( ebpf_result_t ebpf_canonicalize_path(_Out_writes_(output_size) char* output, size_t output_size, _In_z_ const char* input) { - const int PREFIX_SIZE = 5; // Length of "BPF:\\". - strcpy_s(output, output_size, "BPF:\\"); + const int DEVICE_PREFIX_SIZE = 4; // Length of "BPF:". + if (strcpy_s(output, output_size, "BPF:") != 0) { + return EBPF_INVALID_ARGUMENT; + } + if (input[0] != '/' && input[0] != '\\') { + // A relative path is relative to the root. + strcat_s(output, output_size, "\\"); + } // Add the BPF: prefix if not already present. - if (_strnicmp(input, "BPF:\\", PREFIX_SIZE) == 0 || _strnicmp(input, "BPF:/", PREFIX_SIZE) == 0) { - strcpy_s(output + PREFIX_SIZE, output_size - PREFIX_SIZE, input + PREFIX_SIZE); + if (_strnicmp(input, "BPF:", DEVICE_PREFIX_SIZE) == 0) { + strcat_s(output, output_size, input + DEVICE_PREFIX_SIZE); } else { - strcpy_s(output + PREFIX_SIZE, output_size - PREFIX_SIZE, input); + strcat_s(output, output_size, input); } - for (int i = PREFIX_SIZE; output[i] != 0; i++) { + for (int i = DEVICE_PREFIX_SIZE; output[i] != 0; i++) { if (output[i] == '/') { // Convert slashes to backslashes. output[i] = '\\'; @@ -673,12 +679,12 @@ ebpf_canonicalize_path(_Out_writes_(output_size) char* output, size_t output_siz } // Remove other prefix aliases from the beginning. - if (_strnicmp(output + PREFIX_SIZE, "\\ebpf\\global\\", 13) == 0) { - char* next = output + PREFIX_SIZE + 13; - memmove(output + PREFIX_SIZE, next, strlen(next) + 1); + if (_strnicmp(output + DEVICE_PREFIX_SIZE, "\\ebpf\\global\\", 13) == 0) { + char* next = output + DEVICE_PREFIX_SIZE + 13; + memmove(output + DEVICE_PREFIX_SIZE + 1, next, strlen(next) + 1); } - for (int i = PREFIX_SIZE - 1; output[i] != 0;) { + for (int i = DEVICE_PREFIX_SIZE; output[i] != 0;) { if (strncmp(output + i, "\\\\", 2) == 0) { char* next = output + i + 2; memmove(output + i + 1, next, strlen(next) + 1); @@ -689,7 +695,7 @@ ebpf_canonicalize_path(_Out_writes_(output_size) char* output, size_t output_siz memmove(output + i + 1, next, strlen(next) + 1); } else if (strncmp(output + i, "\\..\\", 4) == 0) { int previous_index = i - 1; - for (; previous_index >= 4; previous_index--) { + for (; previous_index >= DEVICE_PREFIX_SIZE; previous_index--) { if (output[previous_index] == '\\') { // Back up to the previous separator. char* next = output + i + 4; @@ -698,12 +704,12 @@ ebpf_canonicalize_path(_Out_writes_(output_size) char* output, size_t output_siz break; } } - if (previous_index < PREFIX_SIZE - 1) { + if (previous_index < DEVICE_PREFIX_SIZE) { return EBPF_INVALID_ARGUMENT; } } else if (strcmp(output + i, "\\..") == 0) { int previous_index = i - 1; - for (; previous_index >= PREFIX_SIZE - 1; previous_index--) { + for (; previous_index >= DEVICE_PREFIX_SIZE; previous_index--) { if (output[previous_index] == '\\') { // Terminate the string at the previous separator. output[previous_index] = 0; @@ -711,7 +717,7 @@ ebpf_canonicalize_path(_Out_writes_(output_size) char* output, size_t output_siz break; } } - if (previous_index < PREFIX_SIZE - 1) { + if (previous_index < DEVICE_PREFIX_SIZE) { return EBPF_INVALID_ARGUMENT; } } else { From b601d54361004b489ea5ec6831c96440de4165a3 Mon Sep 17 00:00:00 2001 From: Dave Thaler Date: Mon, 21 Apr 2025 15:57:49 -0700 Subject: [PATCH 6/6] Export ebpf_canonicalize_pin_path Signed-off-by: Dave Thaler --- ebpfapi/Source.def | 1 + include/ebpf_api.h | 14 ++++++++++++++ libs/api/ebpf_api.cpp | 24 ++++++++++++++++++++---- tests/end_to_end/end_to_end.cpp | 18 +++++++++++++++++- 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/ebpfapi/Source.def b/ebpfapi/Source.def index e69a62fd01..168eef5c05 100644 --- a/ebpfapi/Source.def +++ b/ebpfapi/Source.def @@ -109,6 +109,7 @@ EXPORTS ebpf_api_close_handle ebpf_api_get_pinned_map_info ebpf_api_map_info_free + ebpf_canonicalize_pin_path ebpf_close_fd ebpf_duplicate_fd ebpf_enumerate_programs diff --git a/include/ebpf_api.h b/include/ebpf_api.h index 943e3426f7..5aeeb24481 100644 --- a/include/ebpf_api.h +++ b/include/ebpf_api.h @@ -619,6 +619,20 @@ extern "C" size_t next_path_len, _Inout_ ebpf_object_type_t* type) EBPF_NO_EXCEPT; + /** + * @brief Canonicalize a path using filesystem canonicalization rules. + * + * @param[out] output Buffer in which to write canonicalized path. + * @param[in] output_size Size of output buffer. + * @param[out] error_code Zero on success, non-zero Win32 error code on failure. + * + * @retval EBPF_SUCCESS The operation was successful. + * @retval EBPF_INVALID_ARGUMENT The input path was invalid. + */ + _Must_inspect_result_ ebpf_result_t + ebpf_canonicalize_pin_path(_Out_writes_(output_size) char* output, size_t output_size, _In_z_ const char* input) + EBPF_NO_EXCEPT; + typedef struct _ebpf_program_info ebpf_program_info_t; /** diff --git a/libs/api/ebpf_api.cpp b/libs/api/ebpf_api.cpp index 7b7f82877d..60e4e3e6c8 100644 --- a/libs/api/ebpf_api.cpp +++ b/libs/api/ebpf_api.cpp @@ -4201,10 +4201,16 @@ ebpf_get_next_pinned_object_path( ebpf_assert(start_path); ebpf_assert(next_path); - size_t start_path_length = strlen(start_path); + char canonical_start_path[MAX_PATH]; + ebpf_result_t result = ebpf_canonicalize_path(canonical_start_path, sizeof(canonical_start_path), start_path); + if (result != ERROR_SUCCESS) { + EBPF_RETURN_RESULT(EBPF_INVALID_ARGUMENT); + } + + size_t canonical_start_path_length = strlen(canonical_start_path); ebpf_protocol_buffer_t request_buffer( - EBPF_OFFSET_OF(ebpf_operation_get_next_pinned_object_path_request_t, start_path) + start_path_length); + EBPF_OFFSET_OF(ebpf_operation_get_next_pinned_object_path_request_t, start_path) + canonical_start_path_length); ebpf_protocol_buffer_t reply_buffer( EBPF_OFFSET_OF(ebpf_operation_get_next_pinned_object_path_reply_t, next_path) + (next_path_len - 1)); ebpf_operation_get_next_pinned_object_path_request_t* request = @@ -4217,10 +4223,10 @@ ebpf_get_next_pinned_object_path( reply->header.length = static_cast(reply_buffer.size()); request->type = *type; - memcpy(request->start_path, start_path, start_path_length); + memcpy(request->start_path, canonical_start_path, canonical_start_path_length); uint32_t error = invoke_ioctl(request_buffer, reply_buffer); - ebpf_result_t result = win32_error_code_to_ebpf_result(error); + result = win32_error_code_to_ebpf_result(error); if (result != EBPF_SUCCESS) { EBPF_RETURN_RESULT(result); } @@ -4233,6 +4239,16 @@ ebpf_get_next_pinned_object_path( } CATCH_NO_MEMORY_EBPF_RESULT +_Must_inspect_result_ ebpf_result_t +ebpf_canonicalize_pin_path(_Out_writes_(output_size) char* output, size_t output_size, _In_z_ const char* input) + NO_EXCEPT_TRY +{ + EBPF_LOG_ENTRY(); + ebpf_result_t result = ebpf_canonicalize_path(output, output_size, input); + EBPF_RETURN_RESULT(result); +} +CATCH_NO_MEMORY_EBPF_RESULT + static ebpf_result_t _get_next_id(ebpf_operation_id_t operation, ebpf_id_t start_id, _Out_ ebpf_id_t* next_id) NO_EXCEPT_TRY { diff --git a/tests/end_to_end/end_to_end.cpp b/tests/end_to_end/end_to_end.cpp index 980ad0bee3..ddde70ea33 100644 --- a/tests/end_to_end/end_to_end.cpp +++ b/tests/end_to_end/end_to_end.cpp @@ -1676,6 +1676,10 @@ TEST_CASE("pinned_map_enum", "[end_to_end]") static void _verify_canonical_path(int fd, _In_z_ const char* original_path, _In_z_ const char* canonical_path) { + char path[EBPF_MAX_PIN_PATH_LENGTH] = ""; + REQUIRE(ebpf_canonicalize_pin_path(path, sizeof(path), original_path) == EBPF_SUCCESS); + REQUIRE(strcmp(path, canonical_path) == 0); + // Pin the fd to the original path. REQUIRE(ebpf_object_pin(fd, original_path) == EBPF_SUCCESS); @@ -1691,7 +1695,6 @@ _verify_canonical_path(int fd, _In_z_ const char* original_path, _In_z_ const ch // Look up the actual path pinned. ebpf_object_type_t object_type = EBPF_OBJECT_UNKNOWN; - char path[EBPF_MAX_PIN_PATH_LENGTH] = ""; while (ebpf_get_next_pinned_object_path(path, path, sizeof(path), &object_type) == EBPF_SUCCESS) { int fd2 = bpf_obj_get(path); if (fd2 < 0) { @@ -1851,6 +1854,19 @@ TEST_CASE("ebpf_get_next_pinned_object_path", "[end_to_end][pinning]") REQUIRE(count == expected_count); + // Try some non-canonical paths. + object_type = EBPF_OBJECT_UNKNOWN; + REQUIRE(ebpf_get_next_pinned_object_path("/", path, sizeof(path), &object_type) == EBPF_SUCCESS); + REQUIRE(strcmp(path, "BPF:\\map1") == 0); + object_type = EBPF_OBJECT_UNKNOWN; + REQUIRE(ebpf_get_next_pinned_object_path("/none", path, sizeof(path), &object_type) == EBPF_SUCCESS); + REQUIRE(strcmp(path, "BPF:\\program1") == 0); + object_type = EBPF_OBJECT_UNKNOWN; + REQUIRE(ebpf_get_next_pinned_object_path("/foo/../ebpf", path, sizeof(path), &object_type) == EBPF_SUCCESS); + REQUIRE(strcmp(path, "BPF:\\map1") == 0); + object_type = EBPF_OBJECT_UNKNOWN; + REQUIRE(ebpf_get_next_pinned_object_path("/foo/../zub", path, sizeof(path), &object_type) == EBPF_NO_MORE_KEYS); + // Clean up. REQUIRE(ebpf_object_unpin(paths[0]) == EBPF_SUCCESS); REQUIRE(ebpf_object_unpin(paths[1]) == EBPF_SUCCESS);