1
0
mirror of https://github.com/veracrypt/VeraCrypt.git synced 2025-11-11 02:58:02 -06:00

Windows driver: harden UpdateBuffer against integer overflow. Make completion backoff per request (no shared state)

This commit is contained in:
Mounir IDRASSI
2025-09-08 10:36:18 +09:00
parent 062b385a69
commit 0e19cb9223
2 changed files with 20 additions and 23 deletions

View File

@@ -266,22 +266,22 @@ UpdateBuffer(
uint64 sectorLength = DeList->DE[i].Sectors.Length; uint64 sectorLength = DeList->DE[i].Sectors.Length;
uint64 sectorOffset = DeList->DE[i].Sectors.Offset; uint64 sectorOffset = DeList->DE[i].Sectors.Offset;
// Check that sectorOffset and sectorLength are valid within secRegion // Check that sectorOffset and sectorLength are valid within secRegion (guard against overflow)
ULONGLONG regionBoundEnd; // sectorOffset + sectorLength
if (sectorOffset > (uint64)secRegionSize || if (sectorOffset > (uint64)secRegionSize ||
sectorLength == 0 || sectorLength == 0 ||
(sectorOffset + sectorLength) > (uint64)secRegionSize) FAILED(ULongLongAdd(sectorOffset, sectorLength, &regionBoundEnd)) ||
regionBoundEnd > (ULONGLONG)secRegionSize)
{ {
// Invalid entry - skip // Invalid entry - skip
continue; continue;
} }
// Safely compute end = start + length - 1 (guard against overflow) // Safely compute inclusive end = start + length - 1
ULONGLONG secEnd; ULONGLONG secEnd, tmp;
HRESULT hr; if (FAILED(ULongLongAdd(sectorStart, sectorLength, &tmp)) || tmp == 0)
// sectorLength != 0 already checked above continue; // invalid descriptor (overflow or zero)
hr = ULongLongAdd(sectorStart, sectorLength - 1, &secEnd); secEnd = tmp - 1;
if (hr != S_OK)
secEnd = (ULONGLONG)-1;
GetIntersection( GetIntersection(
bufferDiskOffset, bufferLength, bufferDiskOffset, bufferLength,
@@ -293,12 +293,16 @@ UpdateBuffer(
uint64 bufferPos = intersectStart - bufferDiskOffset; uint64 bufferPos = intersectStart - bufferDiskOffset;
uint64 regionPos = sectorOffset + (intersectStart - sectorStart); uint64 regionPos = sectorOffset + (intersectStart - sectorStart);
// Check buffer boundaries // Check buffer boundaries using safe add
if (bufferPos + intersectLength > bufferLength) ULONGLONG bufEndCheck;
if (FAILED(ULongLongAdd(bufferPos, (ULONGLONG)intersectLength, &bufEndCheck)) ||
bufEndCheck > (ULONGLONG)bufferLength)
continue; // Intersection out of buffer range continue; // Intersection out of buffer range
// Check secRegion boundaries // Check secRegion boundaries using safe add
if (regionPos + intersectLength > secRegionSize) ULONGLONG regEndCheck;
if (FAILED(ULongLongAdd(regionPos, (ULONGLONG)intersectLength, &regEndCheck)) ||
regEndCheck > (ULONGLONG)secRegionSize)
continue; // Intersection out of secRegion range continue; // Intersection out of secRegion range
updated = TRUE; updated = TRUE;
@@ -449,12 +453,10 @@ static BOOLEAN HandleCompleteOriginalIrp(EncryptedIoQueue* queue, EncryptedIoReq
&queue->CompletionThreadQueueLock); &queue->CompletionThreadQueueLock);
KeSetEvent(&queue->CompletionThreadQueueNotEmptyEvent, IO_DISK_INCREMENT, FALSE); KeSetEvent(&queue->CompletionThreadQueueNotEmptyEvent, IO_DISK_INCREMENT, FALSE);
// Prefer waiting on a real signal over blind sleeps // Prefer waiting on a real signal over blind sleeps (per-request exponential backoff 1...32ms)
if (queue->OverflowBackoffMs == 0) queue->OverflowBackoffMs = 1; ULONG backoff = 1u << min(request->WiRetryCount - 1, 5); // 1,2,4,8,16,32
LARGE_INTEGER duetime; duetime.QuadPart = -(LONGLONG)queue->OverflowBackoffMs * 10000; LARGE_INTEGER duetime; duetime.QuadPart = -(LONGLONG)backoff * 10000;
// Wait briefly for a pooled work item to be returned
KeWaitForSingleObject(&queue->WorkItemAvailableEvent, Executive, KernelMode, FALSE, &duetime); KeWaitForSingleObject(&queue->WorkItemAvailableEvent, Executive, KernelMode, FALSE, &duetime);
if (queue->OverflowBackoffMs < 32) queue->OverflowBackoffMs <<= 1;
return FALSE; // we requeued so caller must not free request return FALSE; // we requeued so caller must not free request
} }
@@ -469,7 +471,6 @@ static BOOLEAN HandleCompleteOriginalIrp(EncryptedIoQueue* queue, EncryptedIoReq
wi->Item = request->Item; wi->Item = request->Item;
IoQueueWorkItem(wi->WorkItem, CompleteIrpWorkItemRoutine, DelayedWorkQueue, wi); IoQueueWorkItem(wi->WorkItem, CompleteIrpWorkItemRoutine, DelayedWorkQueue, wi);
queue->OverflowBackoffMs = 0;
return TRUE; // caller should free request return TRUE; // caller should free request
} }
@@ -1371,7 +1372,6 @@ retry_preallocated:
KeInitializeSpinLock(&queue->WorkItemLock); KeInitializeSpinLock(&queue->WorkItemLock);
KeInitializeEvent(&queue->WorkItemAvailableEvent, SynchronizationEvent, FALSE); KeInitializeEvent(&queue->WorkItemAvailableEvent, SynchronizationEvent, FALSE);
queue->OverflowBackoffMs = 1;
queue->MaxWorkItems = EncryptionMaxWorkItems; queue->MaxWorkItems = EncryptionMaxWorkItems;
// Enforce [1, VC_MAX_WORK_ITEMS] // Enforce [1, VC_MAX_WORK_ITEMS]
if (queue->MaxWorkItems == 0) if (queue->MaxWorkItems == 0)

View File

@@ -143,9 +143,6 @@ typedef struct
LIST_ENTRY FreeWorkItemsList; LIST_ENTRY FreeWorkItemsList;
KSPIN_LOCK WorkItemLock; KSPIN_LOCK WorkItemLock;
// Backoff (ms) when overflow work-item alloc fails. grows up to 32ms, reset on success
ULONG OverflowBackoffMs;
volatile LONG ActiveWorkItems; volatile LONG ActiveWorkItems;
KEVENT NoActiveWorkItemsEvent; KEVENT NoActiveWorkItemsEvent;