Unclean user exit causes kernel service errors #92
Conversation
…le() 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.
|
This sounds a lot like what I was referring to in #90 @basil00. I've long known that manually unloading the driver via |
|
Thanks for finding this @FullyArticulate. There are a few issues:
It seems that the WFPsampler drivers do not wrap multiple calls to |
|
All FWPM calls fail once the process exits uncleanly. As best I can determine, the handle that the process uses to do RPC with FWPM is closed before Divert's cleanup code is called (cleanup is called within the context of the user process). It doesn't matter whether the FWPM cleanup was done within a transaction wrapper or not--the engine handle is gone. Because everything put into FWPM within the context of the user process is within a DYNAMIC session, there's no penalty for not cleanly deleting everything from FWPM--it happens automatically when the RPC connection was closed. The exception is the Fwps stuff, which stands alone from the FWPM session. I believe my patch is correct for both clean and unclean closing modes. I've not looked closely at the WFPsampler code, but it doesn't look like there's any way for FWPM calls to occur after an unclean cleanup occurs. |
|
Thanks @FullyArticulate that sounds like a reasonable analysis. I will accept your patch but need to set up some form of CLA arrangement first since I license WinDivert commercially. |
|
I have no idea what's entailed in that. If it makes it easier, feel free to treat the patch as public domain, or "reverse engineer" my patch to get the bug fixed. |
|
CLA is a contributor license agreement and AFAIK it's not really necessary
but I do understand, as a project owner to desire to maintain the sanity of
your project.
Check this out
https://julien.ponge.org/blog/developer-certificate-of-origin-versus-contributor-license-agreements/
as a possible alternative. Linux kernel project uses DCO instead of CLA.
…On Aug 16, 2017 2:34 PM, "FullyArticulate" ***@***.***> wrote:
I have no idea what's entailed in that. If it makes it easier, feel free
to treat the patch as public domain, or "reverse engineer" my patch to get
the bug fixed.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#92 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKtty12cvteDH2CWoMDkwbU0JWzmhTEFks5sYzY8gaJpZM4O30jJ>
.
|
Very sorry for the inconvenience. This is something I've been meaning to set up for some time but never bothered since this project very rarely gets PRs. It should be set up now. The CLA is just the standard SAP version with WinDivert/basil00 replacing "SAP". If you don't want to, or can't be bothered to sign, that is fine and I'll "reverse engineer" the fix. However, I'd prefer to merge the pull request as is to properly credit @FullyArticulate for finding the fix. This matter is not urgent since I cannot release new WinDivert binaries anyway, thanks to #53. |
|
Oh, I just had to click something? I had imagined finding legal docs, signing, scanning, faxing, etc. :-) It should be done now. |
|
No is just a click. Funny the automated cla-assistant check was still failing...but I can manually check that @FullyArticulate has signed so it does not matter. I suspect the automated check is failing because the system thinks @Azerton also needs to sign. |
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.