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?
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 <[email protected]>
---
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
