On Tue, Sep 22, 2026 at 03:18:13PM +0200, Michal Luczaj wrote:
On 9/16/26 14:30, Stefano Garzarella wrote:
On Tue, Sep 15, 2026 at 03:15:14PM +0200, Michal Luczaj wrote:
A non-blocking connect() running in parallel with a blocking connect(),
combined with a racy listen() that hits right after a connect timeout:
TCP_SYN_SENT -> TCP_CLOSE -> TCP_LISTEN, while the connect() loop is still
in progress.
Enforce the invariant. Prevent a socket from becoming a listener after
acquiring a transport.
We should improve this comment; it's not entirely clear to me, TBH.
The race I was thinking about:
sk is CLOSE UNCONNECTED
non-blocking connect():
sk := SYN_SENT CONNECTING
enqueue vsock_connect_timeout()
blocking connect():
release_sock()
schedule_timeout()
vsock_connect_timeout():
sk := CLOSE UNCONNECTED
listen():
sk := LISTEN UNCONNECTED
lock_sock()
sk is TCP_LISTEN UNCONNECTED
It's not really critical (blocking connect() just timeouts), but I thought
the invariant should be enforced once and for all.
I see, would it better to do this change in net-next?
@@ -1973,13 +1973,13 @@ static int vsock_listen(struct socket *sock, int
backlog)
goto out;
}
- if (sock->state != SS_UNCONNECTED) {
+ vsk = vsock_sk(sk);
+
+ if (sock->state != SS_UNCONNECTED || vsk->transport) {
Are we changing the behavior when an error occurs?
If we call `connect()` on a socket (with no others running in parallel),
it fails, and then when we call `listen()`, it now fails, whereas before
it didn't. Can this happen? Is that what we want?
Ah, true, I didn't consider that. So yeah, we'd changing the behaviour.
If so, we should mention it at least in the commit description; if not,
perhaps we should unassign the transport in the `connect` call.
Do you mean immediately un-assign on every transition from SYN_SENT to
CLOSE (failure, timeout, signal)? Then we could also drop the re-assign
logic. I think that's a nice idea.
yeah, that!
---
I've addressed all your other comments for v2 and went through Ashiko's
reports (side effects of lockless peer_shutdown write, imperfect
no-transport TCP_LISTENER enforcement). I've decided to try the
eager-unassign approach. I think/hope this way we sidestep the lockless
writes and enforce the invariant without breaking the API, while fixing the
bugs.
This should probably be RFC, but I'm posting as v3[1] so netdev's LLM can
have a go (too). Hope I'm not breaking any workflow. Let me know what you
think.
I think you can add RFC also on a v3 patch, just to make it clear you
are not sure it's ready to be merged.
That said, thanks for that :-) I'll take a look today or next week
because I'm off tomorrow.
I'm just worried it's becoming too big for net. Anyway, I'll comment
there.
Thanks,
Stefano
