From 504132080a69cde2e9d720b0726624cc762caa62 Mon Sep 17 00:00:00 2001 From: Mike Neuman Date: Tue, 15 Aug 2017 09:32:44 -0700 Subject: [PATCH] Bugfix: If the user's app exits (or crashes) before calling CloseHandle() on the WinDivert handle, the driver's cleanup function cannot make FWPM calls. The cleanup function detects this FWPM error and returns without ever calling the Fwps cleanup functions (which are independent of fwpm). As a result, the driver will not unregister the callouts, which leaves the Windows kernel confused. You can reproduce the problem by having a user app close uncleanly, then "sc stop windivert1.2", then try to re-run the app. You'll get "file not found" when StartService() is called. --- sys/windivert.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sys/windivert.c b/sys/windivert.c index 08404c7..9af8317 100644 --- a/sys/windivert.c +++ b/sys/windivert.c @@ -1133,8 +1133,12 @@ static void windivert_uninstall_callouts(context_t context) status = FwpmTransactionBegin0(context->engine_handle, 0); if (!NT_SUCCESS(status)) { + // If the userspace app closes without closing the handle to + // WinDivert, any actions on engine_handle fail because the + // RPC handle was closed first. So, this path is "normal" if + // the user's app crashed or never closed the WinDivert handle. DEBUG_ERROR("failed to begin WFP transaction", status); - return; + goto unregister_callouts; } for (i = 0; i < WINDIVERT_CONTEXT_MAXLAYERS; i++) { @@ -1162,15 +1166,15 @@ static void windivert_uninstall_callouts(context_t context) if (!NT_SUCCESS(status)) { FwpmTransactionAbort0(context->engine_handle); - return; + goto unregister_callouts; } status = FwpmTransactionCommit0(context->engine_handle); if (!NT_SUCCESS(status)) { DEBUG_ERROR("failed to commit WFP transaction", status); - return; + //fallthrough } - +unregister_callouts: for (i = 0; i < WINDIVERT_CONTEXT_MAXLAYERS; i++) { FwpsCalloutUnregisterByKey0(&context->callout_guid[i]);