Skip to content

[net] close the listening socket on setup failure and open sockets CLOEXEC - #349

Open
ManoharPaturi wants to merge 1 commit into
google:masterfrom
ManoharPaturi:pr-net-sockhygiene
Open

ManoharPaturi wants to merge 1 commit into
google:masterfrom
ManoharPaturi:pr-net-sockhygiene

Conversation

@ManoharPaturi

Copy link
Copy Markdown

Problem

Two descriptor issues in getRecvSocket():

  1. If fcntl(O_NONBLOCK) or setsockopt(SO_REUSEADDR) fails after the socket is created, the function returns -1 without closing the socket — the fd leaks for the lifetime of the supervisor. In --mode listen with --max_conns retry loops this accumulates.
  2. The listening socket was created without SOCK_CLOEXEC, and acceptConn() accepted connections without SOCK_CLOEXEC as well, so both survive any execve() in the supervisor process (e.g. the newuidmap/newgidmap helper exec path in subproc::systemExe(), which has no fd sweep of its own).

Change

  • close(sockfd) before the error returns after socket creation.
  • SOCK_CLOEXEC on the socket() creation and on the accept4() flags.

Testing

Built and ran the full test suite (unit + cmdline) plus an ASan/UBSan build on Ubuntu 24.04 — all green, no new findings.

…OEXEC

getRecvSocket() leaked the freshly created listening socket when
fcntl(O_NONBLOCK) or setsockopt(SO_REUSEADDR) failed. The listening
socket was also created without SOCK_CLOEXEC and accepted
connections without SOCK_CLOEXEC, so both stayed open across any
execve() in the supervisor process.
@ManoharPaturi

Copy link
Copy Markdown
Author

Small branch hygiene fix: my local CI workflow file accidentally ended up in this branch — force-pushed the branch rebuilt from current master with only the intended change. The diff is now just the patch itself; the actions scan failure from earlier is gone.

@ManoharPaturi

ManoharPaturi commented Sep 27, 2026 •

Copy link
Copy Markdown
Author

@robertswiecki Could you review this when convenient? It addresses two descriptor issues in getRecvSocket(): the listening socket was leaked when fcntl() or setsockopt() failed after creation, and both the listener and accepted connections were opened without SOCK_CLOEXEC. Happy to split the two changes into separate PRs if preferred.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant