Fix crash from stale TrustedInstaller PID on elevated tweaks#130
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CreateProcessAsTrustedInstallercaches the TrustedInstaller service PID once at app startup (TrustedInstaller.StartTrustedInstallerService(), called only fromLoadingWindow.xaml.cs) and never refreshes it. TrustedInstaller is a demand-start Windows service that stops itself when idle, so any elevated tweak run after that happens callsOpenProcesson a PID that no longer exists, which fails withERROR_INVALID_PARAMETER(87) and crashes the app with an unhandledWin32Exception. This matches #127 exactly (same exception type/class/method, different call site).Why this isn't already fixed
#127's own thread shows a fix already shipped in 5.5.2 (
f757319, "fix buffer size overflow in CreateProcessAsTrustedInstaller" - same change as17b49beonmain, just a different SHA from the squash-merge). That's a real fix, but for a different bug in the same function (a marshaling/buffer-size mismatch), not this one. After that release shipped,DkmS1953hit the identical crash from a third call site (ServicesTweaks.ApplyTweaks), on top of the original report'sConfidentialityTweaksand this PR'sWindowsDefender.Deactivate. Three independent callers hitting the same stale-PID failure through the sharedCreateProcessAsTrustedInstallerhelper is why the fix belongs there, not in any one caller.Fix
Before opening the parent process handle, check whether the cached PID still refers to a running process (
Process.GetProcessById+HasExited). If not, restart the TrustedInstaller service and use the refreshed PID for that attempt. This fixes the shared helper, so it covers every caller (Defender, Confidentiality, Services, etc.), not just one tweak.Verification
No test project exists in this repo, so here's a runnable repro proving the exact failure mode this fix addresses:
Also verified:
msbuild GTweak.sln /t:Rebuild /p:Configuration=Releasesucceeds both before and after this change (0 errors).IsProcessRunningguard correctly returnsfalsefor a PID with no live process (confirmed viaProcess.GetProcessByIdthrowingArgumentException), the exact condition that triggers the refresh path.Fixes #127