Skip to content

do-not-merge: virtionet port mapping POC #40145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
OneBlue wants to merge 4 commits into
base: feature/wsl-for-apps
Choose a base branch
from
Draft

do-not-merge: virtionet port mapping POC #40145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3d9ee86
virtiofs port mapping poc
OneBlue Apr 9, 2026
0d97d14
Format
OneBlue Apr 9, 2026
dbfb7d8
Save state
OneBlue Apr 17, 2026
302657c
Save state
OneBlue Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<package id="Microsoft.WSL.bsdtar" version="0.0.2-2" />
<package id="Microsoft.WSL.Dependencies.amd64fre" version="10.0.27820.1000-250318-1700.rs-base2-hyp" targetFramework="native" />
<package id="Microsoft.WSL.Dependencies.arm64fre" version="10.0.27820.1000-250318-1700.rs-base2-hyp" targetFramework="native" />
<package id="Microsoft.WSL.DeviceHost" version="1.2.10-0" />
<package id="Microsoft.WSL.DeviceHost" version="1.3.3.8" />
<package id="Microsoft.WSL.Kernel" version="6.6.114.1-1" targetFramework="native" />
<package id="Microsoft.WSL.LinuxSdk" version="1.20.0" targetFramework="native" />
<package id="Microsoft.WSL.TestData" version="0.4.0" />
Expand Down
30 changes: 30 additions & 0 deletions src/linux/init/GnsEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,36 @@ std::tuple<bool, int> GnsEngine::ProcessNextMessage()
wsl::shared::string::GuidToString<char>(createDeviceRequest.lowerEdgeAdapterId.value_or(emptyGuid)).c_str(),
gelnic.Name().c_str());
manager.InitializeLoopbackConfiguration(gelnic);

if (createDeviceRequest.disableLoopbackMirroring)
{
// Masquerade loopback-sourced traffic that gets forwarded to non-loopback interfaces.
// Without this, packets forwarded to containers arrive with source 127.0.0.1, and the
// container sends the reply to its own loopback instead of back through the bridge.
// iptables -t nat -A POSTROUTING -s 127.0.0.0/8 ! -o lo -j MASQUERADE

const char* argv[] = {
"/sbin/iptables", "-t", "nat", "-A", "POSTROUTING",
"-s", "127.0.0.0/8",
"!", "-o", "lo",
"-j", "MASQUERADE",
nullptr};

int status = -1;
if (UtilCreateProcessAndWait("/sbin/iptables", argv, &status) < 0 || status != 0)
{
throw RuntimeErrorWithSourceLocation(
std::format("iptables POSTROUTING MASQUERADE failed, status: {}", status));
}

// Enable route_localnet on all interfaces (including future ones like docker0).
// This allows packets with 127.x.x.x source/destination to be routed on non-loopback
// interfaces, which is required for conntrack to reverse the masquerade on reply packets.
WriteToFile("/proc/sys/net/ipv4/conf/all/route_localnet", "1");
WriteToFile("/proc/sys/net/ipv4/conf/default/route_localnet", "1");
WriteToFile("/proc/sys/net/ipv4/conf/eth0/route_localnet", "1");
}

break;
}
default:
Expand Down
2 changes: 1 addition & 1 deletion src/linux/init/WSLCInit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ void HandleMessageImpl(wsl::shared::SocketChannel& Channel, const WSLC_MOUNT& Me
THROW_LAST_ERROR_IF(mount(mountPoint, chrootTarget.c_str(), "none", MS_MOVE, nullptr) < 0);
}

THROW_LAST_ERROR_IF(MountInit(std::format("{}/wsl-init", target).c_str()) < 0); // Required to call /gns later
THROW_LAST_ERROR_IF(MountInit(std::format("{}/init", target).c_str()) < 0); // Required to call /gns later

Comment on lines 580 to 583
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MountInit is now bind-mounting WSL init into the chroot at {target}/init. However, WSLCEnableCrashDumpCollection/CreateCaptureCrashSymlink in this file still points the crash-capture symlink at /wsl-init (not /init), while the main init path used elsewhere in the repo is /init (LX_INIT_PATH). To avoid breaking crash dump collection in the chroot, align the symlink target with the mounted init path (or ensure /wsl-init is present inside the chroot).

Copilot uses AI. Check for mistakes.
// If it exists, mount /etc/resolv.conf
if (std::filesystem::exists("/etc/resolv.conf"))
Expand Down
8 changes: 7 additions & 1 deletion src/shared/inc/hns_schema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,10 @@ struct CreateDeviceRequest
std::wstring deviceName;
std::optional<GUID> lowerEdgeAdapterId;
std::optional<std::wstring> lowerEdgeDeviceName;
bool disableLoopbackMirroring{};
};

NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT_FROM_ONLY(CreateDeviceRequest, type, deviceName, lowerEdgeAdapterId, lowerEdgeDeviceName);
NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT_FROM_ONLY(CreateDeviceRequest, type, deviceName, lowerEdgeAdapterId, lowerEdgeDeviceName, disableLoopbackMirroring);

inline void to_json(nlohmann::json& j, const CreateDeviceRequest& request)
{
Expand All @@ -323,6 +324,11 @@ inline void to_json(nlohmann::json& j, const CreateDeviceRequest& request)
{
j["lowerEdgeDeviceName"] = request.lowerEdgeDeviceName.value();
}

if (request.disableLoopbackMirroring)
{
j["disableLoopbackMirroring"] = true;
}
}

struct ModifyGuestDeviceSettingRequest
Expand Down
116 changes: 81 additions & 35 deletions src/windows/common/VirtioNetworking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ void VirtioNetworking::StartPortTracker(wil::unique_socket&& socket)

m_gnsPortTrackerChannel.emplace(
std::move(socket),
[&](const SOCKADDR_INET& addr, int protocol, bool allocate) { return HandlePortNotification(addr, protocol, allocate); },
[&](const SOCKADDR_INET& addr, int protocol, bool allocate) {
return HandlePortNotification(addr, protocol, INETADDR_PORT(reinterpret_cast<const SOCKADDR*>(&addr)), allocate);
},
[](const std::string&, bool) {}); // TODO: reconsider if InterfaceStateCallback is needed.
}

Expand All @@ -94,14 +96,13 @@ try
}
CATCH_LOG()

HRESULT VirtioNetworking::HandlePortNotification(const SOCKADDR_INET& addr, int protocol, bool allocate) const noexcept
HRESULT VirtioNetworking::HandlePortNotification(const SOCKADDR_INET& addr, int protocol, uint16_t guestPort, bool allocate) const noexcept
{
if (addr.si_family == AF_INET6 && WI_IsFlagClear(m_flags, VirtioNetworkingFlags::Ipv6))
{
return S_OK;
}

int result = 0;
const auto ipAddress = (addr.si_family == AF_INET) ? reinterpret_cast<const void*>(&addr.Ipv4.sin_addr)
: reinterpret_cast<const void*>(&addr.Ipv6.sin6_addr);
const bool loopback = INET_IS_ADDR_LOOPBACK(addr.si_family, ipAddress);
Expand All @@ -111,10 +112,12 @@ HRESULT VirtioNetworking::HandlePortNotification(const SOCKADDR_INET& addr, int
// Only intercepting 127.0.0.1; any other loopback address will remain on 'lo'.
if (addr.Ipv4.sin_addr.s_addr != htonl(INADDR_LOOPBACK))
{
return result;
return S_OK;
}
}

auto hostPort = INETADDR_PORT(reinterpret_cast<const SOCKADDR*>(&addr));

if (WI_IsFlagSet(m_flags, VirtioNetworkingFlags::LocalhostRelay) && (unspecified || loopback))
{
SOCKADDR_INET localAddr = addr;
Expand All @@ -130,57 +133,99 @@ HRESULT VirtioNetworking::HandlePortNotification(const SOCKADDR_INET& addr, int
localAddr.Ipv6.sin6_port = addr.Ipv6.sin6_port;
}
}
result = ModifyOpenPorts(c_loopbackDeviceName, localAddr, protocol, allocate);
LOG_HR_IF_MSG(
E_FAIL, result != S_OK, "Failure adding localhost relay port %d", INETADDR_PORT(reinterpret_cast<const SOCKADDR*>(&localAddr)));

try
{
hostPort = ModifyOpenPorts(c_loopbackDeviceName, localAddr, hostPort, guestPort, protocol, allocate);
}
catch (...)
{
LOG_CAUGHT_EXCEPTION_MSG("Failure adding localhost relay port %d", guestPort);
}
}

if (!loopback)
{
const int localResult = ModifyOpenPorts(c_eth0DeviceName, addr, protocol, allocate);
LOG_HR_IF_MSG(E_FAIL, localResult != S_OK, "Failure adding relay port %d", INETADDR_PORT(reinterpret_cast<const SOCKADDR*>(&addr)));
if (result == 0)
try
{
hostPort = ModifyOpenPorts(c_eth0DeviceName, addr, hostPort, guestPort, protocol, allocate);
}
catch (...)
{
result = localResult;
LOG_CAUGHT_EXCEPTION_MSG("Failure adding relay port %d", guestPort);
}
}

return result;
return S_OK;
}
Comment on lines +137 to 160
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HandlePortNotification swallows all failures (catch-all around ModifyOpenPorts calls) and unconditionally returns S_OK. Since MapPort/UnmapPort call into this path, explicit port map/unmap requests can report success even when the guest update failed, leaving the system in an inconsistent state. Consider propagating HRESULTs for explicit operations (or splitting tracked notifications vs explicit map/unmap so explicit calls surface errors).

Copilot uses AI. Check for mistakes.

int VirtioNetworking::ModifyOpenPorts(_In_ PCWSTR tag, _In_ const SOCKADDR_INET& addr, _In_ int protocol, _In_ bool isOpen) const
uint16_t VirtioNetworking::ModifyOpenPorts(
_In_ PCWSTR tag, _In_ const SOCKADDR_INET& hostAddress, _In_ uint16_t HostPort, _In_ uint16_t GuestPort, _In_ int protocol, _In_ bool isOpen) const
{
if (protocol != IPPROTO_TCP && protocol != IPPROTO_UDP)
{
LOG_HR_MSG(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED), "Unsupported bind protocol %d", protocol);
return 0;
}
THROW_HR_IF_MSG(
HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED),
protocol != IPPROTO_TCP && protocol != IPPROTO_UDP,
"Unsupported bind protocol %d",
protocol);

auto lock = m_lock.lock_exclusive();
const auto server = m_guestDeviceManager->GetRemoteFileSystem(VIRTIO_NET_CLASS_ID, c_defaultDeviceTag);
if (server)
THROW_HR_IF(E_NOT_SET, !server);

const auto hostAddressStr = wsl::windows::common::string::SockAddrInetToString(hostAddress);

// format: tag={tag}[;host_port={port}];guest_port={port}[;listen_addr={addr}|;allocate=false][;udp]
std::wstring portString = std::format(L"tag={};guest_port={};listen_addr={}", tag, GuestPort, hostAddressStr.c_str());

Comment on lines +177 to +179
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listen_addr is interpolated into a semicolon-delimited options string that gets parsed on the guest side. Since MapVirtioNetPort’s ListenAddress ultimately comes from a caller-supplied string, it would be safer to validate/normalize it to a real IPv4/IPv6 address (e.g., via inet_pton/InetPton) and reject values containing ';' or other delimiters so a malformed address can’t perturb option parsing.

Copilot uses AI. Check for mistakes.
Comment on lines +177 to +179
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ModifyOpenPorts always includes listen_addr=... in the options string even when isOpen is false and allocate=false is appended. The comment immediately above indicates listen_addr and allocate=false are mutually exclusive; emitting both risks breaking close/unmap behavior. Build the string so listen_addr is only included for open mappings, and for closes emit allocate=false without listen_addr (matching the expected guest contract).

Copilot uses AI. Check for mistakes.
if (HostPort != WSLC_EPHEMERAL_PORT)
{
std::wstring portString = std::format(L"tag={};port_number={}", tag, INETADDR_PORT(reinterpret_cast<const SOCKADDR*>(&addr)));
if (protocol == IPPROTO_UDP)
{
portString += L";udp";
}
portString += std::format(L";host_port={}", HostPort);
}

if (!isOpen)
{
portString += L";allocate=false";
}
else
{
const auto addrStr = wsl::windows::common::string::SockAddrInetToWstring(addr);
portString += std::format(L";listen_addr={}", addrStr);
}
if (!isOpen)
{
portString += L";allocate=false";
}
Comment on lines +177 to +188
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ModifyOpenPorts always includes a listen_addr field in the options string ("...;listen_addr={}") even when isOpen==false (allocate=false). The comment above indicates listen_addr and allocate=false are mutually exclusive; emitting both (or emitting listen_addr with an empty value) risks breaking close/unmap behavior. Build the options string so listen_addr is only included for open mappings, and use allocate=false without listen_addr for closes if that’s what the guest expects.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@benhillis benhillis Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems legit


if (protocol == IPPROTO_UDP)
{
portString += L";udp";
}

const HRESULT addShareResult = server->AddShare(portString.c_str(), nullptr, 0);
WSL_LOG("MapVirtioPort", TraceLoggingValue(portString.c_str(), "PortString"), TraceLoggingValue(addShareResult, "Result"));

LOG_IF_FAILED(server->AddShare(portString.c_str(), nullptr, 0));
if (HostPort == WSLC_EPHEMERAL_PORT && isOpen && SUCCEEDED(addShareResult))
{
Comment on lines +198 to +199
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For ephemeral binds, this interprets IPlan9FileSystem::AddShare's HRESULT success code as S_OK + allocatedPort. This is a nonstandard encoding for HRESULT and could misbehave for success codes like S_FALSE or other non-zero successes. Consider returning the allocated port through an explicit out-param/response channel, or at least range-check that addShareResult is within [S_OK, S_OK + 65535] before decoding.

Suggested change
if (HostPort == WSLC_EPHEMERAL_PORT && isOpen && SUCCEEDED(addShareResult))
{
if (HostPort == WSLC_EPHEMERAL_PORT && isOpen)
{
THROW_IF_FAILED_MSG(addShareResult, "Failed to set virtionet port mapping: %ls", portString.c_str());
constexpr HRESULT c_maxEncodedEphemeralPortResult = S_OK + UINT16_MAX;
THROW_HR_IF_MSG(
E_UNEXPECTED,
addShareResult < S_OK || addShareResult > c_maxEncodedEphemeralPortResult,
"Unexpected AddShare success code for ephemeral port mapping: 0x%08x (%ls)",
static_cast<unsigned int>(addShareResult),
portString.c_str());

Copilot uses AI. Check for mistakes.
// For anonymous binds, the allocated host port is encoded in the return value.
return static_cast<uint16_t>(addShareResult - S_OK);
}

return 0;
THROW_IF_FAILED_MSG(addShareResult, "Failed to set virtionet port mapping: %ls", portString.c_str());
return HostPort;
}

HRESULT VirtioNetworking::MapPort(_In_ const SOCKADDR_INET& ListenAddress, _In_ USHORT GuestPort, _In_ int Protocol, _Out_ USHORT* AllocatedHostPort) const
try
{
RETURN_HR_IF(E_POINTER, AllocatedHostPort == nullptr);
RETURN_HR_IF_MSG(E_INVALIDARG, Protocol != IPPROTO_TCP && Protocol != IPPROTO_UDP, "Invalid protocol: %i", Protocol);

*AllocatedHostPort = 0;

return HandlePortNotification(ListenAddress, Protocol, GuestPort, true);
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MapPort never populates *AllocatedHostPort for anonymous binds. ModifyOpenPorts can compute an allocated host port (return value), but MapPort currently just calls HandlePortNotification and leaves *AllocatedHostPort as 0, so callers (e.g. WSLCVirtualMachine::MapPort) can’t discover the chosen host port. Plumb the allocated host port back to AllocatedHostPort (and ensure the COM method returns it).

Suggested change
return HandlePortNotification(ListenAddress, Protocol, GuestPort, true);
const auto hostPort = INETADDR_PORT(reinterpret_cast<const SOCKADDR*>(&ListenAddress));
*AllocatedHostPort = ModifyOpenPorts(c_defaultDeviceTag, ListenAddress, hostPort, GuestPort, Protocol, true);
return S_OK;

Copilot uses AI. Check for mistakes.
}
CATCH_RETURN()

HRESULT VirtioNetworking::UnmapPort(_In_ const SOCKADDR_INET& ListenAddress, _In_ USHORT GuestPort, _In_ int Protocol) const
try
{
RETURN_HR_IF(E_INVALIDARG, Protocol != IPPROTO_TCP && Protocol != IPPROTO_UDP);

const auto hostPort = INETADDR_PORT(reinterpret_cast<const SOCKADDR*>(&ListenAddress));
return HandlePortNotification(ListenAddress, Protocol, GuestPort, false);
}
CATCH_RETURN()

void VirtioNetworking::RefreshGuestConnection()
{
Expand Down Expand Up @@ -289,6 +334,7 @@ void VirtioNetworking::SetupLoopbackDevice()
createLoopbackDevice.deviceName = c_loopbackDeviceName;
createLoopbackDevice.type = hns::DeviceType::Loopback;
createLoopbackDevice.lowerEdgeAdapterId = m_localhostAdapterId.value();
createLoopbackDevice.disableLoopbackMirroring = WI_IsFlagSet(m_flags, VirtioNetworkingFlags::DisableLoopbackMirroring);
constexpr auto loopbackType = GnsMessageType(createLoopbackDevice);
m_gnsChannel.SendNetworkDeviceMessage(loopbackType, ToJsonW(createLoopbackDevice).c_str());
}
Expand Down
9 changes: 7 additions & 2 deletions src/windows/common/VirtioNetworking.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ enum class VirtioNetworkingFlags
DnsTunneling = 0x2,
Ipv6 = 0x4,
DnsTunnelingSocket = 0x8,
DisableLoopbackMirroring = 0x10,
};
DEFINE_ENUM_FLAG_OPERATORS(VirtioNetworkingFlags);

Expand Down Expand Up @@ -46,11 +47,15 @@ class VirtioNetworking : public INetworkingEngine
void FillInitialConfiguration(LX_MINI_INIT_NETWORKING_CONFIGURATION& message) override;
void StartPortTracker(wil::unique_socket&& socket) override;

HRESULT MapPort(_In_ const SOCKADDR_INET& ListenAddress, _In_ USHORT GuestPort, _In_ int Protocol, _Out_ USHORT* AllocatedHostPort) const;

HRESULT UnmapPort(_In_ const SOCKADDR_INET& ListenAddress, _In_ USHORT GuestPort, _In_ int Protocol) const;

private:
static void NETIOAPI_API_ OnNetworkConnectivityChange(PVOID context, NL_NETWORK_CONNECTIVITY_HINT hint);

HRESULT HandlePortNotification(const SOCKADDR_INET& addr, int protocol, bool allocate) const noexcept;
int ModifyOpenPorts(_In_ PCWSTR tag, _In_ const SOCKADDR_INET& addr, _In_ int protocol, _In_ bool isOpen) const;
HRESULT HandlePortNotification(const SOCKADDR_INET& addr, int protocol, uint16_t guestPort, bool allocate) const noexcept;
uint16_t ModifyOpenPorts(_In_ PCWSTR tag, _In_ const SOCKADDR_INET& hostAddress, _In_ uint16_t HostPort, _In_ uint16_t GuestPort, _In_ int protocol, _In_ bool isOpen) const;
void RefreshGuestConnection();
void SetupLoopbackDevice();
void SendDefaultRoute(const std::wstring& gateway, wsl::shared::hns::ModifyRequestType requestType);
Expand Down
45 changes: 44 additions & 1 deletion src/windows/service/exe/HcsVirtualMachine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Module Name:
#include "wslutil.h"
#include "lxinitshared.h"
#include "DnsResolver.h"
#include "string.hpp"

using namespace wsl::windows::common;
using helpers::WindowsBuildNumbers;
Expand Down Expand Up @@ -416,7 +417,8 @@ try
}
else if (m_networkingMode == WSLCNetworkingModeVirtioProxy)
{
wsl::core::VirtioNetworkingFlags flags = wsl::core::VirtioNetworkingFlags::Ipv6;
wsl::core::VirtioNetworkingFlags flags = wsl::core::VirtioNetworkingFlags::Ipv6 | wsl::core::VirtioNetworkingFlags::LocalhostRelay |
wsl::core::VirtioNetworkingFlags::DisableLoopbackMirroring;
if (FeatureEnabled(WslcFeatureFlagsDnsTunneling))
{
WI_SetFlag(flags, wsl::core::VirtioNetworkingFlags::DnsTunnelingSocket);
Expand Down Expand Up @@ -581,6 +583,47 @@ try
}
CATCH_RETURN()

HRESULT HcsVirtualMachine::MapVirtioNetPort(_In_ USHORT HostPort, _In_ USHORT GuestPort, _In_ int Protocol, _In_ LPCSTR ListenAddress, _Out_ USHORT* AllocatedHostPort)
try
{
RETURN_HR_IF(E_POINTER, AllocatedHostPort == nullptr || ListenAddress == nullptr);

*AllocatedHostPort = 0;

auto listenAddr = wsl::windows::common::string::StringToSockAddrInet(
wsl::shared::string::MultiByteToWide(ListenAddress));

listenAddr.Ipv4.sin_port = HostPort;

Comment on lines +593 to +597
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MapVirtioNetPort sets listenAddr.Ipv4.sin_port = HostPort without converting to network byte order. Callers (e.g. VMPortMapping::HostPort/SetHostPort) treat ports as host-order, and INETADDR_PORT expects the stored port to be in network order, so this will produce incorrect ports for non-zero values. Use htons(HostPort) or INETADDR_SET_PORT (and apply the same fix in UnmapVirtioNetPort).

Copilot uses AI. Check for mistakes.
std::lock_guard lock(m_lock);

auto* virtioNet = dynamic_cast<wsl::core::VirtioNetworking*>(m_networkEngine.get());
RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED), virtioNet == nullptr);

return virtioNet->MapPort(listenAddr, GuestPort, Protocol, AllocatedHostPort);
}
CATCH_RETURN()

HRESULT HcsVirtualMachine::UnmapVirtioNetPort(_In_ USHORT HostPort, _In_ USHORT GuestPort, _In_ int Protocol, _In_ LPCSTR ListenAddress)
try
{
RETURN_HR_IF(E_POINTER, ListenAddress == nullptr);

auto listenAddr = wsl::windows::common::string::StringToSockAddrInet(
wsl::shared::string::MultiByteToWide(ListenAddress));

listenAddr.Ipv4.sin_port = HostPort;

Comment on lines +612 to +616
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnmapVirtioNetPort also assigns listenAddr.Ipv4.sin_port = HostPort without converting to network byte order, which can cause unmap requests to target the wrong port and leak mappings. Set the port using htons(HostPort) / INETADDR_SET_PORT (consistent with how ports are stored in SOCKADDR_*).

Copilot uses AI. Check for mistakes.

std::lock_guard lock(m_lock);

auto* virtioNet = dynamic_cast<wsl::core::VirtioNetworking*>(m_networkEngine.get());
RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED), virtioNet == nullptr);

return virtioNet->UnmapPort(listenAddr, GuestPort, Protocol);
}
CATCH_RETURN()

void CALLBACK HcsVirtualMachine::OnVmExitCallback(HCS_EVENT* Event, void* Context)
try
{
Expand Down
4 changes: 4 additions & 0 deletions src/windows/service/exe/HcsVirtualMachine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ class HcsVirtualMachine
IFACEMETHOD(DetachDisk)(_In_ ULONG Lun) override;
IFACEMETHOD(AddShare)(_In_ LPCWSTR WindowsPath, _In_ BOOL ReadOnly, _Out_ GUID* ShareId) override;
IFACEMETHOD(RemoveShare)(_In_ REFGUID ShareId) override;
IFACEMETHOD(MapVirtioNetPort)
Copy link
Copy Markdown
Member

@benhillis benhillis Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for naming just MapPort and UnmapPort would be better.

(_In_ USHORT HostPort, _In_ USHORT GuestPort, _In_ int Protocol, _In_ LPCSTR ListenAddress, _Out_ USHORT* AllocatedHostPort) override;
IFACEMETHOD(UnmapVirtioNetPort)
(_In_ USHORT HostPort, _In_ USHORT GuestPort, _In_ int Protocol, _In_ LPCSTR ListenAddress) override;

private:
struct DiskInfo
Expand Down
2 changes: 1 addition & 1 deletion src/windows/service/exe/WSLCSessionManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings,
}
else
{
THROW_HR_IF(WSLC_E_INVALID_SESSION_NAME, Settings->DisplayName == nullptr || wcslen(Settings->DisplayName) == 0);
THROW_HR_IF(E_INVALIDARG, Settings->StoragePath != nullptr && wcslen(Settings->StoragePath) == 0);
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CreateSession’s validation now calls wcslen(Settings->DisplayName) without first validating that Settings->DisplayName is non-null and non-empty. This can AV if a caller passes a null DisplayName, and it also changes behavior by allowing empty names to reach the reserved-name check. Re-introduce a null/empty check (or guard the wcslen calls) before using DisplayName.

Suggested change
THROW_HR_IF(E_INVALIDARG, Settings->StoragePath != nullptr && wcslen(Settings->StoragePath) == 0);
THROW_HR_IF(E_INVALIDARG, Settings->StoragePath != nullptr && wcslen(Settings->StoragePath) == 0);
THROW_HR_IF(E_INVALIDARG, Settings->DisplayName == nullptr || Settings->DisplayName[0] == L'\0');

Copilot uses AI. Check for mistakes.
THROW_HR_IF(WSLC_E_INVALID_SESSION_NAME, wcslen(Settings->DisplayName) >= std::size(WSLCSessionInformation{}.DisplayName));
THROW_HR_IF_MSG(
Expand Down Expand Up @@ -176,6 +175,7 @@ void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings,

// Initialize settings for the default session.
std::unique_ptr<SessionSettings> defaultSettings;

if (Settings == nullptr)
{
defaultSettings = SessionSettings::Default(callerToken.get(), tokenInfo.Elevated, resolvedDisplayName);
Expand Down
Loading
Loading