From e4e6b167e23d50a0a1772f718b261113a167d9e4 Mon Sep 17 00:00:00 2001 From: Mounir IDRASSI Date: Mon, 15 Sep 2025 14:34:22 +0900 Subject: [PATCH] Windows driver: add safe MapIrpDataBuffer function to prevent rare BSOD when irp->MdlAddress is NULL Introduce MapIrpDataBuffer to handle Direct/Buffered/Neither I/O, probing & locking pages and allocating a temp MDL when needed. Replace blind MmGetSystemAddressForMdlSafe usage. clean up TempUserMdl in OnItemCompleted to avoid crashes when MdlAddress is NULL. Issue reported at https://sourceforge.net/p/veracrypt/discussion/technical/thread/e43bde8d86/ --- src/Driver/EncryptedIoQueue.c | 114 +++++++++++++++++++++++++++++++--- src/Driver/EncryptedIoQueue.h | 1 + 2 files changed, 107 insertions(+), 8 deletions(-) diff --git a/src/Driver/EncryptedIoQueue.c b/src/Driver/EncryptedIoQueue.c index bf11092d..e1cd7bbc 100644 --- a/src/Driver/EncryptedIoQueue.c +++ b/src/Driver/EncryptedIoQueue.c @@ -19,6 +19,86 @@ #include "Volumes.h" #include +// Returns STATUS_SUCCESS on success and sets *outVa. +// On failure, returns STATUS_INVALID_USER_BUFFER or STATUS_INSUFFICIENT_RESOURCES +// and leaves *outVa as NULL. If *outTempMdl not NULL, the caller must unlock/free it at completion. +__drv_maxIRQL(APC_LEVEL) static NTSTATUS + MapIrpDataBuffer( + _In_ PIRP irp, + _In_ BOOL isWriteIRP, // TRUE for IRP_MJ_WRITE (we READ from caller buffer) + _In_ ULONG length, + _Outptr_result_bytebuffer_(length) PUCHAR *outVa, + _Outptr_result_maybenull_ PMDL *outTempMdl) +{ + ULONG mapFlags = HighPagePriority | MdlMappingNoExecute; + PUCHAR va = NULL; + + ASSERT(outVa && outTempMdl); + *outVa = NULL; + *outTempMdl = NULL; + + ASSERT(KeGetCurrentIrql() <= APC_LEVEL); + + if (length == 0) + return STATUS_INVALID_PARAMETER; + + // If this is a WRITE IRP we only read from caller’s buffer: ask for a no-write mapping. + if (isWriteIRP) + mapFlags |= MdlMappingNoWrite; + + // --- Direct I/O --- + if (irp->MdlAddress) + { + if (MmGetMdlByteCount(irp->MdlAddress) < length) + return STATUS_INVALID_USER_BUFFER; // caller asked for more than mapped + + va = (PUCHAR)MmGetSystemAddressForMdlSafe(irp->MdlAddress, mapFlags); + if (!va) + return STATUS_INSUFFICIENT_RESOURCES; // low PTEs, etc. + + *outVa = va; + return STATUS_SUCCESS; + } + + // --- Buffered I/O --- + if (irp->AssociatedIrp.SystemBuffer) + { + *outVa = (PUCHAR)irp->AssociatedIrp.SystemBuffer; + return STATUS_SUCCESS; + } + + // --- Neither I/O --- + if (!irp->UserBuffer) + return STATUS_INVALID_USER_BUFFER; + + PMDL mdl = IoAllocateMdl(irp->UserBuffer, length, FALSE, FALSE, NULL); + if (!mdl) + return STATUS_INSUFFICIENT_RESOURCES; + + __try + { + // For WRITE IRPs we read from user => IoReadAccess. + // For READ IRPs we write to user => IoWriteAccess. + MmProbeAndLockPages(mdl, irp->RequestorMode, isWriteIRP ? IoReadAccess : IoWriteAccess); + } + __except (EXCEPTION_EXECUTE_HANDLER) + { + IoFreeMdl(mdl); + return STATUS_INVALID_USER_BUFFER; // bad pointer/range/rights + } + + va = (PUCHAR)MmGetSystemAddressForMdlSafe(mdl, mapFlags); + if (!va) + { + MmUnlockPages(mdl); + IoFreeMdl(mdl); + return STATUS_INSUFFICIENT_RESOURCES; + } + + *outTempMdl = mdl; + *outVa = va; + return STATUS_SUCCESS; +} static void AcquireBufferPoolMutex (EncryptedIoQueue *queue) { @@ -160,6 +240,12 @@ static void DecrementOutstandingIoCount (EncryptedIoQueue *queue) static void OnItemCompleted (EncryptedIoQueueItem *item, BOOL freeItem) { + if (item->TempUserMdl) { + MmUnlockPages(item->TempUserMdl); + IoFreeMdl(item->TempUserMdl); + item->TempUserMdl = NULL; + } + DecrementOutstandingIoCount (item->Queue); IoReleaseRemoveLock (&item->Queue->RemoveLock, item->OriginalIrp); @@ -700,6 +786,7 @@ static VOID MainThreadProc (PVOID threadArg) item->Queue = queue; item->OriginalIrp = irp; + item->TempUserMdl = NULL; item->Status = STATUS_SUCCESS; IoSetCancelRoutine (irp, NULL); @@ -764,11 +851,17 @@ static VOID MainThreadProc (PVOID threadArg) { UINT64_STRUCT dataUnit; - dataBuffer = (PUCHAR) MmGetSystemAddressForMdlSafe (irp->MdlAddress, (HighPagePriority | MdlMappingNoExecute)); - if (!dataBuffer) + dataBuffer = NULL; + NTSTATUS mapStatus = MapIrpDataBuffer( + irp, + FALSE, + item->OriginalLength, + &dataBuffer, + &item->TempUserMdl); + if (!NT_SUCCESS(mapStatus)) { TCfree (buffer); - CompleteOriginalIrp (item, STATUS_INSUFFICIENT_RESOURCES, 0); + CompleteOriginalIrp (item, mapStatus, 0); continue; } @@ -883,12 +976,17 @@ static VOID MainThreadProc (PVOID threadArg) CompleteOriginalIrp (item, STATUS_MEDIA_WRITE_PROTECTED, 0); continue; } - - dataBuffer = (PUCHAR) MmGetSystemAddressForMdlSafe (irp->MdlAddress, (HighPagePriority | MdlMappingNoExecute)); - - if (dataBuffer == NULL) + + dataBuffer = NULL; + NTSTATUS mapStatus = MapIrpDataBuffer( + irp, + item->Write, + item->OriginalLength, + &dataBuffer, + &item->TempUserMdl); + if (!NT_SUCCESS(mapStatus)) { - CompleteOriginalIrp (item, STATUS_INSUFFICIENT_RESOURCES, 0); + CompleteOriginalIrp (item, mapStatus, 0); continue; } diff --git a/src/Driver/EncryptedIoQueue.h b/src/Driver/EncryptedIoQueue.h index e3e19d37..ecd34bd6 100644 --- a/src/Driver/EncryptedIoQueue.h +++ b/src/Driver/EncryptedIoQueue.h @@ -156,6 +156,7 @@ typedef struct ULONG OriginalLength; LARGE_INTEGER OriginalOffset; NTSTATUS Status; + PMDL TempUserMdl; // NULL if none. Used in MapIrpDataBuffer logic #ifdef TC_TRACE_IO_QUEUE LARGE_INTEGER OriginalIrpOffset;