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 <[email protected]> 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/[email protected]?part=1
