socket.fromfd() leaks the dup()'d fd when family/type/proto are invalid #160561

Activity

1. 0x-bala

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Oct 24, 2026
2. wlkr73 commented on Oct 24, 2026

   

   wlkr73

   on Oct 24, 2026

   More actions

   I can reproduce this on main, and the leak is in fromfd itself rather than in socket().

   Lib/socket.py:574 dup()s first and then hands the descriptor to socket(), so when the constructor raises for a bad family/type/proto nothing ever closes the duplicate.

   On 3.16.0a1+dev (main at 53b0004, free-threading build, descriptors counted through /proc/self/fd):

   proto 1<<100: raised=OverflowError fd_delta_over_3_calls=3
   type 1<<100: raised=OverflowError fd_delta_over_3_calls=3
   family 1<<100: raised=OverflowError fd_delta_over_3_calls=3
   valid control: fd_delta_over_5_calls=0
   

   The arguments from your report (family 9999, family -1, type 9999, proto 1<<30) all return a socket here and do not leak, so on this build the leak needs an argument that overflows a C int. The defect is the same shape either way.

   Closing the duplicate when the constructor raises fixes it:

        nfd = dup(fd)
   -    return socket(family, type, proto, nfd)
   +    try:
   +        return socket(family, type, proto, nfd)
   +    except BaseException:
   +        close(nfd)
   +        raise

   The same three bad calls with that in place give fd_delta_over_3_calls=0 on all three lines, control still 0. A testFromFdInvalidArgsDoesNotLeak case, new in the diff's test hunk because main has no such test, that takes the same count before and after one rejected call reads 3 open descriptors against 0 with the change, so run against current main's Lib/socket.py it fails and against the patched one it passes: with HEAD's Lib/socket.py shadowed in, ./python -m unittest test.test_socket.BasicTCPTest.testFromFdInvalidArgsDoesNotLeak reports AssertionError: 7 != 6 (FAIL), and the same command against the patched Lib/socket.py on the same binary is OK. ./python -m test test_socket -m 'testFromFd*' is 4/4 OK and the two existing testFromFd cases are unchanged. The whole of test_socket on the patched tree is run=755 skipped=50, Result: SUCCESS in 40.3 s, so nothing else in that file moves.

   The diff is 22 insertions across Lib/socket.py and Lib/test/test_socket.py. Say the word and I will open a PR, or the whole diff can go here.
3. 0x-bala commented on Oct 24, 2026

   

   0x-bala

   on Oct 24, 2026

   Author

   More actions

   Thanks for confirming and for the thorough analysis. The fix looks right to me. Please go ahead and open the PR; I’m happy to review or test it on my side.
4. wlkr73 commented on Oct 24, 2026

   

   wlkr73

   on Oct 24, 2026

   More actions

   #160579 is up with the fix and a test that counts /proc/self/fd either side of one rejected fromfd call. The test fails against unmodified Lib/socket.py (AssertionError: 7 != 6) and passes on the branch. test_socket is green on the free-threading debug build (run=755, skipped=50).
5. 0x-bala commented on Oct 24, 2026

   

   0x-bala

   on Oct 24, 2026

   Author

   More actions

   The regression test counts the entries in /proc/self/fd before and after one rejected fromfd() call. It fails against unmodified Lib/socket.py (AssertionError: 7 != 6) and passes on the branch. test_socket is green on the free-threading debug build (run=755, skipped=50).
6. 0x-bala commented on Oct 24, 2026

   

   0x-bala

   on Oct 24, 2026

   Author

   More actions

   Thanks for the PR and the careful analysis. I re-ran each value separately on 3.14.4 / Linux (GCC 15.2.0). Family 9999, family -1, type 9999 and proto 1<<30 return a socket without leaking, as you saw. Only arguments that overflow a C int leak, one fd per call: family, type and proto set to 1<<100 each raise OverflowError after the dup() and leave a delta of 3 over 3 calls. The valid control is 0. I’ll edit the issue description so it no longer claims the original values leak. The fix looks right to me and I’ll review the PR.
7. wlkr73

   added a commit that references this issue on Oct 24, 2026

   pythongh-160561: Close the duplicated descriptor when fromfd() is rej…

   ...

   c7af374
8. wlkr73 commented on Oct 24, 2026

   

   wlkr73

   on Oct 24, 2026

   More actions

   Thanks for re-running them, that matches my measurements exactly: with 1 << 100 in family, type or proto each call raises OverflowError after the dup() and leaves one descriptor behind, three over three calls, and the valid control leaves none.

   Since your run shows all three arguments reach the same path, I extended the regression test to cover each of them as a subtest rather than only proto. On the unpatched Lib/socket.py that is three failures (7 != 6, 8 != 7, 9 != 8); with the guard all three pass. The branch is force-pushed, so gh-160579 now carries the wider test.
9. 0x-bala commented on Oct 24, 2026

   

   0x-bala

   on Oct 24, 2026

   Author

   More actions

   Tested the patched Lib/socket.py from this PR branch on 3.14.4 / Linux (GCC 15.2.0). I loaded that file directly over the system 3.14.4 interpreter and confirmed socket.file pointed at it. All eight cases in my script now show a delta of 0 over 3 calls: the valid control, the four values from the original report, and family/type/proto set to 1<<100. The three 1<<100 cases still raise OverflowError but no longer leave a descriptor behind (previously a delta of 3 each). The guard looks right to me. I haven’t built CPython or run the new unittest myself, so I can’t independently confirm the 7 != 6 failure numbers.
10. aisk

    added

    topic-socketRelated to the socket, socketserver and selectors modulesRelated to the socket, socketserver and selectors modules

    on Oct 24, 2026