From 04915d69879b39998bf4ea58ff1da2c604018ee3 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Sat, 6 Jun 2026 13:10:00 -0700 Subject: [PATCH 1/3] Fix virtiofs bind mounts exposing files as root-owned (#40719) Add the 'metadata' option to virtiofs shares created via HcsVirtualMachine::AddShare (the WSLC/Docker container path). Without this option, the virtiofs device host cannot persist per-file uid/gid in NTFS extended attributes, so all files default to uid=0/gid=0 regardless of the creating user. This matches the behavior of the regular distro mount path (WslCoreVm::AddVirtioFsShare) which receives metadata/uid/gid options from the Linux init's ConvertDrvfsMountOptionsToPlan9. Also adds a regression test (WindowsMountsVirtioFsFileOwnership) that verifies file ownership is preserved on virtiofs mounts. Fixes #40719 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/windows/service/exe/HcsVirtualMachine.cpp | 2 +- test/windows/WSLCTests.cpp | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/windows/service/exe/HcsVirtualMachine.cpp b/src/windows/service/exe/HcsVirtualMachine.cpp index 0bfb5062b0..4d0a179e9e 100644 --- a/src/windows/service/exe/HcsVirtualMachine.cpp +++ b/src/windows/service/exe/HcsVirtualMachine.cpp @@ -580,7 +580,7 @@ try } else { - std::wstring options = ReadOnly ? L"ro" : L""; + std::wstring options = ReadOnly ? L"ro;metadata" : L"metadata"; if (!m_swiotlbOption.empty()) { if (!options.empty()) diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 879e554957..46b2d85584 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -3388,6 +3388,42 @@ class WSLCTests ValidateWindowsMounts(true); } + // Validates that virtiofs mounts preserve file ownership for non-root users (regression test for #40719). + WSLC_TEST_METHOD(WindowsMountsVirtioFsFileOwnership) + { + auto settings = GetDefaultSessionSettings(L"virtiofs-ownership-test"); + WI_SetFlag(settings.FeatureFlags, WslcFeatureFlagsVirtioFs); + + auto createNewSession = !WI_IsFlagSet(m_defaultSessionSettings.FeatureFlags, WslcFeatureFlagsVirtioFs); + auto session = createNewSession ? CreateSession(settings) : m_defaultSession; + + auto testFolder = std::filesystem::current_path() / "test-folder-virtiofs-ownership"; + std::filesystem::create_directories(testFolder); + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { std::filesystem::remove_all(testFolder); }); + + VERIFY_SUCCEEDED(session->MountWindowsFolder(testFolder.c_str(), "/win-path", false)); + auto unmount = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { session->UnmountWindowsFolder("/win-path"); }); + + // Create a file and chown to uid 1000:100, then verify ownership is preserved. + // Without the 'metadata' option on the virtiofs share, chown appears to succeed but + // subsequent stat reports uid=0/gid=0 because ownership is not persisted. + auto result = ExpectCommandResult( + session.get(), + {"/bin/sh", "-c", "touch /win-path/owned.txt && chown 1000:100 /win-path/owned.txt && stat -c '%u %g' /win-path/owned.txt"}, + 0); + + VERIFY_ARE_EQUAL(result.Output[1], std::string("1000 100\n")); + + // Verify that a file created by a non-root user retains the creator's ownership. + // Use 'su' to run as uid 65534 (nobody) and create a file, then verify ownership. + result = ExpectCommandResult( + session.get(), + {"/bin/sh", "-c", "rm -f /win-path/nobody.txt && su nobody -s /bin/sh -c 'touch /win-path/nobody.txt' && stat -c '%u' /win-path/nobody.txt"}, + 0); + + VERIFY_ARE_EQUAL(result.Output[1], std::string("65534\n")); + } + // Validates that VirtioFs shares are reused across mount/unmount cycles for the same Windows folder. WSLC_TEST_METHOD(WindowsMountsVirtioFsShareReuse) { From af5bcb9c244233134e8a22876e0b8aecd0e9b59f Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Sat, 6 Jun 2026 13:39:54 -0700 Subject: [PATCH 2/3] Address code review feedback - Add inline comment explaining why 'metadata' is required - Use unique mount point (/virtiofs-ownership-test) to avoid test interference - Use numeric UID (su '#65534') instead of username to avoid environment dependency Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/windows/service/exe/HcsVirtualMachine.cpp | 2 ++ test/windows/WSLCTests.cpp | 12 +++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/windows/service/exe/HcsVirtualMachine.cpp b/src/windows/service/exe/HcsVirtualMachine.cpp index 4d0a179e9e..5e4f41b834 100644 --- a/src/windows/service/exe/HcsVirtualMachine.cpp +++ b/src/windows/service/exe/HcsVirtualMachine.cpp @@ -580,6 +580,8 @@ try } else { + // N.B. The 'metadata' option is required so the virtiofs device host persists per-file + // uid/gid in NTFS extended attributes. Without it, all files appear as root-owned. std::wstring options = ReadOnly ? L"ro;metadata" : L"metadata"; if (!m_swiotlbOption.empty()) { diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 46b2d85584..e04eac9cfc 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -3401,24 +3401,26 @@ class WSLCTests std::filesystem::create_directories(testFolder); auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { std::filesystem::remove_all(testFolder); }); - VERIFY_SUCCEEDED(session->MountWindowsFolder(testFolder.c_str(), "/win-path", false)); - auto unmount = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { session->UnmountWindowsFolder("/win-path"); }); + static constexpr auto mountPoint = "/virtiofs-ownership-test"; + + VERIFY_SUCCEEDED(session->MountWindowsFolder(testFolder.c_str(), mountPoint, false)); + auto unmount = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { session->UnmountWindowsFolder(mountPoint); }); // Create a file and chown to uid 1000:100, then verify ownership is preserved. // Without the 'metadata' option on the virtiofs share, chown appears to succeed but // subsequent stat reports uid=0/gid=0 because ownership is not persisted. auto result = ExpectCommandResult( session.get(), - {"/bin/sh", "-c", "touch /win-path/owned.txt && chown 1000:100 /win-path/owned.txt && stat -c '%u %g' /win-path/owned.txt"}, + {"/bin/sh", "-c", "touch /virtiofs-ownership-test/owned.txt && chown 1000:100 /virtiofs-ownership-test/owned.txt && stat -c '%u %g' /virtiofs-ownership-test/owned.txt"}, 0); VERIFY_ARE_EQUAL(result.Output[1], std::string("1000 100\n")); // Verify that a file created by a non-root user retains the creator's ownership. - // Use 'su' to run as uid 65534 (nobody) and create a file, then verify ownership. + // Use numeric UID via su to avoid dependency on specific usernames being configured. result = ExpectCommandResult( session.get(), - {"/bin/sh", "-c", "rm -f /win-path/nobody.txt && su nobody -s /bin/sh -c 'touch /win-path/nobody.txt' && stat -c '%u' /win-path/nobody.txt"}, + {"/bin/sh", "-c", "rm -f /virtiofs-ownership-test/nonroot.txt && su -s /bin/sh '#65534' -c 'touch /virtiofs-ownership-test/nonroot.txt' && stat -c '%u' /virtiofs-ownership-test/nonroot.txt"}, 0); VERIFY_ARE_EQUAL(result.Output[1], std::string("65534\n")); From 64d6fd830a4dd15404394daff6be7499d891c37b Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 11 Jun 2026 11:51:00 -0700 Subject: [PATCH 3/3] test: use 'nobody' user instead of numeric UID in virtiofs ownership test The su command with numeric UID syntax ('#65534') requires the user to exist in /etc/passwd. Use the 'nobody' account directly since it is already available in the test VHD. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/windows/WSLCTests.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index e04eac9cfc..81d4451fac 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -3417,10 +3417,9 @@ class WSLCTests VERIFY_ARE_EQUAL(result.Output[1], std::string("1000 100\n")); // Verify that a file created by a non-root user retains the creator's ownership. - // Use numeric UID via su to avoid dependency on specific usernames being configured. result = ExpectCommandResult( session.get(), - {"/bin/sh", "-c", "rm -f /virtiofs-ownership-test/nonroot.txt && su -s /bin/sh '#65534' -c 'touch /virtiofs-ownership-test/nonroot.txt' && stat -c '%u' /virtiofs-ownership-test/nonroot.txt"}, + {"/bin/sh", "-c", "rm -f /virtiofs-ownership-test/nonroot.txt && su -s /bin/sh nobody -c 'touch /virtiofs-ownership-test/nonroot.txt' && stat -c '%u' /virtiofs-ownership-test/nonroot.txt"}, 0); VERIFY_ARE_EQUAL(result.Output[1], std::string("65534\n"));