@samitouri / QOSAMI-WSL / commits / c40ea600

Fix minor issues found during code review (#41104)

src/linux/init/DnsServer.h:48 Change the data type from uint16_t to size_t to avoid a potential counter overflow on valid data. As the counter needs to hold at most 2 + 65535. src/linux/init/init.cpp:2575 Drain the pending pids on SIGCHLD. Multiple SIGCHLD will only trigger the signalfd once if not processed fast enough: https://man7.org/linux/man-pages/man7/signal.7.html src/linux/init/util.cpp:3497 Fix a wrong condition. The old logic will mask most of the errors as EINVAL. src/windows/common/WslCoreMessageQueue.h:174 Avoid a tight race condition between submit and cancel on m_tpHandle. src/windows/service/exe/LxssUserSession.cpp:897 Avoid wrong context order. The order is asserted in debug builds. The old wrong MountDisk -> CreateVm context will crash the debug service. src/windows/service/exe/WslCoreVm.cpp:1003 Fix a wrong revert logic. Where the disk will be reverted to offline if it was online. src/windows/service/exe/WslCoreVm.cpp:2755 Block more invalid mount names.

Feng Wang committed Aug 5, 2026 at 10:42 UTC c40ea600c4bff850ac0a16be6824f8c3436bfe69
8 files changed +59 -24
localization/strings/en-US/Resources.resw
+1 -1
@@ -160,7 +160,7 @@ To force WSL2 to stop and detach the disk, run 'wsl.exe {}'.</value>
160 <comment>"mount" is a technical term meaning to make a disk/filesystem accessible. Use the standard technical term in your locale, or keep "mount" if commonly used.</comment>
161 </data>
162 <data name="MessageDiskMountNameInvalid" xml:space="preserve">
163 - <value>The specified mount name contains an invalid '/' character. Please retry without the invalid character.</value>
163 + <value>The mount name cannot be empty, '.', '..', or contain '/'. Please retry with a valid mount name.</value>
164 <comment>"mount" is a technical term meaning to make a disk/filesystem accessible. Use the standard technical term in your locale, or keep "mount" if commonly used.</comment>
165 </data>
166 <data name="MessageDiskMounted" xml:space="preserve">
src/linux/init/DnsServer.h
+2 -2
@@ -44,8 +44,8 @@ private:
44 wil::unique_fd m_tcpConnection;
45
46 // Offset in m_currentDnsRequest indicating how much of the current DNS request on
47 - // the TCP connection has been read. Using 2 bytes to represent the offset as the request length is represented using 2 bytes.
48 - uint16_t m_currentRequestOffset = 0;
47 + // the TCP connection has been read.
48 + size_t m_currentRequestOffset = 0;
49
50 // Buffer containing the current DNS request received on the TCP connection.
51 std::vector<gsl::byte> m_currentDnsRequest;
src/linux/init/init.cpp
+21 -11
@@ -2595,23 +2595,33 @@ Return Value:
2595 continue;
2596 }
2597
2598 - int Status{};
2599 - auto Pid = waitpid(-1, &Status, WNOHANG);
2600 - if (Pid == 0)
2601 - {
2602 - continue;
2603 - }
2604 - else if (Pid > 0)
2598 + bool distroInitExited = false;
2599 + for (;;)
2600 {
2606 - if (Pid == distroInitPid.value())
2601 + int Status{};
2602 + auto Pid = waitpid(-1, &Status, WNOHANG);
2603 + if (Pid == 0)
2604 + {
2605 + break;
2606 + }
2607 + else if (Pid > 0)
2608 + {
2609 + distroInitExited |= (Pid == distroInitPid.value());
2610 + }
2611 + else if (errno == ECHILD)
2612 {
2608 - LOG_ERROR("Init has exited. Terminating distribution");
2613 break;
2614 }
2615 + else
2616 + {
2617 + FATAL_ERROR("waitpid failed {}", errno);
2618 + }
2619 }
2612 - else if (errno != ECHILD)
2620 +
2621 + if (distroInitExited)
2622 {
2614 - FATAL_ERROR("waitpid failed {}", errno);
2623 + LOG_ERROR("Init has exited. Terminating distribution");
2624 + break;
2625 }
2626 }
2627 }
src/linux/init/util.cpp
+1 -1
@@ -3516,7 +3516,7 @@ int ProcessCreateProcessMessage(wsl::shared::Transaction& Transaction, gsl::span
3516 {
3517 execResult = 0;
3518 }
3519 - else if (execResult == sizeof(execResult))
3519 + else if (ReadResult == sizeof(execResult))
3520 {
3521 // Otherwise, return the error code to the service
3522 execResult = abs(execResult);
src/windows/common/WslCoreMessageQueue.h
+6 -4
@@ -171,10 +171,11 @@ public:
171 const auto queueLock = m_lock.lock_exclusive();
172 THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_CANCELLED), m_isCanceled);
173 m_workItems.emplace_back(new_result);
174 +
175 + // always maintain a 1:1 ratio for calls to submit_with_results() and ::SubmitThreadpoolWork
176 + SubmitThreadpoolWork(m_tpHandle.get());
177 }
178
176 - // always maintain a 1:1 ratio for calls to SubmitWorkWithResults() and ::SubmitThreadpoolWork
177 - SubmitThreadpoolWork(m_tpHandle.get());
179 return new_result;
180 }
181 catch (...)
@@ -194,10 +195,11 @@ public:
195 const auto queueLock = m_lock.lock_exclusive();
196 THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_CANCELLED), m_isCanceled);
197 m_workItems.emplace_back(std::forward<SimpleFunction_t>(functor));
198 +
199 + // always maintain a 1:1 ratio for calls to submit() and ::SubmitThreadpoolWork
200 + SubmitThreadpoolWork(m_tpHandle.get());
201 }
202
199 - // always maintain a 1:1 ratio for calls to SubmitWork() and ::SubmitThreadpoolWork
200 - SubmitThreadpoolWork(m_tpHandle.get());
203 return true;
204 }
205 catch (...)
src/windows/service/exe/LxssUserSession.cpp
+1 -2
@@ -892,11 +892,10 @@ HRESULT LxssUserSessionImpl::MountDisk(
892 _Out_ int* Step,
893 _Out_ LPWSTR* MountName)
894 {
895 - ExecutionContext context(Context::MountDisk);
896 -
895 std::lock_guard lock(m_instanceLock);
896 return wil::ResultFromException([&]() {
897 _CreateVm();
898 + ExecutionContext context(Context::MountDisk);
899 const auto MountDiskType = WI_IsFlagSet(Flags, LXSS_ATTACH_MOUNT_FLAGS_VHD) ? WslCoreVm::DiskType::VHD : WslCoreVm::DiskType::PassThrough;
900 const auto MountResult = m_utilityVm->MountDisk(Disk, MountDiskType, PartitionIndex, Name, Type, Options);
901 const auto MountNameWide = wsl::shared::string::MultiByteToWide(MountResult.MountPointName);
src/windows/service/exe/WslCoreVm.cpp
+4 -3
@@ -1000,7 +1000,7 @@ ULONG WslCoreVm::AttachDiskLockHeld(
1000 if (WI_IsFlagSet(diskFlags, DiskStateFlags::Online))
1001 {
1002 const auto diskHandle = wsl::windows::common::disk::OpenDevice(Disk, GENERIC_READ | GENERIC_WRITE, m_vmConfig.MountDeviceTimeout);
1003 - wsl::windows::common::disk::SetOnline(diskHandle.get(), false, m_vmConfig.MountDeviceTimeout);
1003 + wsl::windows::common::disk::SetOnline(diskHandle.get(), true, m_vmConfig.MountDeviceTimeout);
1004 }
1005 });
1006
@@ -2742,8 +2742,9 @@ std::string WslCoreVm::s_GetMountTargetName(_In_ PCWSTR Disk, _In_opt_ PCWSTR Na
2742 if (ARGUMENT_PRESENT(Name))
2743 {
2744 auto mountName = wsl::shared::string::WideToMultiByte(Name);
2745 - // Throw if the name contains '/' since it is a linux path separator
2746 - THROW_HR_IF(WSL_E_VM_MODE_INVALID_MOUNT_NAME, mountName.find('/') != std::string::npos);
2745 + THROW_HR_IF(
2746 + WSL_E_VM_MODE_INVALID_MOUNT_NAME,
2747 + mountName.empty() || mountName == "." || mountName == ".." || mountName.find('/') != std::string::npos);
2748 return mountName;
2749 }
2750
test/windows/MountTests.cpp
+23
@@ -341,6 +341,29 @@ class MountTests
341 WaitForDiskReady();
342 }
343
344 + WSL2_TEST_METHOD(SpecifyInvalidMountName)
345 + {
346 + SKIP_UNSUPPORTED_ARM64_MOUNT_TEST();
347 +
348 + FormatDisk({L"ext4"}, true);
349 +
350 + for (const auto* name : {L"\"\"", L".", L"..", L"foo/bar"})
351 + {
352 + const auto mountCommand = std::format(L"--mount {} --vhd --name {} --partition 1", VhdDevice, name);
353 + const auto [output, error] = LxsstuLaunchWslAndCaptureOutput(mountCommand, -1);
354 + VERIFY_ARE_EQUAL(
355 + output,
356 + L"The mount name cannot be empty, '.', '..', or contain '/'. Please retry with a valid mount name.\r\n"
357 + L"Error code: Wsl/Service/MountDisk/WSL_E_VM_MODE_INVALID_MOUNT_NAME\r\n",
358 + name);
359 + VERIFY_ARE_EQUAL(error, L"", name);
360 + }
361 +
362 + const auto disk = GetBlockDeviceInWsl();
363 + VERIFY_IS_TRUE(IsBlockDevicePresent(disk));
364 + VERIFY_IS_FALSE(GetBlockDeviceMount(disk + L"1").has_value());
365 + }
366 +
367 // Test ensuring that name collision detection works in --mount --name
368 WSL2_TEST_METHOD(SpecifyMountNameCollision)
369 {