@samitouri / QOSAMI-WSL / commits / 3f00f988

Fix socket race condition in port tracking (#40187)

* Make port 0 resolution inline * reduce retry delay to 10ms * add e2e test * resolve comments * resolve comments * [wip] add test debug info * copy the new binary to test stage * switch to using inline perl sript instead of linux binary --------- Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

Feng Wang committed Apr 23, 2026 at 05:23 UTC 3f00f9882f1fe09c85fe7f70ba2acd80b51e0062

3 files changed +65 -82

src/linux/init/GnsPortTracker.cpp

+27 -68

@@ -73,7 +73,6 @@ void GnsPortTracker::Run()

73 // for port deallocation

75 std::thread{std::bind(&GnsPortTracker::RunPortRefresh, this)}.detach();

76 - std::thread{std::bind(&GnsPortTracker::RunDeferredResolve, this)}.detach();

77 auto future = std::make_optional(m_allocatedPortsRefresh.get_future());

78 std::optional<PortRefreshResult> refreshResult;

@@ -121,13 +120,28 @@ void GnsPortTracker::Run()

120 {

121 try

122 {

124 - std::lock_guard lock(m_deferredMutex);

125 - m_deferredQueue.push_back(std::move(bindCall->PortZeroBind.value()));

126 - m_deferredCv.notify_one();

123 + auto allocation = ResolvePortZeroBind(std::move(bindCall->PortZeroBind.value()));

124 + if (allocation.has_value())

125 + {

126 + const auto portResult = HandleRequest(allocation.value());

127 + if (portResult == 0)

128 + {

129 + TrackPort(std::move(allocation.value()));

130 + }

131 + else

132 + {

133 + GNS_LOG_ERROR(

134 + "Failed to register resolved port-0 bind: family ({}) port ({}) protocol ({}), error {}",

135 + allocation->Family,

136 + allocation->Port,

137 + allocation->Protocol,

138 + portResult);

139 + }

140 + }

141 }

142 catch (const std::exception& e)

143 {

130 - GNS_LOG_ERROR("Failed to queue port-0 bind for deferred resolution, {}", e.what());

144 + GNS_LOG_ERROR("Failed to resolve port-0 bind, {}", e.what());

145 }

146 }

147 }

@@ -148,30 +162,6 @@ void GnsPortTracker::Run()

162 }

163 }

164

151 - // Process any port-0 binds that the background thread has resolved.

152 - std::deque<PortAllocation> resolved;

153 - {

154 - std::lock_guard lock(m_resolvedMutex);

155 - resolved.swap(m_resolvedQueue);

156 - }

157 - for (auto& allocation : resolved)

158 - {

159 - const auto result = HandleRequest(allocation);

160 - if (result == 0)

161 - {

162 - TrackPort(std::move(allocation));

163 - }

164 - else

165 - {

166 - GNS_LOG_ERROR(

167 - "Failed to register resolved port-0 bind: family ({}) port ({}) protocol ({}), error {}",

168 - allocation.Family,

169 - allocation.Port,

170 - allocation.Protocol,

171 - result);

172 - }

173 - }

174 -

165 // Only look at bound ports if there's something to deallocate to avoid wasting cycles

166 if (refreshResult.has_value())

167 {

@@ -539,43 +529,16 @@ catch (const std::exception& e)

529 GNS_LOG_ERROR("Failed to track port allocation, {}", e.what());

530 }

531

542 -void GnsPortTracker::RunDeferredResolve()

543 -{

544 - UtilSetThreadName("GnsPortZero");

545 -

546 - for (;;)

547 - {

548 - DeferredPortLookup lookup{0, {}, 0};

549 - {

550 - std::unique_lock lock(m_deferredMutex);

551 - m_deferredCv.wait(lock, [&] { return !m_deferredQueue.empty(); });

552 - lookup = std::move(m_deferredQueue.front());

553 - m_deferredQueue.pop_front();

554 - }

555 -

556 - const auto pid = lookup.Pid;

557 - try

558 - {

559 - ResolvePortZeroBind(std::move(lookup));

560 - }

561 - catch (const std::exception& e)

562 - {

563 - GNS_LOG_ERROR("Failed to resolve port-0 bind for pid {}, {}", pid, e.what());

564 - }

565 - }

566 -}

567 -

568 -void GnsPortTracker::ResolvePortZeroBind(DeferredPortLookup lookup)

532 +std::optional<GnsPortTracker::PortAllocation> GnsPortTracker::ResolvePortZeroBind(DeferredPortLookup lookup)

533 {

534 // The socket fd was already duplicated (via pidfd_getfd) while the target process

535 // was stopped by seccomp, so it remains valid even if the process has closed or

536 // reused the original fd number.

537

574 - // The bind() syscall is being completed asynchronously on the seccomp dispatcher

575 - // thread after CompleteRequest() unblocks it. Poll getsockname() briefly until

576 - // the kernel assigns a port.

538 + // The bind() syscall has been completed (CompleteRequest() already unblocked the

539 + // caller). Poll getsockname() briefly until the kernel assigns a port.

540 constexpr int maxRetries = 25;

578 - constexpr auto retryDelay = std::chrono::milliseconds(100);

541 + constexpr auto retryDelay = std::chrono::milliseconds(10);

542

543 in_port_t port = 0;

544 in6_addr address = {};

@@ -593,7 +556,7 @@ void GnsPortTracker::ResolvePortZeroBind(DeferredPortLookup lookup)

556 if (getsockname(lookup.DuplicatedSocketFd.get(), reinterpret_cast<sockaddr*>(&storage), &addrLen) != 0)

557 {

558 GNS_LOG_ERROR("Port-0 bind: getsockname failed for pid {} (errno {})", lookup.Pid, errno);

596 - return;

559 + return {};

560 }

561

562 resolvedFamily = static_cast<int>(storage.ss_family);

@@ -613,7 +576,7 @@ void GnsPortTracker::ResolvePortZeroBind(DeferredPortLookup lookup)

576 else

577 {

578 GNS_LOG_ERROR("Port-0 bind: unexpected address family ({}) for pid {}", resolvedFamily, lookup.Pid);

616 - return;

579 + return {};

580 }

581

582 if (port != 0)

@@ -625,16 +588,12 @@ void GnsPortTracker::ResolvePortZeroBind(DeferredPortLookup lookup)

588 if (port == 0)

589 {

590 GNS_LOG_ERROR("Port-0 bind: kernel did not assign a port for pid {} after retries", lookup.Pid);

628 - return;

591 + return {};

592 }

593

631 - PortAllocation allocation(port, resolvedFamily, lookup.Protocol, address);

594 GNS_LOG_INFO(

595 "Port-0 bind resolved: family ({}) port ({}) protocol ({}) for pid {}", resolvedFamily, port, lookup.Protocol, lookup.Pid);

634 - {

635 - std::lock_guard lock(m_resolvedMutex);

636 - m_resolvedQueue.push_back(std::move(allocation));

637 - }

596 + return PortAllocation(port, resolvedFamily, lookup.Protocol, address);

597 }

598

599 std::ostream& operator<<(std::ostream& out, const GnsPortTracker::PortAllocation& entry)

src/linux/init/GnsPortTracker.h

+1 -14

@@ -1,9 +1,7 @@

3 #pragma once

4 -#include <deque>

4 #include <map>

6 -#include <mutex>

5 #include <set>

6 #include <utility>

7 #include <optional>

@@ -146,9 +144,7 @@ private:

144

145 static wil::unique_fd DuplicateSocketFd(pid_t Pid, int SocketFd);

146

149 - void ResolvePortZeroBind(DeferredPortLookup lookup);

150 -

151 - void RunDeferredResolve();

147 + std::optional<PortAllocation> ResolvePortZeroBind(DeferredPortLookup lookup);

148

149 void TrackPort(PortAllocation allocation);

150

@@ -163,15 +159,6 @@ private:

159 std::shared_ptr<SecCompDispatcher> m_seccompDispatcher;

160

161 std::string m_networkNamespace;

166 -

167 - std::mutex m_deferredMutex;

168 - std::condition_variable m_deferredCv;

169 - std::deque<DeferredPortLookup> m_deferredQueue;

170 -

171 - // Resolved port-0 allocations posted by the background RunDeferredResolve thread

172 - // for the main Run() loop to process (keeps SocketChannel access single-threaded).

173 - std::mutex m_resolvedMutex;

174 - std::deque<PortAllocation> m_resolvedQueue;

162 };

163

164 std::ostream& operator<<(std::ostream& out, const GnsPortTracker::PortAllocation& portAllocation);

test/windows/NetworkTests.cpp

+37

@@ -2136,6 +2136,24 @@ class NetworkTests

2136 std::chrono::minutes(2)));

2137 }

2138

2139 + static void VerifyPortZeroRebindSucceeds()

2140 + {

2141 + // Verify that bind(0) -> close -> immediate rebind on the same port succeeds.

2142 + // Uses a perl one-liner to perform the entire sequence in a single process,

2143 + // matching the semantics of a native C test (no SO_REUSEADDR, same-process rebind).

2144 + VERIFY_ARE_EQUAL(

2145 + LxsstuLaunchWsl(L"perl -MSocket -e '"

2146 + L"socket(S1,AF_INET,SOCK_STREAM,0) or die;"

2147 + L"bind(S1,sockaddr_in(0,INADDR_ANY)) or die;"

2148 + L"my $port=(sockaddr_in(getsockname(S1)))[0];"

2149 + L"close(S1);"

2150 + L"socket(S2,AF_INET,SOCK_STREAM,0) or die;"

2151 + L"bind(S2,sockaddr_in($port,INADDR_ANY)) or die;"

2152 + L"close(S2)"

2153 + L"'"),

2154 + 0L);

2155 + }

2156 +

2157 template <typename T>

2158 static void VerifyNotBound(T& Address, int AddressFamily, int Protocol)

2159 {

@@ -4035,6 +4053,16 @@ class MirroredTests

4053 NetworkTests::VerifyPortZeroBindIsTracked(false);

4054 }

4055

4056 + WSL2_TEST_METHOD(PortZeroRebindSucceeds)

4057 + {

4058 + MIRRORED_NETWORKING_TEST_ONLY();

4059 +

4060 + m_config->Update(LxssGenerateTestConfig({.networkingMode = wsl::core::NetworkingMode::Mirrored}));

4061 + WaitForMirroredStateInLinux();

4062 +

4063 + NetworkTests::VerifyPortZeroRebindSucceeds();

4064 + }

4065 +

4066 WSL2_TEST_METHOD(ExplicitEphemeralBind)

4067 {

4068 MIRRORED_NETWORKING_TEST_ONLY();

@@ -4855,6 +4883,15 @@ class VirtioProxyTests

4883 NetworkTests::VerifyPortZeroBindIsTracked();

4884 }

4885

4886 + WSL2_TEST_METHOD(PortZeroRebindSucceeds)

4887 + {

4888 + VIRTIOPROXY_TEST_ONLY();

4889 +

4890 + m_config->Update(LxssGenerateTestConfig({.networkingMode = wsl::core::NetworkingMode::VirtioProxy}));

4891 +

4892 + NetworkTests::VerifyPortZeroRebindSucceeds();

4893 + }

4894 +

4895 WSL2_TEST_METHOD(HttpProxySimple)

4896 {

4897 VIRTIOPROXY_TEST_ONLY();