Defer filter list deletion to when deleting filter context, add debugging info and traces (#3300)

Co-authored-by: Dhiren Vispute <Dhiren.Vispute@microsoft.com>

netebpfext/net_ebpf_ext.c

@@ -280,8 +280,6 @@ net_ebpf_extension_wfp_filter_context_cleanup(_Frees_ptr_ net_ebpf_extension_wfp
     // lingering WFP classify callbacks will exit as it would not find any hook client associated
     // with the filter context. This is best effort & no locks are held.
     filter_context->client_detached = TRUE;
-    filter_context->filter_ids = NULL;
-    filter_context->filter_ids_count = 0;
     DEREFERENCE_FILTER_CONTEXT(filter_context);
 }
 
@@ -353,18 +351,27 @@ net_ebpf_extension_get_callout_id_for_hook(net_ebpf_extension_hook_id_t hook_id)
     return callout_id;
 }
 void
-net_ebpf_extension_delete_wfp_filters(uint32_t filter_count, _Frees_ptr_ _In_count_(filter_count) uint64_t* filter_ids)
+net_ebpf_extension_delete_wfp_filters(
+    uint32_t filter_count, _Frees_ptr_ _In_count_(filter_count) net_ebpf_ext_wfp_filter_id_t* filter_ids)
 {
     NET_EBPF_EXT_LOG_ENTRY();
-    NTSTATUS status;
+    NTSTATUS status = STATUS_SUCCESS;
+
     for (uint32_t index = 0; index < filter_count; index++) {
-        status = FwpmFilterDeleteById(_fwp_engine_handle, filter_ids[index]);
+        status = FwpmFilterDeleteById(_fwp_engine_handle, filter_ids[index].id);
         if (!NT_SUCCESS(status)) {
             NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(
                 NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmFilterDeleteById", status);
+        } else {
+            filter_ids[index].state = NET_EBPF_EXT_WFP_FILTER_DELETED;
+            NET_EBPF_EXT_LOG_MESSAGE_UINT64_UINT64(
+                NET_EBPF_EXT_TRACELOG_LEVEL_VERBOSE,
+                NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION,
+                "Deleted WFP filter: ",
+                index,
+                filter_ids[index].id);
         }
     }
-    ExFreePool(filter_ids);
     NET_EBPF_EXT_LOG_EXIT();
 }
 
@@ -375,12 +382,12 @@ net_ebpf_extension_add_wfp_filters(
     uint32_t condition_count,
     _In_opt_count_(condition_count) const FWPM_FILTER_CONDITION* conditions,
     _Inout_ net_ebpf_extension_wfp_filter_context_t* filter_context,
-    _Outptr_result_buffer_maybenull_(filter_count) uint64_t** filter_ids)
+    _Outptr_result_buffer_maybenull_(filter_count) net_ebpf_ext_wfp_filter_id_t** filter_ids)
 {
     NTSTATUS status = STATUS_SUCCESS;
     ebpf_result_t result = EBPF_SUCCESS;
     bool is_in_transaction = FALSE;
-    uint64_t* local_filter_ids = NULL;
+    net_ebpf_ext_wfp_filter_id_t* local_filter_ids = NULL;
     *filter_ids = NULL;
 
     NET_EBPF_EXT_LOG_ENTRY();
@@ -392,12 +399,12 @@ net_ebpf_extension_add_wfp_filters(
         goto Exit;
     }
 
-    local_filter_ids = (uint64_t*)ExAllocatePoolUninitialized(
+    local_filter_ids = (net_ebpf_ext_wfp_filter_id_t*)ExAllocatePoolUninitialized(
-        NonPagedPoolNx, sizeof(uint64_t) * filter_count, NET_EBPF_EXTENSION_POOL_TAG);
+        NonPagedPoolNx, (sizeof(net_ebpf_ext_wfp_filter_id_t) * filter_count), NET_EBPF_EXTENSION_POOL_TAG);
     NET_EBPF_EXT_BAIL_ON_ALLOC_FAILURE_RESULT(
         NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, local_filter_ids, "local_filter_ids", result);
 
-    memset(local_filter_ids, 0, sizeof(uint64_t) * filter_count);
+    memset(local_filter_ids, 0, (sizeof(net_ebpf_ext_wfp_filter_id_t) * filter_count));
 
     status = FwpmTransactionBegin(_fwp_engine_handle, 0);
     if (!NT_SUCCESS(status)) {
@@ -409,6 +416,7 @@ net_ebpf_extension_add_wfp_filters(
 
     for (uint32_t index = 0; index < filter_count; index++) {
         FWPM_FILTER filter = {0};
+        uint64_t local_filter_id = 0;
         const net_ebpf_extension_wfp_filter_parameters_t* filter_parameter = &parameters[index];
 
         filter.layerKey = *filter_parameter->layer_guid;
@@ -428,7 +436,7 @@ net_ebpf_extension_add_wfp_filters(
         REFERENCE_FILTER_CONTEXT(filter_context);
         filter.rawContext = (uint64_t)(uintptr_t)filter_context;
 
-        status = FwpmFilterAdd(_fwp_engine_handle, &filter, NULL, &local_filter_ids[index]);
+        status = FwpmFilterAdd(_fwp_engine_handle, &filter, NULL, &local_filter_id);
         if (!NT_SUCCESS(status)) {
             NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE_MESSAGE_STRING(
                 NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION,
@@ -438,6 +446,16 @@ net_ebpf_extension_add_wfp_filters(
                 (char*)filter_parameter->name);
             result = EBPF_INVALID_ARGUMENT;
             goto Exit;
+        } else {
+            local_filter_ids[index].id = local_filter_id;
+            local_filter_ids[index].name = (wchar_t*)filter_parameter->name;
+            local_filter_ids[index].state = NET_EBPF_EXT_WFP_FILTER_ADDED;
+            NET_EBPF_EXT_LOG_MESSAGE_UINT64_UINT64(
+                NET_EBPF_EXT_TRACELOG_LEVEL_VERBOSE,
+                NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION,
+                "Added WFP filter: ",
+                index,
+                local_filter_id);
         }
     }
 

netebpfext/net_ebpf_ext.h

@@ -75,13 +75,30 @@ typedef struct _net_ebpf_extension_wfp_filter_parameters_array
 /**
  * "Base class" for all WFP filter contexts used by net ebpf extension hooks.
  */
+
+typedef enum _net_ebpf_ext_wfp_filter_state
+{
+    NET_EBPF_EXT_WFP_FILTER_ADDED = 1,
+    NET_EBPF_EXT_WFP_FILTER_DELETED = 2,
+
+} net_ebpf_ext_wfp_filter_state_t;
+
+typedef struct _net_ebpf_ext_wfp_filter_id
+{
+    wchar_t* name;
+    uint64_t id;
+    net_ebpf_ext_wfp_filter_state_t state;
+} net_ebpf_ext_wfp_filter_id_t;
+
 typedef struct _net_ebpf_extension_wfp_filter_context
 {
     volatile long reference_count;                                ///< Reference count.
     const struct _net_ebpf_extension_hook_client* client_context; ///< Pointer to hook NPI client.
-    uint64_t* filter_ids;                                         ///< Array of WFP filter Ids.
+
-    uint32_t filter_ids_count;                                    ///< Number of WFP filter Ids.
+    net_ebpf_ext_wfp_filter_id_t* filter_ids; ///< Array of WFP filter Ids.
-    bool client_detached : 1;                                     ///< True if client has detached.
+    uint32_t filter_ids_count;                ///< Number of WFP filter Ids.
+
+    bool client_detached : 1; ///< True if client has detached.
 } net_ebpf_extension_wfp_filter_context_t;
 
 #define REFERENCE_FILTER_CONTEXT(filter_context)                  \
@@ -94,6 +111,9 @@ typedef struct _net_ebpf_extension_wfp_filter_context
         if (InterlockedDecrement(&(filter_context)->reference_count) == 0) {          \
             net_ebpf_extension_hook_client_leave_rundown(                             \
                 (net_ebpf_extension_hook_client_t*)(filter_context)->client_context); \
+            if ((filter_context)->filter_ids != NULL) {                               \
+                ExFreePool((filter_context)->filter_ids);                             \
+            }                                                                         \
             ExFreePool((filter_context));                                             \
         }                                                                             \
     }
@@ -193,7 +213,7 @@ net_ebpf_extension_add_wfp_filters(
     uint32_t condition_count,
     _In_opt_count_(condition_count) const FWPM_FILTER_CONDITION* conditions,
     _Inout_ net_ebpf_extension_wfp_filter_context_t* filter_context,
-    _Outptr_result_buffer_maybenull_(filter_count) uint64_t** filter_ids);
+    _Outptr_result_buffer_maybenull_(filter_count) net_ebpf_ext_wfp_filter_id_t** filter_ids);
 
 /**
  * @brief Deletes WFP filters with specified filter IDs.
@@ -202,7 +222,8 @@ net_ebpf_extension_add_wfp_filters(
  * @param[in]  filter_ids ID of the filter being deleted.
  */
 void
-net_ebpf_extension_delete_wfp_filters(uint32_t filter_count, _Frees_ptr_ _In_count_(filter_count) uint64_t* filter_ids);
+net_ebpf_extension_delete_wfp_filters(
+    uint32_t filter_count, _Frees_ptr_ _In_count_(filter_count) net_ebpf_ext_wfp_filter_id_t* filter_ids);
 
 // eBPF WFP Provider GUID.
 // ddb851f5-841a-4b77-8a46-bb7063e9f162