Cap guest usage of host ephemeral range to half of the ports in mirrored mode#41085
Cap guest usage of host ephemeral range to half of the ports in mirrored mode#41085FetoiuCatalin wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates mirrored-mode guest port allocation behavior so the guest cannot consume more than half of the host’s ephemeral port range (per protocol), reducing the risk of host ephemeral port exhaustion.
Changes:
- Add per-protocol ephemeral-range “cap” computation and enforce it during guest port reservation requests in
GuestNetworkService. - Track host-ephemeral ports “in use” (seeded by overlap with the guest’s reserved ephemeral range, then incremented/decremented on reserve/release).
- Remove/adjust tests that previously asserted “deny any bind into host ephemeral range outside guest reserved range” semantics.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/windows/NetworkTests.cpp | Reformatting plus removal of mirrored-mode tests that asserted prior host-ephemeral deny behavior. |
| src/windows/service/exe/WslCoreGuestNetworkService.h | Adds declarations/state to compute/enforce per-protocol ephemeral cap and track in-use counts. |
| src/windows/service/exe/WslCoreGuestNetworkService.cpp | Implements cap computation + overlap seeding, and enforces cap during port allocation requests. |
Comments suppressed due to low confidence (1)
test/windows/NetworkTests.cpp:4294
- The tests that previously validated mirrored-mode behavior for guest binds into the host ephemeral range were removed, but there isn't a replacement test that validates the new cap semantics (i.e., that guest allocations in the host ephemeral range are allowed up to the cap and denied once the cap is reached). Without coverage, regressions here could silently reintroduce host ephemeral exhaustion or over-restrict guest binds.
WSL2_TEST_METHOD(NonRootNamespaceEphemeralBind)
{
MIRRORED_NETWORKING_TEST_ONLY();
| uint32_t wsl::core::networking::GuestNetworkService::ComputeHostEphemeralPortCap(int Protocol) const noexcept | ||
| { | ||
| const auto& range = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortRange : m_hostTcpEphemeralPortRange; | ||
| if (range.second < range.first) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| // Cap the guest at (at most) half of the host ephemeral range so it can't exhaust the host's ports. | ||
| return (static_cast<uint32_t>(range.second) - range.first + 1) / 2; | ||
| } |
| result = m_releasePort.value()(it->second.Handle); | ||
| m_reservedPorts.erase(it); | ||
|
|
||
| if (isHostEphemeralPort) | ||
| { | ||
| auto& portsInUse = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortsInUse : m_hostTcpEphemeralPortsInUse; | ||
| if (portsInUse > 0) | ||
| { | ||
| portsInUse--; | ||
| } | ||
| } |
There was a problem hiding this comment.
please strongly consider this change
| WSL2_TEST_METHOD(NonRootNamespaceEphemeralBind) | ||
| { | ||
| MIRRORED_NETWORKING_TEST_ONLY(); |
| const auto& range = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortRange : m_hostTcpEphemeralPortRange; | ||
| if (range.second < range.first) | ||
| { | ||
| return 0; |
There was a problem hiding this comment.
can we ASSERT this an have tests on CHK builds?
this should never happen - it's a terrible code bug when it does
| } | ||
|
|
||
| // Cap the guest at (at most) half of the host ephemeral range so it can't exhaust the host's ports. | ||
| return (static_cast<uint32_t>(range.second) - range.first + 1) / 2; |
There was a problem hiding this comment.
why does this need a static_cast<uint32_t>? what type is range.second?
| result = m_releasePort.value()(it->second.Handle); | ||
| m_reservedPorts.erase(it); | ||
|
|
||
| if (isHostEphemeralPort) | ||
| { | ||
| auto& portsInUse = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortsInUse : m_hostTcpEphemeralPortsInUse; | ||
| if (portsInUse > 0) | ||
| { | ||
| portsInUse--; | ||
| } | ||
| } |
There was a problem hiding this comment.
please strongly consider this change
| @@ -4283,66 +4283,6 @@ class MirroredTests | |||
| VERIFY_IS_TRUE(canBindUdp); | |||
There was a problem hiding this comment.
we should add tests for this behavior
There was a problem hiding this comment.
definitely need to validate that we can hit our limit, then they fail, then once sockets are closed and it falls below our limit, we can allocate again. then repeat that pattern (hit limit -> fail, fall below limit->succeed, repeat) in the same test.
There was a problem hiding this comment.
I was hesitant to do this as even for the smallest host range (which seems to be enforced to 255), we would need to bind 128 times and I was not sure if that's too heavy for an automated test
I can write it and see if it takes too long to run. I can set the host range temporarily to 255 and then revert it to the original host range
There was a problem hiding this comment.
calling bind 128 times is not taxing on anything - it should be just fine
| if (isHostEphemeralPort) | ||
| { | ||
| auto& portsInUse = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortsInUse : m_hostTcpEphemeralPortsInUse; | ||
| if (portsInUse > 0) |
There was a problem hiding this comment.
when would this be == 0 if isHostEphemeralPort is true?
should we add an assert here too and go through testing?
Summary of the issue and fix
https://github.com/microsoft/WSL/pull/40597/changes introduced an app-compat issue where scenarios such as Docker can happen to use ports from the host ephemeral range, which are now denied. To preserve app-compat and also prevent the container from exhausting the host ephemeral port range, add a cap for the number of ports the container can use, set to half of the size of the host ephemeral port range
Testing:
Manual testing by setting the host ephemeral range to a small size (255 ports), then using a bash script to attempt binding to all the ports in the range and confirming that only half (127) can bind
Existing automated NetworkTests
Closes: [WSL 2.9.3] Docker fails to bind ports with "address already in use" when networkingMode=mirrored is enabled #40984