Fix localhost relay hang on peer disconnect and ioctl interface name handling

[Eamon (Eamon2009)]

Summary

Fixed the relay worker thread hanging when a peer disconnects. The loop only checked for POLLIN, so POLLHUP and POLLERR from a closed connection were ignored, causing an infinite spin at 100% CPU. Now it handles all three events and shuts down cleanly.
Changed the relay exit condition from || to && so the thread waits for both directions to close before exiting. This prevents dropping buffered data when one side shuts down early.
Also made the interface name null-termination explicit in the ioctl handler. The code relied on struct zero-initialization, which works but is fragile. Now it copies sizeof-1 bytes and sets the last byte to '\0' directly.

also #41342

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR fixes two correctness issues in the Linux-side localhost relay path used by WSL: ensuring relay threads don’t exit early on a half-close, and ensuring interface names passed via the ioctl path are always NUL-terminated to avoid out-of-bounds reads on the host side.

Changes:

• Updated the relay worker loop termination condition so it exits only after both directions have closed, allowing buffered data to fully drain on half-close.
• Added explicit NUL-termination of LX_GNS_TUN_BRIDGE_REQUEST::InterfaceName after copying from ifreq, preventing non-terminated interface names from being interpreted as longer strings.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Eamon (Eamon2009)]

@microsoft-github-policy-service agree

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/linux/init/localhost.cpp:301

• This does not complete end-to-end half-close handling. The Windows side still exits as soon as either direction reaches EOF: src/windows/common/relay.cpp:319-322 uses ||, and its EOF paths then return from SocketRelay, closing the hvsocket before a response triggered by the forwarded FIN can arrive. A Windows client that calls shutdown(SD_SEND) therefore still cannot reliably receive the Linux server's response. The host relay must also keep the remaining direction alive (with its pending-I/O cleanup adjusted accordingly), ideally with an end-to-end half-close test.

                    if ((pollDescriptors[0].fd == -1) && (pollDescriptors[1].fd == -1))

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/linux/init/localhost.cpp:465

• The removal of the explanatory comment for why listen() is intercepted loses important context: listen() can implicitly autobind an ephemeral port, and without this rationale it's non-obvious why __NR_listen needs the same handling as bind(). Consider restoring this comment to preserve maintainability for future changes.

    seccompDispatcher->RegisterHandler(
        __NR_bind, [&portTracker](seccomp_notif* notification) { return portTracker.ProcessSecCompNotification(notification); });

    seccompDispatcher->RegisterHandler(
        __NR_listen, [&portTracker](seccomp_notif* notification) { return portTracker.ProcessSecCompNotification(notification); });

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Blue (OneBlue)]

LGTM, one minor comment

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/linux/init/localhost.cpp:318

• The relay loop now waits for both directions to close, but the per-fd handling only reacts to POLLIN. On Linux, poll() can signal a peer shutdown via POLLHUP/POLLERR without POLLIN, which would prevent UtilReadBuffer() from being called and could leave fd non--1, causing the thread to wait indefinitely.

                for (;;)
                {
                    if ((pollDescriptors[0].fd == -1) && (pollDescriptors[1].fd == -1))
                    {
                        return;

src/linux/init/localhost.cpp:518

• The memcpy change avoids copying all IFNAMSIZ bytes, but it still relies on request{} zero-initialization for null termination. Adding an explicit terminator makes the intent clear and keeps the safety property even if the struct initialization changes later (and it matches the PR description’s claim of explicit null termination).

        auto& ifRequest = *reinterpret_cast<ifreq*>(ifreqMemory->data());
        memcpy(request.InterfaceName, ifRequest.ifr_ifrn.ifrn_name, sizeof(request.InterfaceName) - 1);
        request.InterfaceUp = ifRequest.ifr_ifru.ifru_flags & IFF_UP;

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/linux/init/localhost.cpp:493

• The interface name is currently truncated with sizeof(InterfaceName) - 1, but the code relies on request{} zero-initialization to guarantee a trailing NUL. Making the terminator explicit would prevent accidental regressions if the initialization ever changes and makes the intent clearer.

        memcpy(request.InterfaceName, ifRequest.ifr_ifrn.ifrn_name, sizeof(request.InterfaceName) - 1);
        request.InterfaceUp = ifRequest.ifr_ifru.ifru_flags & IFF_UP;

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/linux/init/localhost.cpp:314

• The loop now waits for both read sides to close, but it still returns immediately on any write failure. If a peer half-closes its read side (or otherwise stops reading) while still sending data, writes can fail with EPIPE/ECONNRESET even though the opposite direction could still be drained. Consider treating expected write errors as closing just that relay direction (stop reading this source + shutdown the destination write side) instead of terminating the whole worker thread.

                        if (pollDescriptors[Index].revents & (POLLIN | POLLERR | POLLHUP))
                        {
                            bytesRead = UtilReadBuffer(pollDescriptors[Index].fd, buffer);
                            if (bytesRead == 0)

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/linux/init/localhost.cpp:311

• The relay loop now treats POLLERR/POLLHUP as readable, but the loop exit condition returns as soon as either side hits EOF (fd is set to -1). That prevents proper TCP half-close behavior (e.g., client FIN but server still sends a response), because the remaining direction never gets a chance to drain. The hang/spin risk is already addressed by handling POLLHUP; the loop should only terminate once both directions are closed.

                        if (pollDescriptors[Index].revents & (POLLIN | POLLERR | POLLHUP))

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/linux/init/localhost.cpp:319

• The PR description says the relay loop exit condition was changed from && to || so the loop exits when either side closes, but the code change here does the opposite (|| -> &&) and now only exits when both fds are -1. Please reconcile the intended behavior (exit on either close vs support half-close until both sides close) and align either the implementation or the PR description accordingly.

                for (;;)
                {
                    if ((pollDescriptors[0].fd == -1) && (pollDescriptors[1].fd == -1))
                    {
                        return;

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.