Re: [PATCH net] vsock: Do not reset a TCP_CLOSING socket

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch intentionally leaves a reproducible kernel panic (`list_add double add`) unfixed when a socket's connect() races with driver unbind or VMCI detach.
--

commit 1aaee088d1b836a02d4d13c4a6eec8193f64408f
Author: Michal Luczaj <mhal@rbox.co>

vsock: Do not reset a TCP_CLOSING socket

Ensure connect() resets the socket only if it has never been established.
Handle the previously overlooked TCP_ESTABLISHED -> TCP_CLOSING transition
(on VIRTIO_VSOCK_OP_RST), which could race with the connect loop.

Link: https://lore.kernel.org/netdev/anzT1fREOSyHT99k@v4bel/

> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index f840498b58afb..eec5dd6daebb1 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
[ ... ]
> @@ -1875,7 +1872,8 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
>  	}
>  
>  	err = sock_error(sk);
> -	if (err) {
> +	if (err &&
> +	    sk->sk_state != TCP_ESTABLISHED && sk->sk_state != TCP_CLOSING) {
>  		sk->sk_state = TCP_CLOSE;
>  		sock->state = SS_UNCONNECTED;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does this error path still
leave a race condition that triggers a kernel panic?

If vsock_connect() races with an asynchronous transport reset like
virtio_vsock_reset_sock() or vmci_transport_handle_detach(), the transport sets
sk_state = TCP_CLOSE and sk_err = ECONNRESET, but does not remove the socket
from vsock_connected_table.

When the vsock_connect() loop exits due to the error, sk_state is TCP_CLOSE.
Since that is neither TCP_ESTABLISHED nor TCP_CLOSING, this block will execute
and set sock->state = SS_UNCONNECTED.

This allows user space to retry connect() on the same socket. If the second
connection handshake succeeds, the socket is re-inserted into the connected
table via vsock_insert_connected(), triggering a list_add double add panic.

Is there a way to prevent user space from retrying connect() in this scenario,
or should the socket be removed from the connected table during the transport
reset?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vsock-connect-reset-closing-v1-1-50298b9ccfbf@rbox.co?part=1