[netebpfext] Add deleted filter contexts to zombie list for debugging…

… purposes (microsoft#4003) * add to zombie list (untested * fix * fix * CR feedback * CR comments * cleanup --------- Co-authored-by: Alan Jowett <alanjo@microsoft.com> Co-authored-by: Anurag Saxena <43585259+saxena-anurag@users.noreply.github.com>
Alan-Jowett · Dec 17, 2024 · 20334ef · 20334ef
1 parent 571761b
commit 20334ef
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 14 deletions.
diff --git a/netebpfext/net_ebpf_ext.c b/netebpfext/net_ebpf_ext.c
@@ -34,6 +34,12 @@ static bool _net_ebpf_xdp_providers_registered = false;
 static bool _net_ebpf_bind_providers_registered = false;
 static bool _net_ebpf_sock_addr_providers_registered = false;
 static bool _net_ebpf_sock_ops_providers_registered = false;
+#if !defined(NDEBUG)
+// Global objects used to store filter contexts that are being cleaned up. This is currently only used in debug
+// contexts.
+EX_SPIN_LOCK _net_ebpf_filter_zombie_list_lock = {0};
+_Guarded_by_(_net_ebpf_filter_zombie_list_lock) static LIST_ENTRY _net_ebpf_filter_zombie_list = {0};
+#endif
 
 static net_ebpf_ext_sublayer_info_t _net_ebpf_ext_sublayers[] = {
     {&EBPF_DEFAULT_SUBLAYER, L"EBPF Sub-Layer", L"Sub-Layer for use by eBPF callouts", 0, SUBLAYER_WEIGHT_MAXIMUM},
@@ -902,6 +908,10 @@ net_ebpf_ext_register_providers()
     }
     _net_ebpf_sock_ops_providers_registered = true;
 
+#if !defined(NDEBUG)
+    InitializeListHead(&_net_ebpf_filter_zombie_list);
+#endif
+
 Exit:
     if (!NT_SUCCESS(status)) {
         net_ebpf_ext_unregister_providers();
@@ -996,4 +1006,24 @@ net_ebpf_ext_remove_client_context(
     }
 
     ExReleaseSpinLockExclusive(&filter_context->lock, old_irql);
-}
+}
+
+#if !defined(NDEBUG)
+void
+net_ebpf_ext_add_filter_context_to_zombie_list(_Inout_ net_ebpf_extension_wfp_filter_context_t* filter_context)
+{
+    KIRQL old_irql = ExAcquireSpinLockExclusive(&_net_ebpf_filter_zombie_list_lock);
+    InsertHeadList(&_net_ebpf_filter_zombie_list, &filter_context->link);
+    ExReleaseSpinLockExclusive(&_net_ebpf_filter_zombie_list_lock, old_irql);
+}
+
+void
+net_ebpf_ext_remove_filter_context_from_zombie_list(_Inout_ net_ebpf_extension_wfp_filter_context_t* filter_context)
+{
+    if (!IsListEmpty(&filter_context->link)) {
+        KIRQL old_irql = ExAcquireSpinLockExclusive(&_net_ebpf_filter_zombie_list_lock);
+        RemoveEntryList(&filter_context->link);
+        ExReleaseSpinLockExclusive(&_net_ebpf_filter_zombie_list_lock, old_irql);
+    }
+}
+#endif
diff --git a/netebpfext/net_ebpf_ext.h b/netebpfext/net_ebpf_ext.h
@@ -143,19 +143,40 @@ typedef struct _net_ebpf_extension_wfp_filter_context
     HANDLE wfp_engine_handle;  ///< WFP engine handle.
 } net_ebpf_extension_wfp_filter_context_t;
 
-#define CLEAN_UP_FILTER_CONTEXT(filter_context)                   \
-    if ((filter_context) != NULL) {                               \
-        if ((filter_context)->filter_ids != NULL) {               \
-            ExFreePool((filter_context)->filter_ids);             \
-        }                                                         \
-        if ((filter_context)->client_contexts != NULL) {          \
-            ExFreePool((filter_context)->client_contexts);        \
-        }                                                         \
-        if ((filter_context)->wfp_engine_handle != NULL) {        \
-            FwpmEngineClose((filter_context)->wfp_engine_handle); \
-        }                                                         \
-        ExFreePool((filter_context));                             \
+// Macro definition of warning suppression for 26100. This is only used in the cleanup context, for which
+// we are the only reference of the memory
+#define PRAGMA_WARNING_PUSH _Pragma("warning(push)")
+// Warning 26100: Variable should be protected by a lock.
+#define PRAGMA_WARNING_SUPPRESS_26100 _Pragma("warning(suppress: 26100)")
+#define PRAGMA_WARNING_POP _Pragma("warning(pop)")
+
+#define CLEAN_UP_FILTER_CONTEXT_COMMON(filter_context)        \
+    if ((filter_context)->filter_ids != NULL) {               \
+        ExFreePool((filter_context)->filter_ids);             \
+    }                                                         \
+    PRAGMA_WARNING_PUSH                                       \
+    PRAGMA_WARNING_SUPPRESS_26100                             \
+    if ((filter_context)->client_contexts != NULL) {          \
+        ExFreePool((filter_context)->client_contexts);        \
+    }                                                         \
+    PRAGMA_WARNING_POP                                        \
+    if ((filter_context)->wfp_engine_handle != NULL) {        \
+        FwpmEngineClose((filter_context)->wfp_engine_handle); \
+    }                                                         \
+    ExFreePool((filter_context));
+
+#if !defined(NDEBUG)
+#define CLEAN_UP_FILTER_CONTEXT(filter_context)                                \
+    if ((filter_context) != NULL) {                                            \
+        net_ebpf_ext_remove_filter_context_from_zombie_list((filter_context)); \
+        CLEAN_UP_FILTER_CONTEXT_COMMON(filter_context);                        \
+    }
+#else
+#define CLEAN_UP_FILTER_CONTEXT(filter_context)         \
+    if ((filter_context) != NULL) {                     \
+        CLEAN_UP_FILTER_CONTEXT_COMMON(filter_context); \
     }
+#endif
 
 #define REFERENCE_FILTER_CONTEXT(filter_context)                  \
     if ((filter_context) != NULL) {                               \
@@ -379,4 +400,22 @@ net_ebpf_ext_remove_client_context(
 ebpf_result_t
 net_ebpf_ext_add_client_context(
     _Inout_ net_ebpf_extension_wfp_filter_context_t* filter_context,
-    _In_ const struct _net_ebpf_extension_hook_client* hook_client);
+    _In_ const struct _net_ebpf_extension_hook_client* hook_client);
+
+#if !defined(NDEBUG)
+/**
+ * @brief Add the filter context to the zombie list.
+ *
+ * @param filter_context Filter context to add to the zombie list.
+ */
+void
+net_ebpf_ext_add_filter_context_to_zombie_list(_Inout_ net_ebpf_extension_wfp_filter_context_t* filter_context);
+
+/**
+ * @brief Remove the filter context from the zombie list.
+ *
+ * @param filter_context Filter context to remove from the zombie list.
+ */
+void
+net_ebpf_ext_remove_filter_context_from_zombie_list(_Inout_ net_ebpf_extension_wfp_filter_context_t* filter_context);
+#endif
diff --git a/netebpfext/net_ebpf_ext_hook_provider.c b/netebpfext/net_ebpf_ext_hook_provider.c
@@ -610,8 +610,14 @@ _Requires_exclusive_lock_held_(provider_context->lock) static void _net_ebpf_ext
 {
     NET_EBPF_EXT_LOG_ENTRY();
 
+    // Remove the list entry from the provider's list of filter contexts.
     RemoveEntryList(&filter_context->link);
 
+#if !defined(NDEBUG)
+    // Add the entry to the zombie list (for debugging purposes)
+    net_ebpf_ext_add_filter_context_to_zombie_list(filter_context);
+#endif
+
     // Release the filter context.
     provider_context->dispatch.delete_filter_context(filter_context);