Cherry pick PR #3495 and #3485. (#3496)

* Fixes for memory leaks and crashes (#3485) * test * fix * fix * fix * fix * fix * more fixes * fix * fix * fix * Revert "test" This reverts commit ff02406. * fix * fix * Revert overly strict check in link attach to provider (#3495) Signed-off-by: Alan Jowett <[email protected]> Co-authored-by: Alan Jowett <[email protected]> --------- Signed-off-by: Alan Jowett <[email protected]> Co-authored-by: Anurag Saxena <[email protected]> Co-authored-by: Alan Jowett <[email protected]> Co-authored-by: Alan Jowett <[email protected]>
microsoft · Apr 25, 2024 · 6093388 · 6093388
1 parent d7821f5
commit 6093388
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 7 deletions.
diff --git a/libs/execution_context/ebpf_link.c b/libs/execution_context/ebpf_link.c
@@ -153,13 +153,23 @@ _ebpf_link_client_attach_provider(
     }
 
     if (memcmp(&provider_registration_instance->ModuleId->Guid, &link->attach_type, sizeof(link->attach_type)) != 0) {
-        // This is not the provider we are looking for.
+        EBPF_LOG_MESSAGE_GUID_GUID(
+            EBPF_TRACELOG_LEVEL_VERBOSE,
+            EBPF_TRACELOG_KEYWORD_LINK,
+            "Attach provider ModuleId does not match link.",
+            &provider_registration_instance->ModuleId->Guid,
+            &link->attach_type);
         status = STATUS_NOINTERFACE;
         goto Done;
     }
 
     if (memcmp(&attach_provider_data->supported_program_type, &link->program_type, sizeof(link->program_type)) != 0) {
-        // This is not the provider we are looking for.
+        EBPF_LOG_MESSAGE_GUID_GUID(
+            EBPF_TRACELOG_LEVEL_VERBOSE,
+            EBPF_TRACELOG_KEYWORD_LINK,
+            "Attach provider program type does not match link.",
+            &provider_registration_instance->ModuleId->Guid,
+            &link->attach_type);
         status = STATUS_NOINTERFACE;
         goto Done;
     }

diff --git a/libs/execution_context/ebpf_program.c b/libs/execution_context/ebpf_program.c
@@ -401,10 +401,12 @@ _ebpf_program_type_specific_program_information_attach_provider(
 
     if (!_ebpf_program_match_provider_data_module_id(
             provider_registration_instance->ModuleId, &program->parameters.program_type)) {
-        EBPF_LOG_MESSAGE(
+        EBPF_LOG_MESSAGE_GUID_GUID(
             EBPF_TRACELOG_LEVEL_ERROR,
             EBPF_TRACELOG_KEYWORD_PROGRAM,
-            "Program information provider module ID mismatch.");
+            "Program information provider module ID mismatch.",
+            &program->parameters.program_type,
+            &provider_registration_instance->ModuleId->Guid);
         status = STATUS_INVALID_PARAMETER;
         goto Done;
     }

diff --git a/libs/shared/shared_common.c b/libs/shared/shared_common.c
@@ -105,9 +105,7 @@ ebpf_validate_attach_provider_data(_In_ const ebpf_attach_provider_data_t* attac
     return (
         (attach_provider_data != NULL) &&
         _ebpf_validate_extension_object_header(EBPF_ATTACH_PROVIDER_DATA, &attach_provider_data->header) &&
-        !IsEqualGUID(&attach_provider_data->supported_program_type, &GUID_NULL) &&
-        (attach_provider_data->link_type < BPF_LINK_TYPE_MAX) &&
-        (attach_provider_data->bpf_attach_type < __MAX_BPF_ATTACH_TYPE));
+        !IsEqualGUID(&attach_provider_data->supported_program_type, &GUID_NULL));
 }
 
 static bool
@@ -242,6 +240,9 @@ _duplicate_program_descriptor(
     program_type_descriptor_copy->header.version = EBPF_PROGRAM_TYPE_DESCRIPTOR_CURRENT_VERSION;
     program_type_descriptor_copy->header.size = EBPF_PROGRAM_TYPE_DESCRIPTOR_CURRENT_VERSION_SIZE;
 
+    // Initialize pointers to NULL.
+    program_type_descriptor_copy->context_descriptor = NULL;
+
     program_type_descriptor_copy->name = cxplat_duplicate_string(program_type_descriptor->name);
     if (program_type_descriptor_copy->name == NULL) {
         result = EBPF_NO_MEMORY;
@@ -299,6 +300,7 @@ _duplicate_helper_function_prototype_array(
         memcpy(&local_helper_prototype_array[i], helper_prototype, helper_prototype_size);
         local_helper_prototype_array[i].header.version = EBPF_HELPER_FUNCTION_PROTOTYPE_CURRENT_VERSION;
         local_helper_prototype_array[i].header.size = EBPF_HELPER_FUNCTION_PROTOTYPE_CURRENT_VERSION_SIZE;
+
         local_helper_prototype_array[i].name = cxplat_duplicate_string(helper_prototype->name);
         if (local_helper_prototype_array[i].name == NULL) {
             result = EBPF_NO_MEMORY;
@@ -383,6 +385,9 @@ ebpf_duplicate_program_info(_In_ const ebpf_program_info_t* info, _Outptr_ ebpf_
             info->program_type_specific_helper_prototype,
             info->count_of_program_type_specific_helpers,
             &program_info->program_type_specific_helper_prototype);
+        if (result != EBPF_SUCCESS) {
+            goto Exit;
+        }
     }
 
     result = _duplicate_program_descriptor(info->program_type_descriptor, &program_info->program_type_descriptor);
@@ -418,6 +423,8 @@ _duplicate_helper_function_addresses(
     ebpf_result_t result = EBPF_SUCCESS;
     ebpf_helper_function_addresses_t* helper_function_addresses_copy = NULL;
 
+    *new_helper_function_addresses = NULL;
+
     helper_function_addresses_copy =
         (ebpf_helper_function_addresses_t*)ebpf_allocate(sizeof(ebpf_helper_function_addresses_t));
     if (helper_function_addresses_copy == NULL) {
@@ -484,6 +491,10 @@ ebpf_duplicate_program_data(
     program_data_copy->header.version = EBPF_PROGRAM_DATA_CURRENT_VERSION;
     program_data_copy->header.size = EBPF_PROGRAM_DATA_CURRENT_VERSION_SIZE;
 
+    // Initialize pointers to NULL.
+    program_data_copy->program_type_specific_helper_function_addresses = NULL;
+    program_data_copy->program_info = NULL;
+
     if (program_data->global_helper_function_addresses != NULL) {
         result = _duplicate_helper_function_addresses(
             program_data->global_helper_function_addresses, &program_data_copy->global_helper_function_addresses);