Re: [PATCH net v2 3/5] vsock: Enforce no-transport invariant for TCP_LISTEN sockets

[Michal Luczaj]

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.

>> @@ -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.

---

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.

[1]:
https://lore.kernel.org/netdev/20260922-vsock-connect-reset-closing-v3-0-78907b8200d4@xxxxxxx/

thanks,
Michal