Fix hibernation crash on fresh Windows 11 25H2 (BSOD Event 41)#1671
Fix hibernation crash on fresh Windows 11 25H2 (BSOD Event 41)#1671audriusbuika wants to merge 2 commits intoveracrypt:masterfrom
Conversation
|
@audriusbuika Some remarks:
Minor style note: the new comments contain Unicode punctuation (em-dash). Using ASCII hyphens/apostrophes would avoid GitHub's hidden/Unicode warning noise. My recommendation would be to keep the |
idrassi
left a comment
There was a problem hiding this comment.
The DumpFilter.c MDL address selection looks like the right core fix for the hibernation crash. I am requesting changes because the DispatchPower retry/sleep change introduces separate power-path concerns: it may sleep without a local IRQL guarantee and the bounded retry can proceed without proving that boot encryption setup was actually aborted. Please either rework that part to preserve the existing hibernation/setup invariant safely, or split it out so the MDL fix can be reviewed independently.
src/Driver/DriveFilter.c
Outdated
| while (retries-- > 0 | ||
| && SendDeviceIoControlRequest (RootDeviceObject, TC_IOCTL_ABORT_BOOT_ENCRYPTION_SETUP, NULL, 0, NULL, 0) == STATUS_INSUFFICIENT_RESOURCES) | ||
| { | ||
| TCSleep (100); |
There was a problem hiding this comment.
Please avoid sleeping directly in this DispatchPower path unless the IRQL guarantee is explicit. This branch is reached directly from the incoming IRP_MJ_POWER dispatch path. I do not see a local worker-item handoff or other guarantee that it runs at PASSIVE_LEVEL. TCSleep wraps
KeDelayExecutionThread and requires IRQL <= APC_LEVEL. For hibernation-related power paths, the stack may have cleared DO_POWER_PAGABLE, so this needs either an explicit IRQL-safe implementation or a move to a path that is guaranteed to run at a waitable IRQL.
src/Driver/DriveFilter.c
Outdated
| while (retries-- > 0 | ||
| && SendDeviceIoControlRequest (RootDeviceObject, TC_IOCTL_ABORT_BOOT_ENCRYPTION_SETUP, NULL, 0, NULL, 0) == STATUS_INSUFFICIENT_RESOURCES) |
There was a problem hiding this comment.
This bounded retry can leave boot encryption setup running. STATUS_INSUFFICIENT_RESOURCES from SendDeviceIoControlRequest() can be returned before the IOCTL reaches AbortBootEncryptionSetup() at all, for example if the work item or internal IOCTL IRP cannot be allocated. After 50 attempts this code falls through even though EncryptionSetupThreadAbortRequested may still be false and SetupInProgress may still be true. That conflicts with the invariant already documented in DumpFilterWrite that hibernation should always abort the setup thread before dump writes proceed. Please preserve that invariant by proving the setup thread has stopped before continuing, or make the failure path explicit instead of silently proceeding as if the abort completed.
| // MDL_MAPPED_TO_SYSTEM_VA: MappedSystemVa is the correct kernel VA (StartVa | ||
| // may point elsewhere, e.g. user-mode or stale VA). | ||
| // Windows 11 25H2+ dump stacks may provide mapped MDLs that are NOT from nonpaged | ||
| // pool, so we must prefer MappedSystemVa when available. |
There was a problem hiding this comment.
Small clarification: this comment says we must prefer MappedSystemVa when available, but the code below gives MDL_SOURCE_IS_NONPAGED_POOL precedence over MDL_MAPPED_TO_SYSTEM_VA. If that precedence is intentional, please adjust the comment so it matches the implementation. If MappedSystemVa should really win whenever MDL_MAPPED_TO_SYSTEM_VA is set, the checks should be reordered.
… the risky sleep.
solved problem: #1668