From e138f142caa6b1bb9fe5acc496d4a04253c61959 Mon Sep 17 00:00:00 2001 From: Michael Agun Date: Thu, 30 Jul 2026 00:59:19 -0700 Subject: [PATCH 1/2] Improve WFP filter-delete mock fidelity and add delete fault injection The FWP user-mode mock diverged from real WFP filter-delete behavior: - The delete notification (FWPS_CALLOUT_NOTIFY_DELETE_FILTER) was raised with filterId = 0, so a callout that identifies the deleted filter by id could not match it. - remove_fwpm_filter asserted the callout was still registered and always raised the delete notification, whereas real WFP raises no delete notification once the callout function has been unregistered. Changes: - Set fwps_filter.filterId on the add and delete notifications. - Raise the delete notification only when the callout is still registered. - Add test-only fault injection (usersim_fwp_set_filter_delete_failure_count) plus usersim_fwp_get_fwpm_filter_count and usersim_fwp_clear_fwpm_filters to simulate and inspect a filter whose delete fails without removing the filter or notifying. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c5be9327-3fe0-4580-b5c1-17b6c052bbb2 --- inc/usersim/fwp_test.h | 13 +++++++++++ src/fwp_um.cpp | 24 +++++++++++++++++++ src/fwp_um.h | 53 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/inc/usersim/fwp_test.h b/inc/usersim/fwp_test.h index c9dfbad..f5574c4 100644 --- a/inc/usersim/fwp_test.h +++ b/inc/usersim/fwp_test.h @@ -65,6 +65,19 @@ USERSIM_API void usersim_fwp_set_sublayer_guids( _In_ const GUID& default_sublayer, _In_ const GUID& connect_v4_sublayer, _In_ const GUID& connect_v6_sublayer); +// Test-only: fail the next 'count' WFP FwpmFilterDeleteById calls without removing the filter or issuing a delete +// notification, reproducing the DELETE_FAILED reference-leak scenario. Pass 0 to clear the injection. +USERSIM_API void +usersim_fwp_set_filter_delete_failure_count(uint32_t count); + +// Test-only: number of WFP filters currently present in the simulated engine. +USERSIM_API uint32_t +usersim_fwp_get_fwpm_filter_count(); + +// Test-only: remove any WFP filters left in the simulated engine (cleanup after fault-injection tests). +USERSIM_API void +usersim_fwp_clear_fwpm_filters(); + USERSIM_API void usersim_fwp_sock_ops_v4_remove_flow_context(_In_ uint64_t flow_id); diff --git a/src/fwp_um.cpp b/src/fwp_um.cpp index cdd3142..21c1b14 100644 --- a/src/fwp_um.cpp +++ b/src/fwp_um.cpp @@ -534,6 +534,12 @@ _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmFilterDeleteById0(_In_ HANDLE en // Skip fault injection for this API because return failure status requires to remove filter from the list. auto& engine = *reinterpret_cast(engine_handle); + // Test-only fault injection: fail the delete without removing the filter or issuing a delete notification, + // reproducing the field DELETE_FAILED reference-leak scenario. + if (engine.consume_filter_delete_failure()) { + return (NTSTATUS)STATUS_UNSUCCESSFUL; + } + if (engine.remove_fwpm_filter(id)) { return STATUS_SUCCESS; } else { @@ -1124,6 +1130,24 @@ usersim_fwp_set_sublayer_guids( fwp_engine_t::get()->set_sublayer_guids(default_sublayer, connect_v4_sublayer, connect_v6_sublayer); } +void +usersim_fwp_set_filter_delete_failure_count(uint32_t count) +{ + fwp_engine_t::get()->set_filter_delete_failure_count(count); +} + +uint32_t +usersim_fwp_get_fwpm_filter_count() +{ + return (uint32_t)fwp_engine_t::get()->get_fwpm_filter_count(); +} + +void +usersim_fwp_clear_fwpm_filters() +{ + fwp_engine_t::get()->clear_fwpm_filters(); +} + void usersim_fwp_sock_ops_v4_remove_flow_context( _In_ uint64_t flow_id) diff --git a/src/fwp_um.h b/src/fwp_um.h index 6a1d30f..f9323e1 100644 --- a/src/fwp_um.h +++ b/src/fwp_um.h @@ -139,6 +139,7 @@ typedef class fwp_engine_t callout = get_fwps_callout(&filter->action.calloutKey); CXPLAT_DEBUG_ASSERT(callout != nullptr); + fwps_filter.filterId = id; fwps_filter.context = filter->rawContext; } @@ -158,8 +159,10 @@ typedef class fwp_engine_t exclusive_lock_t l(lock); for (auto& it : fwpm_filters) { if (it.first == id) { + // May be null if the callout function has already been unregistered (e.g., during driver + // unload); in that case WFP delivers no delete notification (handled below). callout = get_fwps_callout(&it.second.action.calloutKey); - CXPLAT_DEBUG_ASSERT(callout != nullptr); + fwps_filter.filterId = id; fwps_filter.context = it.second.rawContext; break; } @@ -168,14 +171,53 @@ typedef class fwp_engine_t return_value = fwpm_filters.erase(id) == 1; } - CXPLAT_DEBUG_ASSERT(callout != nullptr); - __analysis_assume(callout != nullptr); - // Invoke filter delete notification callback. - callout->notifyFn(FWPS_CALLOUT_NOTIFY_DELETE_FILTER, &callout->calloutKey, &fwps_filter); + // If the callout function is still registered, deliver the delete notification as real WFP does. Once the + // callout has been unregistered (e.g., during driver unload), WFP delivers no delete notification. + if (callout != nullptr) { + callout->notifyFn(FWPS_CALLOUT_NOTIFY_DELETE_FILTER, &callout->calloutKey, &fwps_filter); + } return return_value; } + // Test-only: remove any WFP filters left in the engine (used to clean up after fault-injection tests that + // intentionally leave filters undeletable). Does not issue notifications. + void + clear_fwpm_filters() + { + exclusive_lock_t l(lock); + fwpm_filters.clear(); + } + + // Test-only fault injection: fail the next 'count' FwpmFilterDeleteById calls without removing the filter or + // issuing a delete notification, reproducing the WFP DELETE_FAILED reference-leak scenario. + void + set_filter_delete_failure_count(uint32_t count) + { + exclusive_lock_t l(lock); + _filter_delete_failure_count = count; + } + + // Returns true (and consumes one) if the next FwpmFilterDeleteById call should be failed for fault injection. + bool + consume_filter_delete_failure() + { + exclusive_lock_t l(lock); + if (_filter_delete_failure_count > 0) { + _filter_delete_failure_count--; + return true; + } + return false; + } + + // Test-only: number of WFP filters currently present in the engine. + size_t + get_fwpm_filter_count() + { + shared_lock_t l(lock); + return fwpm_filters.size(); + } + _Requires_lock_not_held_(this->lock) void add_fwpm_provider(_In_ const FWPM_PROVIDER* provider) { UNREFERENCED_PARAMETER(provider); @@ -336,6 +378,7 @@ typedef class fwp_engine_t std::shared_mutex lock; uint32_t next_id = 1; uint32_t next_flow_id = 1; + uint32_t _filter_delete_failure_count = 0; // Test-only WFP filter delete fault-injection counter. std::unordered_map fwps_callouts; std::unordered_map fwpm_callouts; std::unordered_map fwpm_filters; From b52c589a4c1f51359e591642b53a4bd5de72aa91 Mon Sep 17 00:00:00 2001 From: Michael Agun Date: Mon, 3 Aug 2026 10:07:43 -0700 Subject: [PATCH 2/2] Document the WFP filter-delete fault-injection test hook Clarify the comments around usersim_fwp_set_filter_delete_failure_count per review feedback: the public declaration in fwp_test.h now describes what the hook does, how it differs from the generic cxplat fault-injection harness (deterministic, opt-in, independent of cxplat_fault_injection_is_enabled()), and how to use it. The injection site in FwpmFilterDeleteById0 and the engine helper in fwp_um.h are reduced to short notes that cross-reference it. Comments only; no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a991e01-4799-44d5-8290-68a48c363260 --- inc/usersim/fwp_test.h | 13 +++++++++++-- src/fwp_um.cpp | 6 +++--- src/fwp_um.h | 4 ++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/inc/usersim/fwp_test.h b/inc/usersim/fwp_test.h index f5574c4..a636d8a 100644 --- a/inc/usersim/fwp_test.h +++ b/inc/usersim/fwp_test.h @@ -65,8 +65,17 @@ USERSIM_API void usersim_fwp_set_sublayer_guids( _In_ const GUID& default_sublayer, _In_ const GUID& connect_v4_sublayer, _In_ const GUID& connect_v6_sublayer); -// Test-only: fail the next 'count' WFP FwpmFilterDeleteById calls without removing the filter or issuing a delete -// notification, reproducing the DELETE_FAILED reference-leak scenario. Pass 0 to clear the injection. +// Test hook to simulate WFP filter-delete failures. Arms the mock to fail the next 'count' FwpmFilterDeleteById +// calls; each returns a failure status while leaving the filter installed and firing no delete notification, so +// a caller can be exercised against a delete that does not take effect. Pass 0 to disarm. +// +// This is deterministic and opt-in, and is separate from the random cxplat fault-injection harness: it stays +// inert until armed (count > 0), consumes one count per failed delete, and does not depend on the harness being +// enabled. A test that arms it should skip itself when cxplat_fault_injection_is_enabled() is true, so random +// injection does not disturb the exact sequence. Example: +// usersim_fwp_set_filter_delete_failure_count(1); // or UINT32_MAX to fail every delete +// ... run the code under test ... +// usersim_fwp_set_filter_delete_failure_count(0); USERSIM_API void usersim_fwp_set_filter_delete_failure_count(uint32_t count); diff --git a/src/fwp_um.cpp b/src/fwp_um.cpp index 21c1b14..658f225 100644 --- a/src/fwp_um.cpp +++ b/src/fwp_um.cpp @@ -531,11 +531,11 @@ fwp_engine_t::test_cgroup_inet6_listen(_In_ fwp_classify_parameters_t* parameter _IRQL_requires_max_(PASSIVE_LEVEL) NTSTATUS FwpmFilterDeleteById0(_In_ HANDLE engine_handle, _In_ uint64_t id) { - // Skip fault injection for this API because return failure status requires to remove filter from the list. auto& engine = *reinterpret_cast(engine_handle); - // Test-only fault injection: fail the delete without removing the filter or issuing a delete notification, - // reproducing the field DELETE_FAILED reference-leak scenario. + // Filter-delete failures are injected with a dedicated, caller-armed counter rather than the generic + // cxplat_fault_injection_inject_fault() path, so tests can fail specific deletes deterministically. See + // usersim_fwp_set_filter_delete_failure_count (fwp_test.h) for behavior and usage. if (engine.consume_filter_delete_failure()) { return (NTSTATUS)STATUS_UNSUCCESSFUL; } diff --git a/src/fwp_um.h b/src/fwp_um.h index f9323e1..9558e85 100644 --- a/src/fwp_um.h +++ b/src/fwp_um.h @@ -189,8 +189,8 @@ typedef class fwp_engine_t fwpm_filters.clear(); } - // Test-only fault injection: fail the next 'count' FwpmFilterDeleteById calls without removing the filter or - // issuing a delete notification, reproducing the WFP DELETE_FAILED reference-leak scenario. + // Arms the deterministic FwpmFilterDeleteById failure counter (see usersim_fwp_set_filter_delete_failure_count + // in fwp_test.h). Fails the next 'count' deletes; 0 disarms. void set_filter_delete_failure_count(uint32_t count) {