Re: [PATCH net v2 5/5] vsock: Handle sudden TCP_CLOSE during connect

On 9/16/26 14:31, Stefano Garzarella wrote:

On Tue, Sep 15, 2026 at 03:15:16PM +0200, Michal Luczaj wrote:

quoted

Virtio/PM events are serviced by virtio_vsock_reset_sock(), which resets

What "PM" means here?

Power Management, namely virtio_vsock_driver.freeze which is defined under
CONFIG_PM_SLEEP. I'll generalize this comment, as suggested below.

quoted

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.

Sure, will do.

Maybe we should document this behaviour also on top of this file.

OK, do you mind if I do that as a follow up? I worry top of the file needs
few more updates.

quoted

+		if (READ_ONCE(vsk->peer_shutdown)) {
+			err = -ECONNRESET;

Is ECONNRESET a valid connect() error to return?

quoted

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

I'm not sure I follow the logic. SHUTDOWN_MASK gets set no matter what's
the state at vmci_transport_handle_detach(). That means even if vmci's
reconnect succeeds, all the recvmsg()/sendmsg() at af_vsock layer would
fail anyway. Am I missing something?

thanks,
Michal