From 487bed1852873082edb649f2ec44f1c6d8876dfb Mon Sep 17 00:00:00 2001 From: Ben Hillis Date: Fri, 5 Jun 2026 09:14:14 -0700 Subject: [PATCH 1/2] Fix MoveDistribution E_ACCESSDENIED when setVhdOwner fails under impersonation (#40716) After a cross-volume MoveFileEx, the new VHD file's owner may be set to BUILTIN\Administrators. The setVhdOwner lambda was opening the file with WRITE_OWNER under user impersonation, which fails with E_ACCESSDENIED if the user doesn't own the file. The subsequent run_as_self() came too late. Move run_as_self() and AcquirePrivilege(SE_RESTORE_NAME) before the CreateFileW call so the file is opened as SYSTEM, which always has WRITE_OWNER regardless of file ownership. Previously, this failure left the VHD at the new location with the registry BasePath still pointing at the old path, corrupting the distro. Fixes #40716 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/windows/service/exe/LxssUserSession.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/windows/service/exe/LxssUserSession.cpp b/src/windows/service/exe/LxssUserSession.cpp index 831b9403d..ba22e10e0 100644 --- a/src/windows/service/exe/LxssUserSession.cpp +++ b/src/windows/service/exe/LxssUserSession.cpp @@ -952,13 +952,17 @@ HRESULT LxssUserSessionImpl::MoveDistribution(_In_ LPCGUID DistroGuid, _In_ LPCW THROW_IF_WIN32_BOOL_FALSE(MoveFileEx(distro.VhdFilePath.c_str(), newVhdPath.c_str(), MOVEFILE_COPY_ALLOWED | MOVEFILE_WRITE_THROUGH)); // Restore the original VHD owner on the moved file. + // Run as self (SYSTEM) for both the file open and the SetSecurityInfo call, + // because after a cross-volume MoveFileEx the new file's owner may be + // BUILTIN\Administrators and the impersonated user token may lack WRITE_OWNER. auto setVhdOwner = [&originalOwner](const std::filesystem::path& vhdPath) { + auto runAsSelf = wil::run_as_self(); + auto privileges = wsl::windows::common::security::AcquirePrivilege(SE_RESTORE_NAME); + wil::unique_hfile vhdHandle(CreateFileW( vhdPath.c_str(), WRITE_OWNER, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT, nullptr)); THROW_LAST_ERROR_IF(!vhdHandle); - auto runAsSelf = wil::run_as_self(); - auto privileges = wsl::windows::common::security::AcquirePrivilege(SE_RESTORE_NAME); THROW_IF_WIN32_ERROR( ::SetSecurityInfo(vhdHandle.get(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, originalOwner, nullptr, nullptr, nullptr)); }; From 7171848737cb203d0c4a86745c90a661be58cce1 Mon Sep 17 00:00:00 2001 From: Ben Hillis Date: Fri, 5 Jun 2026 09:20:55 -0700 Subject: [PATCH 2/2] Add test for MoveDistribution with admin-owned VHD (#40716) Regression test that simulates a cross-volume move scenario: after moving a distro, the VHD owner is explicitly set to BUILTIN\Administrators (mimicking MoveFileEx cross-volume behavior), then a second move is attempted as a non-elevated user. Before the fix, setVhdOwner would fail with E_ACCESSDENIED because CreateFileW(WRITE_OWNER) ran under user impersonation. The test verifies: - The move succeeds even when the VHD is owned by Administrators - The VHD owner is restored to the user's SID after the move - The distro launches successfully after the move Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/windows/UnitTests.cpp | 73 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/test/windows/UnitTests.cpp b/test/windows/UnitTests.cpp index b9d54b369..c8aa0791d 100644 --- a/test/windows/UnitTests.cpp +++ b/test/windows/UnitTests.cpp @@ -3048,6 +3048,79 @@ Error code: Wsl/InstallDistro/WSL_E_DISTRO_NOT_FOUND } } + WSL2_TEST_METHOD(MoveVhdWithAdminOwner) + { + // Regression test for #40716: if the VHD's owner is BUILTIN\Administrators + // (as happens after a cross-volume MoveFileEx from an elevated context), + // the move must still succeed because setVhdOwner runs as SYSTEM. + constexpr auto name = L"move-admin-owner-test-distro"; + constexpr auto firstFolder = L"move-admin-owner-first"; + constexpr auto secondFolder = L"move-admin-owner-second"; + + // Import a WSL2 distro. + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"--import {} . \"{}\" --version 2", name, g_testDistroPath)), 0L); + + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [name]() { + LxsstuLaunchWsl(std::format(L"--unregister {}", name)); + std::filesystem::remove_all(firstFolder); + std::filesystem::remove_all(secondFolder); + }); + + // Move to first folder so we know where the VHD is. + WslShutdown(); + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"--manage {} --move {}", name, firstFolder)), 0L); + + auto vhdPath = std::format(L"{}\\ext4.vhdx", firstFolder); + VERIFY_IS_TRUE(std::filesystem::exists(vhdPath)); + + // Simulate cross-volume MoveFileEx side-effect: change VHD owner to BUILTIN\Administrators. + { + BYTE adminsSidBuffer[SECURITY_MAX_SID_SIZE]; + DWORD sidSize = sizeof(adminsSidBuffer); + THROW_IF_WIN32_BOOL_FALSE(CreateWellKnownSid(WinBuiltinAdministratorsSid, nullptr, adminsSidBuffer, &sidSize)); + + THROW_IF_WIN32_ERROR(SetNamedSecurityInfoW( + const_cast(vhdPath.c_str()), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, adminsSidBuffer, nullptr, nullptr, nullptr)); + + // Verify it took effect. + PSID ownerSid = nullptr; + wil::unique_hlocal descriptor; + THROW_IF_WIN32_ERROR(GetNamedSecurityInfoW( + vhdPath.c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, &ownerSid, nullptr, nullptr, nullptr, &descriptor)); + VERIFY_IS_TRUE(EqualSid(ownerSid, adminsSidBuffer)); + } + + // Now move again as non-elevated. Before the fix, this would fail with E_ACCESSDENIED + // because CreateFileW(WRITE_OWNER) was called under user impersonation. + const auto nonElevatedToken = GetNonElevatedToken(); + WslShutdown(); + VERIFY_ARE_EQUAL( + LxsstuLaunchWsl(std::format(L"--manage {} --move {}", name, secondFolder), nullptr, nullptr, nullptr, nonElevatedToken.get()), 0L); + + auto newVhdPath = std::format(L"{}\\ext4.vhdx", secondFolder); + VERIFY_IS_TRUE(std::filesystem::exists(newVhdPath)); + + // Verify the VHD owner was preserved. The code reads the owner before + // MoveFileEx and restores it afterward. Since we set the owner to + // Administrators before this move, it should still be Administrators. + { + PSID ownerSid = nullptr; + wil::unique_hlocal descriptor; + THROW_IF_WIN32_ERROR(GetNamedSecurityInfoW( + newVhdPath.c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, &ownerSid, nullptr, nullptr, nullptr, &descriptor)); + + BYTE adminsSidCheck[SECURITY_MAX_SID_SIZE] = {}; + DWORD sidSize = sizeof(adminsSidCheck); + THROW_IF_WIN32_BOOL_FALSE(CreateWellKnownSid(WinBuiltinAdministratorsSid, nullptr, adminsSidCheck, &sidSize)); + VERIFY_IS_TRUE(EqualSid(ownerSid, adminsSidCheck)); + } + + // Validate distro still works. + WslShutdown(); + auto [out, err] = LxsstuLaunchWslAndCaptureOutput(std::format(L"-d {} echo ok", name), 0, nullptr, nonElevatedToken.get()); + VERIFY_ARE_EQUAL(out, L"ok\n"); + } + WSL2_TEST_METHOD(Resize) { constexpr auto name = L"resize-test-distro";