From fc8d952df6597d294cda5333df8fa3727981d7e1 Mon Sep 17 00:00:00 2001 From: Sharmila Palani Date: Wed, 29 Mar 2023 19:51:59 -0700 Subject: [PATCH 1/3] Fix memory leaks --- libs/api/ebpf_api.cpp | 8 +++++++- libs/api_common/store_helper_internal.cpp | 17 +++++++++++++++++ libs/execution_context/ebpf_native.c | 1 + netebpfext/user/fwp_um.cpp | 5 ++++- 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/libs/api/ebpf_api.cpp b/libs/api/ebpf_api.cpp index ddf7380874..cc66f3244f 100644 --- a/libs/api/ebpf_api.cpp +++ b/libs/api/ebpf_api.cpp @@ -1753,11 +1753,17 @@ _initialize_ebpf_object_from_native_file( } Exit: - ebpf_free(program); if (result != EBPF_SUCCESS) { + if (program && object.programs.size() == 0) { + ebpf_free(program->section_name); + ebpf_free(program->program_name); + } + clean_up_ebpf_programs(object.programs); clean_up_ebpf_maps(object.maps); } + + ebpf_free(program); ebpf_free_sections(infos); EBPF_RETURN_RESULT(result); } diff --git a/libs/api_common/store_helper_internal.cpp b/libs/api_common/store_helper_internal.cpp index 0c01941322..bb6bb1f708 100644 --- a/libs/api_common/store_helper_internal.cpp +++ b/libs/api_common/store_helper_internal.cpp @@ -374,7 +374,15 @@ ebpf_store_load_program_information( Exit: if (result != EBPF_SUCCESS) { ebpf_free(*program_info); + + // Deallocate the dynamic memory in the program_info_array vector. + if (program_info_array.size() > 0) { + for (auto program_data : program_info_array) { + ebpf_program_info_free(program_data); + } + } } + if (program_data_key) { close_registry_key(program_data_key); } @@ -569,6 +577,15 @@ ebpf_store_load_section_information( Exit: if (result != EBPF_SUCCESS) { ebpf_free(*section_info); + // Deallocate the dynamic memory in the section_info_array vector. + if (section_info_array.size() > 0) { + for (auto section_data : section_info_array) { + ebpf_free(section_data->program_type); + ebpf_free(section_data->attach_type); + ebpf_free(const_cast(section_data->section_prefix)); + ebpf_free(section_data); + } + } } if (section_data_key) { close_registry_key(section_data_key); diff --git a/libs/execution_context/ebpf_native.c b/libs/execution_context/ebpf_native.c index b7025430f5..3883a99f44 100644 --- a/libs/execution_context/ebpf_native.c +++ b/libs/execution_context/ebpf_native.c @@ -314,6 +314,7 @@ ebpf_native_terminate() // ebpf_provider_unload is blocking call until all the // native modules have been detached. ebpf_provider_unload(_ebpf_native_provider); + _ebpf_native_provider = NULL; // All native modules should be cleaned up by now. ebpf_assert(!_ebpf_native_client_table || ebpf_hash_table_key_count(_ebpf_native_client_table) == 0); diff --git a/netebpfext/user/fwp_um.cpp b/netebpfext/user/fwp_um.cpp index ef7d3afbc3..97dde79640 100644 --- a/netebpfext/user/fwp_um.cpp +++ b/netebpfext/user/fwp_um.cpp @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation // SPDX-License-Identifier: MIT +#include "ebpf_fault_injection.h" #include "fwp_um.h" #include "net_ebpf_ext_sock_addr.h" #include "netebpfext_platform.h" @@ -21,6 +22,8 @@ DEFINE_GUID(EBPF_DEFAULT_SUBLAYER, 0x7c7b3fb9, 0x3331, 0x436a, 0x98, 0xe1, 0xb9, std::unique_ptr<_fwp_engine> _fwp_engine::_engine; +static bool fault_injection_enabled = ebpf_fault_injection_is_enabled(); + // Attempt to classify a test packet at a given WFP layer on a given interface index. // This is used to test the xdp hook. FWP_ACTION_TYPE @@ -269,7 +272,7 @@ _fwp_engine::test_cgroup_inet4_connect(_In_ fwp_classify_parameters_t* parameter action = test_callout( FWPS_LAYER_ALE_CONNECT_REDIRECT_V4, FWPM_LAYER_ALE_CONNECT_REDIRECT_V4, EBPF_DEFAULT_SUBLAYER, incoming_value); - ebpf_assert(action == FWP_ACTION_PERMIT); + ebpf_assert(action == FWP_ACTION_PERMIT || !fault_injection_enabled); if (_fwp_um_connect_request != nullptr) { redirected = From 043bea8c2cddb78b5fabb72c382952730cdd1327 Mon Sep 17 00:00:00 2001 From: Sharmila Palani Date: Thu, 30 Mar 2023 13:14:08 -0700 Subject: [PATCH 2/3] Calling clean_up_ebpf_program --- libs/api/ebpf_api.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libs/api/ebpf_api.cpp b/libs/api/ebpf_api.cpp index cc66f3244f..8941d56a4b 100644 --- a/libs/api/ebpf_api.cpp +++ b/libs/api/ebpf_api.cpp @@ -1727,6 +1727,7 @@ _initialize_ebpf_object_from_native_file( program->handle = ebpf_handle_invalid; program->program_type = info->program_type; program->attach_type = info->expected_attach_type; + program->fd = ebpf_fd_invalid; program->section_name = ebpf_duplicate_string(info->section_name); if (program->section_name == nullptr) { @@ -1754,16 +1755,16 @@ _initialize_ebpf_object_from_native_file( Exit: if (result != EBPF_SUCCESS) { - if (program && object.programs.size() == 0) { - ebpf_free(program->section_name); - ebpf_free(program->program_name); + if (program) { + // Deallocate program, if it was allocated but not added to + // the object programs vector. + clean_up_ebpf_program(program); } clean_up_ebpf_programs(object.programs); clean_up_ebpf_maps(object.maps); } - ebpf_free(program); ebpf_free_sections(infos); EBPF_RETURN_RESULT(result); } From 0f4f09f1bed4c29e7942faed83a36ad49589ee63 Mon Sep 17 00:00:00 2001 From: Sharmila Palani Date: Fri, 31 Mar 2023 08:46:20 -0700 Subject: [PATCH 3/3] Removed the change in fwp_um.cpp --- netebpfext/user/fwp_um.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/netebpfext/user/fwp_um.cpp b/netebpfext/user/fwp_um.cpp index 97dde79640..ef7d3afbc3 100644 --- a/netebpfext/user/fwp_um.cpp +++ b/netebpfext/user/fwp_um.cpp @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation // SPDX-License-Identifier: MIT -#include "ebpf_fault_injection.h" #include "fwp_um.h" #include "net_ebpf_ext_sock_addr.h" #include "netebpfext_platform.h" @@ -22,8 +21,6 @@ DEFINE_GUID(EBPF_DEFAULT_SUBLAYER, 0x7c7b3fb9, 0x3331, 0x436a, 0x98, 0xe1, 0xb9, std::unique_ptr<_fwp_engine> _fwp_engine::_engine; -static bool fault_injection_enabled = ebpf_fault_injection_is_enabled(); - // Attempt to classify a test packet at a given WFP layer on a given interface index. // This is used to test the xdp hook. FWP_ACTION_TYPE @@ -272,7 +269,7 @@ _fwp_engine::test_cgroup_inet4_connect(_In_ fwp_classify_parameters_t* parameter action = test_callout( FWPS_LAYER_ALE_CONNECT_REDIRECT_V4, FWPM_LAYER_ALE_CONNECT_REDIRECT_V4, EBPF_DEFAULT_SUBLAYER, incoming_value); - ebpf_assert(action == FWP_ACTION_PERMIT || !fault_injection_enabled); + ebpf_assert(action == FWP_ACTION_PERMIT); if (_fwp_um_connect_request != nullptr) { redirected =