Skip to content

Windows: bound lower device I/O waits - #1808

Closed
michaelsam94 wants to merge 1 commit into
veracrypt:masterfrom
michaelsam94:codex/fix-lower-device-io-timeout
Closed

Windows: bound lower device I/O waits#1808
michaelsam94 wants to merge 1 commit into
veracrypt:masterfrom
michaelsam94:codex/fix-lower-device-io-timeout

Conversation

@michaelsam94

Copy link
Copy Markdown

Summary

  • replace the unbounded synchronous lower-device read/write wait with an owned asynchronous IRP path
  • add a 120-second timeout that logs the stalled lower-device read/write, cancels the IRP, and returns STATUS_IO_TIMEOUT
  • keep the original caller buffer safe after timeout by using a nonpaged bounce buffer and retaining cleanup context until the lower IRP completes

Rationale

Issue #1807 reports a Windows storage hang where a thread is blocked on a pending write whose current driver is \Driver\veracrypt. The previous helper waited indefinitely for lower-device reads/writes, so a stalled USB/storage/filter-stack request could keep VeraCrypt's queue worker blocked indefinitely as well.

This does not assert that VeraCrypt alone causes the reported Bitdefender/exFAT/USB interaction. It makes VeraCrypt fail the lower I/O upward after a bounded wait instead of waiting forever.

Fixes #1807

Testing

  • git diff --check
  • Not run: Windows driver build/runtime testing is not available in this macOS workspace.

@idrassi

idrassi commented Jul 1, 2026

Copy link
Copy Markdown
Member

Thank you for digging into #1807 and putting together a concrete proposal. The idea of avoiding an indefinitely blocked VeraCrypt worker thread when a lower storage stack is wedged is valid, and the bounce-buffer/refcount direction shows real care.

After reviewing the PR and the regression reports, I don't think this is the right fix to merge for #1807.

My current assessment is that the 1.26.29 regression is more likely tied to recent driver I/O dispatch/queueing changes that landed after 1.26.24, especially:

  • a7ebddc5: the change that moved IRP completion dispatch to CriticalWorkQueue and deferred some error/cancel completion paths;
  • d1f73ce4: the ordered flush barrier change that routes IRP_MJ_FLUSH_BUFFERS through EncryptedIoQueue.

I have pushed a more conservative fix to master: revert the completion-dispatch change back toward the previous stable model (c947e56), and keep ordered flush barriers behind an opt-in driver configuration flag, defaulting them off (f6544a5). That reduces the regression surface on the affected paths while we collect more feedback.

The main reason I don't think this PR covers #1807 is that the dump points to a pending IRP_MJ_WRITE on a mounted non-system volume. For mounted non-system volumes, the backing I/O goes through the host file handle path in EncryptedIoQueue.c, for example:

request->Item->Status = ZwWriteFile (
    queue->HostFileHandle,
    NULL,
    NULL,
    NULL,
    &ioStatus,
    request->Data,
    request->Length,
    &request->Offset,
    NULL);

TCReadWriteDevice, which this PR rewrites, is used on the filter/raw-device helper paths, not on that normal mounted-volume ZwWriteFile path. So this adds a substantial new mechanism without directly addressing the path that appears to hang in #1807.

Even as a separate hardening change, there are several issues that would need to be resolved first:

  1. The context is not fully initialized. In the Windows driver, TCalloc() uses ExAllocatePoolUninitialized(), so fields such as Completed and IoStatus contain undefined data unless explicitly initialized. The PR reads these fields, which can skip the wait, return an undefined status, or copy read data before the lower IRP has completed.

  2. The timeout/completion handoff needs a real state machine. After KeWaitForSingleObject() returns STATUS_TIMEOUT, the lower IRP can still complete before or during IoCancelIrp(). In that race, the caller can return STATUS_IO_TIMEOUT even though the I/O completed. For writes, this ambiguity is especially risky.

  3. The driver-created asynchronous IRP cleanup is incomplete. For direct-I/O devices, IoBuildAsynchronousFsdRequest can create and lock an MDL. Since the completion routine returns STATUS_MORE_PROCESSING_REQUIRED and the PR frees the IRP itself, the MDL must be unlocked and freed explicitly before IoFreeIrp().

  4. IoCancelIrp() does not guarantee that a wedged storage IRP will complete. If the lower stack never completes the IRP, the context, bounce buffer, IRP, and any associated MDL must remain allocated forever to avoid use-after-free. So this can unblock the VeraCrypt thread, but it does not actually recover the lower I/O.

  5. The bounce-buffer design adds allocations and full-length copies to every affected lower-device I/O. On the filter path, that is the hot disk I/O path, so it would be a performance and NonPagedPool pressure regression.

I'm going to close this PR rather than merge it for #1807. A bounded lower-device wait mechanism may still be worth exploring later as a separate hardening change, but it would need a design that handles initialization, MDL cleanup, cancellation/completion ownership, write semantics, and performance impact very carefully.

Thanks again for the investigation and for taking the time to propose a concrete fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows 11 kernel storage hang: Bitdefender bdservicehost.exe stalls on pending write to VeraCryptVolumeY during exFAT USB-volume transfer

2 participants