Re: [PATCH net v2 5/5] vsock: Handle sudden TCP_CLOSE during connect
From: Stefano Garzarella <sgarzare@redhat.com>
Date: 2026-09-16 12:31:43
Also in:
kvm, lkml, virtualization
On Tue, Sep 15, 2026 at 03:15:16PM +0200, Michal Luczaj wrote:
Virtio/PM events are serviced by virtio_vsock_reset_sock(), which resets
What "PM" means here?
quoted hunk ↗ jump to hunk
each connected socket. The reset is done under vsock_table_lock but without taking lock_sock(), so from the point of view of vsock_connect() - locklessly. The same pattern exists in VMCI's vmci_transport_handle_detach() and vhost's vhost_vsock_reset_orphans(). The complexity of connect() comes from the fact that: 1. the virtio transport can be reassigned, so the old transport must be safely released; 2. a failed connect can be followed by a retry, so the socket must be reverted to a sensible state. Both cases apply only as long as the socket has not yet established a connection. While connect() waits for TCP_SYN_SENT -> TCP_ESTABLISHED, other transitions can also occur: TCP_SYN_SENT -> TCP_CLOSE on connection failure, timeout or signal TCP_SYN_SENT -> TCP_ESTABLISHED -> TCP_CLOSING on VIRTIO_VSOCK_OP_RST TCP_SYN_SENT -> TCP_ESTABLISHED -> [TCP_CLOSING ->] TCP_CLOSE on event This further complicates connect(). Rather than making every event handler drop the socket from connected_table or adapting connect() to handle more transitions (while missing proper locking), use vsk->peer_shutdown as a poison flag. Whatever state an event leaves the socket in, the flag bricks it and prevents suspicious transport reassignments or TCP_SYN_SENT retransmissions. Fixes: d021c344051a ("VSOCK: Introduce VM Sockets") Signed-off-by: Michal Luczaj <redacted> --- net/vmw_vsock/af_vsock.c | 6 ++++++ 1 file changed, 6 insertions(+)diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index adf3f018347e..972952d04a81 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c@@ -1743,6 +1743,12 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,goto out; } + /* Virtio/PM events are serviced locklessly. */
IMO we should be generic here (i.e. don't mention virtio or mention it like one of the transport, but IIUC also VMCI does something similar) and also we should explain better why we are doing this, like you did in the commit description. Maybe we should document this behaviour also on top of this file.
+ if (READ_ONCE(vsk->peer_shutdown)) {
+ err = -ECONNRESET;Is ECONNRESET a valid connect() error to return?
+ goto out; + } +
From LLM reviewing, can you check if it's valid? :
- M (net/vmw_vsock/af_vsock.c:1747): VMCI regression. vmci_transport_handle_detach() sets
peer_shutdown = SHUTDOWN_MASK unconditionally and then special-cases TCP_SYN_SENT with the
comment "we treat the detach event like a reset" — i.e. a connect() retry is the expected
recovery. It is reachable for a non-connected socket via vmci_transport_peer_detach_cb() (which
uses trans->sk, not the connected table). Since vsock_assign_transport() only clears
peer_shutdown when the transport actually changes (af_vsock.c:671-689), the retry now hits the
new check and returns -ECONNRESET forever: the fd is permanently bricked where it previously
reconnected.
Thanks,
Stefano
/* Set the remote address that we are connecting to. */ memcpy(&vsk->remote_addr, remote_addr, sizeof(vsk->remote_addr)); -- 2.55.0