The commit message of the quick, backportable fix 44b83210c8ad concluded
with:
Alternately we could add some flag to the listening socket which
indicates that no new connections are to be accepted, but I think
this will require some changes to the generic socket code.
This commit is the generic socket code change mentioned there.
For background: a protocol clones a socket from a listener with
solisten_clone() and later puts it on the listen queue with
solisten_enqueue(). If the listener is closed in between, soclose()
has already drained the queues, and the new socket lands on a queue
nobody will ever look at again, so it leaks.
This is almost not a problem today:
- TCP is safe because it holds the listening inpcb lock across the whole thing.
- Unix sockets connected to by file are safe because we restored the "a bit ugly" vnode and mutex threading.
- Unix sockets connected by descriptor (new) are not safe. They still face this problem (though we should make a reproducer to prove this).
This generic socket code change has soclose() mark the listener
SS_LISTENCLOSED while it drains the queues, and has
solisten_enqueue() refuse with ECONNABORTED once that is set. The
caller then soabort()s the socket, the same fate it would have met had
it made it onto the queue just before the close.
solisten_enqueue() now returns an error, with the complete/incomplete
result moved to an optional out-parameter.
Next up would be reverting the "ugly" vnode and mutex threading in the
Unix case, which is no longer needed to prevent leaks.
Note that even in the TCP case, I think this brings some benefit. While
the KASSERT is new, the invariant that it asserts (closing at that point
should be impossible) is not. Now the invariant is explicitly modeled,
which is good for future readers of the code, and hardens the kernel
in INVARIANTS mode.
Assisted-by: Claude Code (Opus 5.5)
Fixes: 6563dcb6b1f5 ("unix: allow connectat(2) to name the peer socket by descriptor")