Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions src/Driver/DriveFilter.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ static uint8 BootLoaderFingerprint[WHIRLPOOL_DIGESTSIZE + SHA512_DIGESTSIZE];
static BOOL CrashDumpEnabled = FALSE;
static BOOL HibernationEnabled = FALSE;

static BOOL LegacyHibernationDriverFilterActive = FALSE;
static uint8 *HibernationWriteBuffer = NULL;
static MDL *HibernationWriteBufferMdl = NULL;

static uint32 HibernationPreventionCount = 0;

static BootEncryptionSetupRequest SetupRequest;
Expand Down Expand Up @@ -972,7 +968,12 @@ static NTSTATUS DispatchPower (PDEVICE_OBJECT DeviceObject, PIRP Irp, DriveFilte
&& irpSp->MinorFunction == IRP_MN_SET_POWER
&& irpSp->Parameters.Power.ShutdownType == PowerActionHibernate)
{
while (SendDeviceIoControlRequest (RootDeviceObject, TC_IOCTL_ABORT_BOOT_ENCRYPTION_SETUP, NULL, 0, NULL, 0) == STATUS_INSUFFICIENT_RESOURCES);
int retries = 50;
while (retries-- > 0
&& SendDeviceIoControlRequest (RootDeviceObject, TC_IOCTL_ABORT_BOOT_ENCRYPTION_SETUP, NULL, 0, NULL, 0) == STATUS_INSUFFICIENT_RESOURCES)
Comment on lines +972 to +973
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
TCSleep (100);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}

// Dismount the system drive on shutdown on Windows 7 and later
Expand Down Expand Up @@ -1208,7 +1209,21 @@ void ReopenBootVolumeHeader (PIRP irp)
}


// Legacy Windows XP/2003 hibernation dump filter
/*
* Legacy Windows XP/2003 hibernation dump filter.
*
* DISABLED: This code is not active — LoadImageNotifyRoutine was never
* registered via PsSetLoadImageNotifyRoutine, so none of these functions
* are reachable at runtime. Additionally the code has known issues:
* - HibernationWriteBuffer / HibernationWriteBufferMdl are never allocated
* (NULL dereference if reached)
* - MmInitializeMdl is called in a context that may run at HIGH_LEVEL
* - dataMdl->MappedSystemVa is accessed without checking MDL flags
*
* Kept behind #if 0 for historical reference. The modern hibernation
* encryption path is in DumpFilter.c (Vista+ dump filter API).
*/
#if 0

typedef NTSTATUS (*HiberDriverWriteFunctionA) (ULONG arg0, PLARGE_INTEGER writeOffset, PMDL dataMdl, PVOID arg3);
typedef NTSTATUS (*HiberDriverWriteFunctionB) (PLARGE_INTEGER writeOffset, PMDL dataMdl);
Expand Down Expand Up @@ -1300,7 +1315,7 @@ static NTSTATUS HiberDriverWriteFunctionFilter (int filterNumber, PLARGE_INTEGER

if (writeB)
return (*OriginalHiberDriverWriteFunctionsB[filterNumber]) (writeOffset, encryptedDataMdl);

return (*OriginalHiberDriverWriteFunctionsA[filterNumber]) (arg0WriteA, writeOffset, encryptedDataMdl, arg3WriteA);
}

Expand Down Expand Up @@ -1475,6 +1490,8 @@ static VOID LoadImageNotifyRoutine (PUNICODE_STRING fullImageName, HANDLE proces
KeLowerIrql (origIrql);
}

#endif /* Legacy XP/2003 hibernation filter */


static VOID SetupThreadProc (PVOID threadArg)
{
Expand Down
23 changes: 19 additions & 4 deletions src/Driver/DumpFilter.c
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,26 @@ static NTSTATUS DumpFilterWrite (PFILTER_EXTENSION filterExtension, PLARGE_INTEG
if ((offset & (ENCRYPTION_DATA_UNIT_SIZE - 1)) != 0)
TC_BUG_CHECK (STATUS_INVALID_PARAMETER);

// Require either a valid mapping or a nonpaged system VA we can read at HIGH_LEVEL.
if ((writeMdl->MdlFlags & (MDL_MAPPED_TO_SYSTEM_VA | MDL_SOURCE_IS_NONPAGED_POOL)) == 0)
TC_BUG_CHECK(STATUS_INVALID_PARAMETER);
// Resolve the system VA we can safely read at HIGH_LEVEL.
// MDL_SOURCE_IS_NONPAGED_POOL: original VA (StartVa + ByteOffset) is a kernel VA.
// 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.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (writeMdl->MdlFlags & MDL_SOURCE_IS_NONPAGED_POOL)
{
writeBuffer = MmGetMdlVirtualAddress (writeMdl);
}
else if (writeMdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA)
{
writeBuffer = writeMdl->MappedSystemVa;
}
else
{
// Unrecognized MDL type — include MdlFlags in bugcheck param3 for diagnosis.
KeBugCheckEx (SECURITY_SYSTEM, __LINE__, (ULONG_PTR) STATUS_INVALID_PARAMETER, (ULONG_PTR) writeMdl->MdlFlags, 'VC');
}

writeBuffer = MmGetMdlVirtualAddress (writeMdl);
if (!writeBuffer)
TC_BUG_CHECK (STATUS_INVALID_PARAMETER);

Expand Down