Fix VHD ownership after cross-volume move to prevent E_ACCESSDENIED (#40159)
* Fix VHD ownership after cross-volume move to prevent E_ACCESSDENIED When MoveDistribution moves a VHD across volumes, MoveFileEx copies the file and the new file's owner may not be the user's SID. This causes HcsGrantVmAccess to fail with E_ACCESSDENIED when later launching the distro, because the impersonated user lacks WRITE_DAC on the file (only implicitly granted to the owner). Fix by explicitly setting the VHD owner to the user's SID after the move, matching what CreateVhd already does at creation time. Uses handle-based SetSecurityInfo with FILE_FLAG_OPEN_REPARSE_POINT to avoid TOCTOU races and symlink following. Also fixes a pre-existing build break in MountTests.cpp from the test refactor (WSL2_TEST_ONLY -> WSL2_TEST_METHOD). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Preserve original VHD owner instead of using GetUserSid() Instead of unconditionally setting the VHD owner to the caller's SID after a cross-volume move, read the original owner before the move and restore it afterward. This avoids changing ownership to someone who didn't originally own the file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ben Hillis committed
Apr 17, 2026 at 07:21 UTC
34ebfc49304c5744037de3409959954c9f3fb764
2 files changed
+100
src/windows/service/exe/LxssUserSession.cpp
+26
@@ -939,13 +939,39 @@ HRESULT LxssUserSessionImpl::MoveDistribution(_In_ LPCGUID DistroGuid, _In_ LPCW
939
THROW_WIN32(error.value());
940
}
941
942
+ // Read the original VHD owner before the move so we can restore it after.
943
+ // Cross-volume MoveFileEx may set the owner to BUILTIN\Administrators for
944
+ // elevated callers, which breaks HcsGrantVmAccess (needs WRITE_DAC via
945
+ // ownership) from non-elevated contexts.
946
+ PSID originalOwner = nullptr;
947
+ wil::unique_hlocal originalDescriptor;
948
+ THROW_IF_WIN32_ERROR(GetNamedSecurityInfoW(
949
+ distro.VhdFilePath.c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, &originalOwner, nullptr, nullptr, nullptr, &originalDescriptor));
950
+
951
// Move the VHD to the new location.
952
THROW_IF_WIN32_BOOL_FALSE(MoveFileEx(distro.VhdFilePath.c_str(), newVhdPath.c_str(), MOVEFILE_COPY_ALLOWED | MOVEFILE_WRITE_THROUGH));
953
954
+ // Restore the original VHD owner on the moved file.
955
+ auto setVhdOwner = [&originalOwner](const std::filesystem::path& vhdPath) {
956
+ wil::unique_hfile vhdHandle(CreateFileW(
957
+ vhdPath.c_str(), WRITE_OWNER, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT, nullptr));
958
+ THROW_LAST_ERROR_IF(!vhdHandle);
959
+
960
+ auto runAsSelf = wil::run_as_self();
961
+ auto privileges = wsl::windows::common::security::AcquirePrivilege(SE_RESTORE_NAME);
962
+ THROW_IF_WIN32_ERROR(
963
+ ::SetSecurityInfo(vhdHandle.get(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, originalOwner, nullptr, nullptr, nullptr));
964
+ };
965
+
966
+ setVhdOwner(newVhdPath);
967
+
968
auto revert = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() {
969
THROW_IF_WIN32_BOOL_FALSE(MoveFileEx(
970
newVhdPath.c_str(), distro.VhdFilePath.c_str(), MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH));
971
972
+ // Fix ownership on the reverted VHD in case MoveFileEx copied across volumes.
973
+ LOG_IF_FAILED(wil::ResultFromException([&] { setVhdOwner(distro.VhdFilePath); }));
974
+
975
// Write the location back to the original path in case the second registry write failed. Otherwise, this is a no-op.
976
registration.Write(Property::BasePath, distro.BasePath.c_str());
977
});
test/windows/UnitTests.cpp
+74
@@ -2801,6 +2801,80 @@ Error code: Wsl/InstallDistro/WSL_E_DISTRO_NOT_FOUND
2801
}
2802
}
2803
2804
+ TEST_METHOD(MoveVhdOwnership)
2805
+ {
2806
+ constexpr auto name = L"move-owner-test-distro";
2807
+ constexpr auto moveElevatedFolder = L"move-owner-elevated";
2808
+ constexpr auto moveNonElevatedFolder = L"move-owner-non-elevated";
2809
+
2810
+ // Import a WSL2 distro.
2811
+ VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"--import {} . \"{}\" --version 2", name, g_testDistroPath)), 0L);
2812
+
2813
+ auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [name]() {
2814
+ LxsstuLaunchWsl(std::format(L"--unregister {}", name));
2815
+ std::filesystem::remove_all(moveElevatedFolder);
2816
+ std::filesystem::remove_all(moveNonElevatedFolder);
2817
+ });
2818
+
2819
+ auto verifyVhdOwner = [](const std::wstring& path) {
2820
+ PSID ownerSid = nullptr;
2821
+ wil::unique_hlocal descriptor;
2822
+ THROW_IF_WIN32_ERROR(GetNamedSecurityInfoW(
2823
+ path.c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, &ownerSid, nullptr, nullptr, nullptr, &descriptor));
2824
+
2825
+ auto userToken = wil::open_current_access_token(TOKEN_QUERY);
2826
+ auto tokenUser = wil::get_token_information<TOKEN_USER>(userToken.get());
2827
+
2828
+ VERIFY_IS_TRUE(EqualSid(ownerSid, tokenUser->User.Sid));
2829
+ };
2830
+
2831
+ const auto nonElevatedToken = GetNonElevatedToken();
2832
+
2833
+ // Move as elevated, launch as non-elevated.
2834
+ // This is the primary bug scenario: MoveFileEx sets owner to BUILTIN\Administrators,
2835
+ // then HcsGrantVmAccess fails with E_ACCESSDENIED when impersonating the non-elevated user.
2836
+ {
2837
+ WslShutdown();
2838
+ VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"--manage {} --move {}", name, moveElevatedFolder)), 0L);
2839
+
2840
+ auto vhdPath = std::format(L"{}\\ext4.vhdx", moveElevatedFolder);
2841
+ VERIFY_IS_TRUE(std::filesystem::exists(vhdPath));
2842
+ verifyVhdOwner(vhdPath);
2843
+
2844
+ WslShutdown();
2845
+ auto [out, err] = LxsstuLaunchWslAndCaptureOutput(std::format(L"-d {} echo ok", name), 0, nullptr, nonElevatedToken.get());
2846
+ VERIFY_ARE_EQUAL(out, L"ok\n");
2847
+ }
2848
+
2849
+ // Move as non-elevated, launch as elevated.
2850
+ {
2851
+ WslShutdown();
2852
+ VERIFY_ARE_EQUAL(
2853
+ LxsstuLaunchWsl(
2854
+ std::format(L"--manage {} --move {}", name, moveNonElevatedFolder),
2855
+ nullptr,
2856
+ nullptr,
2857
+ nullptr,
2858
+ nonElevatedToken.get()),
2859
+ 0L);
2860
+
2861
+ auto vhdPath = std::format(L"{}\\ext4.vhdx", moveNonElevatedFolder);
2862
+ VERIFY_IS_TRUE(std::filesystem::exists(vhdPath));
2863
+ verifyVhdOwner(vhdPath);
2864
+
2865
+ WslShutdown();
2866
+ auto [out, err] = LxsstuLaunchWslAndCaptureOutput(std::format(L"-d {} echo ok", name));
2867
+ VERIFY_ARE_EQUAL(out, L"ok\n");
2868
+ }
2869
+
2870
+ // Also launch as non-elevated after the non-elevated move.
2871
+ {
2872
+ WslShutdown();
2873
+ auto [out, err] = LxsstuLaunchWslAndCaptureOutput(std::format(L"-d {} echo ok", name), 0, nullptr, nonElevatedToken.get());
2874
+ VERIFY_ARE_EQUAL(out, L"ok\n");
2875
+ }
2876
+ }
2877
+
2878
WSL2_TEST_METHOD(Resize)
2879
{
2880
constexpr auto name = L"resize-test-distro";