Fix: Reattach stale cached device after backing volume is reattached

[Feng Wang (chemwolf6922)]

Summary of the Pull Request

The current disk attach caching logic only checks the vhd path. When the backing volume is detached than reattached, the check will pass but the stale reference won't work inside the VM.
The backing volume reattaching can happen after bitlocker locking and unlocking #12599, or vhdx detaching then reattaching.

This PR checks if the cached vhd is stale by holding a handle to the vhd file. And checks if it's still valid before reuse. If not, the vhd will be reattached.

PR Checklist

• Closes: WSL crashes when distro is stored on BitLocker drive #12599
• Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
• Tests: Added/updated if needed and all pass
• Localization: All end user facing strings can be localized
• Dev docs: Added/updated if needed
• Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

The proposed regression test was removed because one of its diskpart commands did not work correctly on the pipeline.
The change was tested manually with these steps:

1. Run distro B to keep the VM running.
2. Run distro A. Which has its distro vhd inside another vhd.
3. Terminate distro A.
4. Detach the outer vhd.
5. Attach the outer vhd.
6. Run distro A. Without the fix, this will fail.

[Copilot]

Pull request overview

This PR addresses a WSL2 disk attach caching edge case where a cached VHD attachment can become stale after the backing host volume is detached and later reattached (e.g., BitLocker lock/unlock). It does so by keeping an open handle to the backing VHD file and validating that handle before reusing a cached attachment, plus adds a regression test and consolidates diskpart-based test volume helpers.

Changes:

• Track an open backing-file handle for attached VHDs and invalidate/re-attach when the backing volume is no longer mounted.
• Add shared test helpers for creating/attaching/detaching/deleting test VHD volumes via diskpart.
• Add a new unit test that reproduces detach/reattach of the backing volume and validates the cached VHD is usable afterward.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/windows/service/exe/WslCoreVm.h	Extends per-attached-disk state to retain a backing VHD file handle.
src/windows/service/exe/WslCoreVm.cpp	Opens/stores the backing file handle and checks it before reusing cached VHD LUNs; reattaches when stale.
test/windows/Common.h	Declares shared diskpart-based test volume helper APIs.
test/windows/Common.cpp	Implements diskpart-based test volume helpers used across tests.
test/windows/UnitTests.cpp	Adds a regression test for cached VHD reuse after backing volume remount.
test/windows/DrvFsTests.cpp	Refactors existing tests to use the new shared test volume helpers.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/windows/service/exe/WslCoreVm.cpp:81

• OpenVhdBackingFile opens the VHD with desired access = 0, but the resulting handle is later used with FSCTL_IS_VOLUME_MOUNTED. With a 0-access handle, DeviceIoControl can fail with ERROR_ACCESS_DENIED, causing valid cached LUNs to be treated as stale and forcing unnecessary detach/reattach.

    wil::unique_hfile file{CreateFileW(
        Path, 0, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)};
    THROW_LAST_ERROR_IF(!file);

src/windows/service/exe/WslCoreVm.cpp:1780

• In GenerateConfigJson's attachDisk lambda, ReserveLun() is called before OpenVhdBackingFile(). If OpenVhdBackingFile throws (e.g., path temporarily unavailable), the reserved LUN is never freed, which can leak LUNs across retries and eventually prevent further attachments.

    auto attachDisk = [&](PCWSTR path, bool grantVmAccess) {
        auto lun = ReserveLun();
        auto backingFile = OpenVhdBackingFile(path);
        hcs::Attachment disk{};

test/windows/Common.cpp:3165

• RunDiskpartScript writes the script using std::wofstream with the default C locale, which can corrupt non-ASCII paths embedded in the script (e.g., temp directories with Unicode user names). Diskpart scripts should be written with a deterministic Unicode encoding (UTF-16LE with BOM) so mount/VHD paths are preserved.

static void RunDiskpartScript(std::wstring_view Script)
{
    const auto scriptFileName = wsl::windows::common::filesystem::GetTempFilename();
    std::wofstream scriptFile(scriptFileName);
    THROW_HR_IF(E_FAIL, !scriptFile.is_open());

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/windows/service/exe/WslCoreVm.cpp:90

• IsBackingVolumeMounted() calls FSCTL_IS_VOLUME_MOUNTED on a handle opened to the VHD backing file (OpenVhdBackingFile). FSCTL_IS_VOLUME_MOUNTED is intended for volume handles, so this can fail with errors unrelated to a remount (e.g., invalid handle/function), causing false “stale” detections and unnecessary RemoveScsiDisk/Reattach, which defeats the attach cache and can drop Mounts tracking.

bool IsBackingVolumeMounted(_In_ HANDLE File)
{
    DWORD bytesReturned{};
    return DeviceIoControl(File, FSCTL_IS_VOLUME_MOUNTED, nullptr, 0, nullptr, 0, &bytesReturned, nullptr);
}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/windows/service/exe/WslCoreVm.cpp:90

• IsBackingVolumeMounted calls FSCTL_IS_VOLUME_MOUNTED on a handle opened to the VHDX file. FSCTL_IS_VOLUME_MOUNTED is a volume FSCTL and may fail or behave inconsistently when issued on a regular file handle, causing false “stale” detection. A more reliable check is to probe the cached handle with a lightweight file query (success implies the backing volume/device is still reachable).

bool IsBackingVolumeMounted(_In_ HANDLE File)
{
    DWORD bytesReturned{};
    return DeviceIoControl(File, FSCTL_IS_VOLUME_MOUNTED, nullptr, 0, nullptr, 0, &bytesReturned, nullptr);
}

src/windows/service/exe/WslCoreVm.cpp:81

• OpenVhdBackingFile opens the VHD with desiredAccess=0, but the cached handle is later used to query state (IsBackingVolumeMounted). Opening with FILE_READ_ATTRIBUTES is safer and avoids access-denied failures that would make every cached handle look stale.

This issue also appears on line 86 of the same file.

    wil::unique_hfile file{CreateFileW(
        Path, 0, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)};
    THROW_LAST_ERROR_IF(!file);

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (5)

.pipelines/wsl-build-pr.yml:12

• This PR hard-codes the CloudTest run to a single unit test (cloudTestName) and also restricts the test stage to only versions: [wsl2]. That significantly reduces CI coverage for this pipeline and isn’t described in the PR summary (which focuses on disk attach caching). Consider leaving these parameters at their defaults and, if needed, passing a one-off test filter when queueing the pipeline.

- template: build-stage.yml@self
  parameters:
    isRelease: false
    cloudTestName: 'UnitTests::UnitTests::CachedVhdIsReattachedAfterBackingVolumeRemount'
    pool: 'wsl-build'

.pipelines/wsl-build-pr-onebranch.yml:55

• This pipeline now hard-codes a single CloudTest name and limits test-stage versions to only wsl2, which drastically reduces validation breadth for all runs of this pipeline. If the intent is only to enable an optional targeted run, it’s safer to keep defaults here and pass the test name as an override when queueing.

    - template: build-stage.yml@self
      parameters:
        isRelease: false
        cloudTestName: 'UnitTests::UnitTests::CachedVhdIsReattachedAfterBackingVolumeRemount'

src/windows/service/exe/WslCoreVm.cpp:90

• IsBackingVolumeMounted returns the raw DeviceIoControl result without checking GetLastError. In AttachDiskLockHeld, any transient/unsupported IOCTL failure will be treated as “volume not mounted”, causing the cached disk to be forcibly removed and reattached unnecessarily. It’s safer to only return false for known “not mounted”/invalid-handle errors and treat other failures as “unknown” (keep existing LUN).

bool IsBackingVolumeMounted(_In_ HANDLE File)
{
    DWORD bytesReturned{};
    return DeviceIoControl(File, FSCTL_IS_VOLUME_MOUNTED, nullptr, 0, nullptr, 0, &bytesReturned, nullptr);
}

test/windows/Common.cpp:3161

• RunDiskpartScript writes the script with std::wofstream, which uses locale-dependent text encoding and doesn’t guarantee an encoding diskpart can parse (especially if paths contain non-ASCII). Other diskpart script helpers in tests write narrow text. Consider writing UTF-8 explicitly to avoid encoding-dependent failures.

    const auto scriptFileName = wsl::windows::common::filesystem::GetTempFilename();
    std::wofstream scriptFile(scriptFileName);
    THROW_HR_IF(E_FAIL, !scriptFile.is_open());

    auto cleanup = wil::scope_exit([&] { DeleteFileW(scriptFileName.c_str()); });

src/windows/service/exe/WslCoreVm.cpp:81

• OpenVhdBackingFile opens the path with desired-access = 0. Since the handle is subsequently used with DeviceIoControl (FSCTL_IS_VOLUME_MOUNTED), requesting at least FILE_READ_ATTRIBUTES makes the intent explicit and avoids access-denied behavior on filesystems/drivers that require read access for FSCTLs.

    wil::unique_hfile file{CreateFileW(
        Path, 0, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)};
    THROW_LAST_ERROR_IF(!file);

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (7)

test/windows/Common.cpp:3161

• RunDiskpartScript writes the script using std::wofstream, which will apply locale-based wchar_t→char conversion. This is inconsistent with the existing Diskpart scripting helper in DrvFsTests.cpp (uses std::ofstream) and can break Diskpart parsing when the script contains non-ASCII characters (or if the locale conversion is lossy). Write the script as a narrow/UTF-8 byte stream instead (e.g., via WideToMultiByte) to make the helper robust and consistent.

    const auto scriptFileName = wsl::windows::common::filesystem::GetTempFilename();
    std::wofstream scriptFile(scriptFileName);
    THROW_HR_IF(E_FAIL, !scriptFile.is_open());

    auto cleanup = wil::scope_exit([&] { DeleteFileW(scriptFileName.c_str()); });
    scriptFile << Script;
    scriptFile.close();

src/windows/service/exe/WslCoreVm.cpp:90

• IsBackingVolumeMounted() issues FSCTL_IS_VOLUME_MOUNTED against a handle opened on the VHD file with desired access=0. FSCTL_IS_VOLUME_MOUNTED is intended for volume handles, and even when it works it can fail due to handle type/access, causing false 'stale' detection and unnecessary disk detach/reattach. A more reliable check is to open the backing file with FILE_READ_ATTRIBUTES and validate the handle via GetFileInformationByHandleEx; if the backing volume is detached/remounted, the query should fail on the stale handle.

bool IsBackingVolumeMounted(_In_ HANDLE File)
{
    DWORD bytesReturned{};
    return DeviceIoControl(File, FSCTL_IS_VOLUME_MOUNTED, nullptr, 0, nullptr, 0, &bytesReturned, nullptr);
}

src/windows/service/exe/WslCoreVm.cpp:1077

• When a cached disk is detected as stale, the code removes the SCSI disk from HCS and erases the entry without notifying the guest (mini_init) to flush caches/stop using the LUN. Other detach paths call UnmountDisk() (which sends LxMiniInitMessageDetach) before RemoveScsiDisk (see DetachDisk). The stale-removal path should do the same (best-effort) to avoid leaving the guest with a suddenly-disappearing disk/LUN.

                const auto staleLun = found->second.Lun;
                wsl::windows::common::hcs::RemoveScsiDisk(m_system.get(), staleLun);
                if (WI_IsFlagSet(found->second.Flags, DiskStateFlags::AccessGranted))
                {
                    wsl::windows::common::hcs::RevokeVmAccess(m_machineId.c_str(), found->first.Path.c_str());

.pipelines/wsl-build-pr.yml:11

• This hard-codes the CloudTest run to a single unit test for every execution of this pipeline, which will significantly reduce PR validation coverage. Consider wiring this to a queue-time variable/parameter (default empty) so the default run still executes the full test filter.

    cloudTestName: 'UnitTests::UnitTests::CachedVhdIsReattachedAfterBackingVolumeRemount'

.pipelines/wsl-build-pr.yml:20

• Limiting the test stage to only 'wsl2' drops the default wsl1/wslc coverage provided by test-stage.yml. If the intent is only to target a specific test occasionally, keep the default versions list here (or make the reduction conditional) so PR validation coverage doesn't regress.

    versions: [wsl2]

.pipelines/wsl-build-pr-onebranch.yml:49

• This hard-codes the CloudTest run to a single unit test for every execution of this pipeline, which will significantly reduce PR validation coverage. Prefer wiring this to a queue-time variable/parameter (default empty) so the default run still executes the full test filter.

        cloudTestName: 'UnitTests::UnitTests::CachedVhdIsReattachedAfterBackingVolumeRemount'

.pipelines/wsl-build-pr-onebranch.yml:55

• Limiting the test stage to only 'wsl2' drops the default wsl1/wslc coverage provided by test-stage.yml. If this is only meant for targeted runs, keep the default versions list here (or gate it on a variable) so CI coverage doesn't regress.

        versions: [wsl2]

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/windows/service/exe/WslCoreVm.cpp:90

• IsBackingVolumeMounted() sends FSCTL_IS_VOLUME_MOUNTED to a handle opened on the VHD file path. FSCTL_IS_VOLUME_MOUNTED is a volume FSCTL and may fail (returning false) for file handles, which would cause cached VHDs to be treated as stale every time and negate the caching logic. A safer stale-handle check is to validate the backing-file handle via GetFileInformationByHandle, and open the file with FILE_READ_ATTRIBUTES so that check is supported.

bool IsBackingVolumeMounted(_In_ HANDLE File)
{
    DWORD bytesReturned{};
    return DeviceIoControl(File, FSCTL_IS_VOLUME_MOUNTED, nullptr, 0, nullptr, 0, &bytesReturned, nullptr);
}

test/windows/UnitTests.cpp:1506

• The PR description says a new unit test was added: "UnitTests::UnitTests::CachedVhdIsReattachedAfterBackingVolumeRemount", but no such test exists in the current changes (or anywhere in the repo). Either add the test (preferred, per checklist) or update the PR description/validation steps to reflect what was actually done.

                ValidateErrorMessage(
                    L"-d DummyBrokenDistro",
                    L"Failed to attach disk 'C:\\DoesNotExit\\ext4.vhdx' to WSL2: The system cannot find the path "
                    L"specified. ",
                    L"Wsl/Service/CreateInstance/MountDisk/ERROR_PATH_NOT_FOUND");

[Blue (OneBlue)]

This is tough because there's no way to atomically check that the disk is actually available before we mount it through HCS, which will reopen the handle on its end.

So there's a race between time we check, and then time HCS actually mounts the file, but this is still an improvement in behavior, so we should merge it. Thank you for looking into this !