[PATCH net v2 0/5] vsock: Fix connect() races

[PATCH net v2 0/5] vsock: Fix connect() races

[Michal Luczaj]

Handle racy socket changes in connect(). Plus related tweaks.

---
Changes in v2:
- CC: drop bouncing addresses
- Add more related fixes
- Handle ENOBUFS (Bobby)
- Consume sk_err (Stefano)
- Link to v1: https://patch.msgid.link/20260909-vsock-connect-reset-closing-v1-1-50298b9ccfbf@rbox.co

---
Michal Luczaj (5):
      vhost/vsock: Fix socket state constant
      vsock/virtio: Streamline socket reset on transport/PM event
      vsock: Enforce no-transport invariant for TCP_LISTEN sockets
      vsock: Do not reset a TCP_CLOSING socket
      vsock: Handle sudden TCP_CLOSE during connect

 drivers/vhost/vsock.c            |  2 +-
 net/vmw_vsock/af_vsock.c         | 32 ++++++++++++++++++--------------
 net/vmw_vsock/virtio_transport.c |  3 +++
 3 files changed, 22 insertions(+), 15 deletions(-)
---
base-commit: 0654f4dba1fbc697f2653aba30cd68587fcbf10e
change-id: 20260820-vsock-connect-reset-closing-98dd28769d7f

Best regards,
--  
Michal Luczaj <mhal@rbox.co>

[PATCH net v2 1/5] vhost/vsock: Fix socket state constant

[Michal Luczaj]

Refactor under Fixes missed one: SS_UNCONNECTED (1) -> TCP_CLOSE (7).
Until now, sk->sk_state = 1 stood for TCP_ESTABLISHED.

Fixes: 3b4477d2dcf2 ("VSOCK: use TCP state constants for sk_state")
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 drivers/vhost/vsock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index abed1fbcf66c..6051533456f6 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -774,7 +774,7 @@ static void vhost_vsock_reset_orphans(struct sock *sk)
 
 	sock_set_flag(sk, SOCK_DONE);
 	vsk->peer_shutdown = SHUTDOWN_MASK;
-	sk->sk_state = SS_UNCONNECTED;
+	sk->sk_state = TCP_CLOSE;
 	sk->sk_err = ECONNRESET;
 	sk_error_report(sk);
 }

-- 
2.55.0

[PATCH net v2 2/5] vsock/virtio: Streamline socket reset on transport/PM event

[Michal Luczaj]

Follow vhost's vhost_vsock_reset_orphans() and VMCI's
vmci_transport_handle_detach(): set SHUTDOWN_MASK, which will come handy
later in the series.

Note that commit c38f57da428b ("vhost/vsock: fix reset orphans race with
close timeout") fixed a race between pending close timer, i.e.
virtio_transport_close_timeout(), and a transport/PM reset. But here we
never set SOCK_DONE, hence no race.

Fixes: 0ea9e1d3a9e3 ("VSOCK: Introduce virtio_transport.ko")
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 net/vmw_vsock/virtio_transport.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 4f9aa9c4c3aa..4d6991321699 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -413,11 +413,14 @@ static void virtio_vsock_event_fill(struct virtio_vsock *vsock)
 
 static void virtio_vsock_reset_sock(struct sock *sk)
 {
+	struct vsock_sock *vsk = vsock_sk(sk);
+
 	/* vmci_transport.c doesn't take sk_lock here either.  At least we're
 	 * under vsock_table_lock so the sock cannot disappear while we're
 	 * executing.
 	 */
 
+	vsk->peer_shutdown = SHUTDOWN_MASK;
 	sk->sk_state = TCP_CLOSE;
 	sk->sk_err = ECONNRESET;
 	sk_error_report(sk);

-- 
2.55.0

[PATCH net v2 3/5] vsock: Enforce no-transport invariant for TCP_LISTEN sockets

[Michal Luczaj]

A non-blocking connect() running in parallel with a blocking connect(),
combined with a racy listen() that hits right after a connect timeout:
TCP_SYN_SENT -> TCP_CLOSE -> TCP_LISTEN, while the connect() loop is still
in progress.

Enforce the invariant. Prevent a socket from becoming a listener after
acquiring a transport.

Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 net/vmw_vsock/af_vsock.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index f840498b58af..499e902becfa 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1973,13 +1973,13 @@ static int vsock_listen(struct socket *sock, int backlog)
 		goto out;
 	}
 
-	if (sock->state != SS_UNCONNECTED) {
+	vsk = vsock_sk(sk);
+
+	if (sock->state != SS_UNCONNECTED || vsk->transport) {
 		err = -EINVAL;
 		goto out;
 	}
 
-	vsk = vsock_sk(sk);
-
 	if (!vsock_addr_bound(&vsk->local_addr)) {
 		err = -EINVAL;
 		goto out;

-- 
2.55.0

[PATCH net v2 4/5] vsock: Do not reset a TCP_CLOSING socket

[Michal Luczaj]

Handle the previously overlooked TCP_ESTABLISHED -> TCP_CLOSING
transition (on VIRTIO_VSOCK_OP_RST), which could race with the connect
loop.

Resetting a socket that is still present in connected_table can lead to
memory corruption. The reporter noted lost transports for in-flight skbs,
and I have reproduced crashes caused by re-insertion into connected_table.

  list_add double add: new=, prev=, next=.
  kernel BUG at lib/list_debug.c:35!
  Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
  Workqueue: vsock-loopback vsock_loopback_work
  RIP: 0010:__list_add_valid_or_report+0x11f/0x130
  Call Trace:
   vsock_insert_connected.cold+0xe/0x13
   virtio_transport_recv_pkt+0x10e9/0x1460
   vsock_loopback_work+0x305/0x480
   process_one_work+0xe4c/0x1560
   worker_thread+0x4f1/0xd60
   kthread+0x36e/0x470
   ret_from_fork+0x47b/0x6b0
   ret_from_fork_asm+0x1a/0x30

Drop the redundant err=0 and the inaccurate comment above signal_pending().
This fix is supplementary to commit 002541ef650b ("vsock: Ignore
signal/timeout on connect() if already established"). Details under Link.

Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Link: https://lore.kernel.org/netdev/anzT1fREOSyHT99k@v4bel/
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 net/vmw_vsock/af_vsock.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 499e902becfa..adf3f018347e 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1712,7 +1712,6 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
 	long timeout;
 	DEFINE_WAIT(wait);
 
-	err = 0;
 	sk = sock->sk;
 	vsk = vsock_sk(sk);
 
@@ -1834,23 +1833,22 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
 		timeout = schedule_timeout(timeout);
 		lock_sock(sk);
 
-		/* Connection established. Whatever happens to socket once we
-		 * release it, that's not connect()'s concern. No need to go
+		/* Connection was established. Whatever happens to socket once
+		 * we release it, that's not connect()'s concern. No need to go
 		 * into signal and timeout handling. Call it a day.
 		 *
 		 * Note that allowing to "reset" an already established socket
 		 * here is racy and insecure.
 		 */
-		if (sk->sk_state == TCP_ESTABLISHED)
-			break;
+		if (sk->sk_state == TCP_ESTABLISHED ||
+		    sk->sk_state == TCP_CLOSING) {
+			err = 0;
+			goto out_wait;
+		}
 
 		/* If connection was _not_ established and a signal/timeout came
 		 * to be, we want the socket's state reset. User space may want
 		 * to retry.
-		 *
-		 * sk_state != TCP_ESTABLISHED implies that socket is not on
-		 * vsock_connected_table. We keep the binding and the transport
-		 * assigned.
 		 */
 		if (signal_pending(current) || timeout == 0) {
 			err = timeout == 0 ? -ETIMEDOUT : sock_intr_errno(timeout);
@@ -1874,8 +1872,8 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 	}
 
-	err = sock_error(sk);
-	if (err) {
+	if (sk->sk_state != TCP_ESTABLISHED && sk->sk_state != TCP_CLOSING) {
+		err = sock_error(sk);
 		sk->sk_state = TCP_CLOSE;
 		sock->state = SS_UNCONNECTED;
 	}

-- 
2.55.0

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

[Michal Luczaj]

Virtio/PM events are serviced by virtio_vsock_reset_sock(), which resets
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 <mhal@rbox.co>
---
 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. */
+		if (READ_ONCE(vsk->peer_shutdown)) {
+			err = -ECONNRESET;
+			goto out;
+		}
+
 		/* Set the remote address that we are connecting to. */
 		memcpy(&vsk->remote_addr, remote_addr,
 		       sizeof(vsk->remote_addr));

-- 
2.55.0

Re: [PATCH net v2 1/5] vhost/vsock: Fix socket state constant

[Stefano Garzarella]

On Tue, Sep 15, 2026 at 03:15:12PM +0200, Michal Luczaj wrote:
>Refactor under Fixes missed one: SS_UNCONNECTED (1) -> TCP_CLOSE (7).
>Until now, sk->sk_state = 1 stood for TCP_ESTABLISHED.
>
>Fixes: 3b4477d2dcf2 ("VSOCK: use TCP state constants for sk_state")
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> drivers/vhost/vsock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Re: [PATCH net v2 2/5] vsock/virtio: Streamline socket reset on transport/PM event

[Stefano Garzarella]

On Tue, Sep 15, 2026 at 03:15:13PM +0200, Michal Luczaj wrote:
>Follow vhost's vhost_vsock_reset_orphans() and VMCI's
>vmci_transport_handle_detach(): set SHUTDOWN_MASK, which will come handy
>later in the series.

IMO it would be better to include the reason here as well. Every commit 
should explain why doing a change.

>
>Note that commit c38f57da428b ("vhost/vsock: fix reset orphans race with
>close timeout") fixed a race between pending close timer, i.e.
>virtio_transport_close_timeout(), and a transport/PM reset. But here we
>never set SOCK_DONE, hence no race.
>
>Fixes: 0ea9e1d3a9e3 ("VSOCK: Introduce virtio_transport.ko")
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> net/vmw_vsock/virtio_transport.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index 4f9aa9c4c3aa..4d6991321699 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -413,11 +413,14 @@ static void virtio_vsock_event_fill(struct virtio_vsock *vsock)
>
> static void virtio_vsock_reset_sock(struct sock *sk)
> {
>+	struct vsock_sock *vsk = vsock_sk(sk);
>+
> 	/* vmci_transport.c doesn't take sk_lock here either.  At least we're
> 	 * under vsock_table_lock so the sock cannot disappear while we're
> 	 * executing.
> 	 */
>
>+	vsk->peer_shutdown = SHUTDOWN_MASK;

In all other places we use WRITE_ONCE/READ_ONCE on vsk->peer_shutdown, 
should we do the same here?

Thanks,
Stefano

> 	sk->sk_state = TCP_CLOSE;
> 	sk->sk_err = ECONNRESET;
> 	sk_error_report(sk);
>
>-- 2.55.0
>

Re: [PATCH net v2 3/5] vsock: Enforce no-transport invariant for TCP_LISTEN sockets

[Stefano Garzarella]

On Tue, Sep 15, 2026 at 03:15:14PM +0200, Michal Luczaj wrote:
>A non-blocking connect() running in parallel with a blocking connect(),
>combined with a racy listen() that hits right after a connect timeout:
>TCP_SYN_SENT -> TCP_CLOSE -> TCP_LISTEN, while the connect() loop is still
>in progress.
>
>Enforce the invariant. Prevent a socket from becoming a listener after
>acquiring a transport.

We should improve this comment; it's not entirely clear to me, TBH.

>
>Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> net/vmw_vsock/af_vsock.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index f840498b58af..499e902becfa 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1973,13 +1973,13 @@ static int vsock_listen(struct socket *sock, int backlog)
> 		goto out;
> 	}
>
>-	if (sock->state != SS_UNCONNECTED) {
>+	vsk = vsock_sk(sk);
>+
>+	if (sock->state != SS_UNCONNECTED || vsk->transport) {

Are we changing the behavior when an error occurs?

If we call `connect()` on a socket (with no others running in parallel), 
it fails, and then when we call `listen()`, it now fails, whereas before 
it didn't. Can this happen? Is that what we want?

If so, we should mention it at least in the commit description; if not, 
perhaps we should unassign the transport in the `connect` call.

Thanks,
Stefano

> 		err = -EINVAL;
> 		goto out;
> 	}
>
>-	vsk = vsock_sk(sk);
>-
> 	if (!vsock_addr_bound(&vsk->local_addr)) {
> 		err = -EINVAL;
> 		goto out;
>
>-- 
>2.55.0
>

Re: [PATCH net v2 4/5] vsock: Do not reset a TCP_CLOSING socket

[Stefano Garzarella]

On Tue, Sep 15, 2026 at 03:15:15PM +0200, Michal Luczaj wrote:
>Handle the previously overlooked TCP_ESTABLISHED -> TCP_CLOSING
>transition (on VIRTIO_VSOCK_OP_RST), which could race with the connect
>loop.
>
>Resetting a socket that is still present in connected_table can lead to
>memory corruption. The reporter noted lost transports for in-flight skbs,
>and I have reproduced crashes caused by re-insertion into connected_table.
>
>  list_add double add: new=, prev=, next=.
>  kernel BUG at lib/list_debug.c:35!
>  Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
>  Workqueue: vsock-loopback vsock_loopback_work
>  RIP: 0010:__list_add_valid_or_report+0x11f/0x130
>  Call Trace:
>   vsock_insert_connected.cold+0xe/0x13
>   virtio_transport_recv_pkt+0x10e9/0x1460
>   vsock_loopback_work+0x305/0x480
>   process_one_work+0xe4c/0x1560
>   worker_thread+0x4f1/0xd60
>   kthread+0x36e/0x470
>   ret_from_fork+0x47b/0x6b0
>   ret_from_fork_asm+0x1a/0x30
>
>Drop the redundant err=0 and the inaccurate comment above signal_pending().
>This fix is supplementary to commit 002541ef650b ("vsock: Ignore
>signal/timeout on connect() if already established"). Details under Link.
>
>Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
>Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
>Link: https://lore.kernel.org/netdev/anzT1fREOSyHT99k@v4bel/
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> net/vmw_vsock/af_vsock.c | 20 +++++++++-----------
> 1 file changed, 9 insertions(+), 11 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 499e902becfa..adf3f018347e 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1712,7 +1712,6 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
> 	long timeout;
> 	DEFINE_WAIT(wait);
>
>-	err = 0;
> 	sk = sock->sk;
> 	vsk = vsock_sk(sk);
>
>@@ -1834,23 +1833,22 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
> 		timeout = schedule_timeout(timeout);
> 		lock_sock(sk);
>
>-		/* Connection established. Whatever happens to socket once we
>-		 * release it, that's not connect()'s concern. No need to go
>+		/* Connection was established. Whatever happens to socket once
>+		 * we release it, that's not connect()'s concern. No need to go
> 		 * into signal and timeout handling. Call it a day.
> 		 *
> 		 * Note that allowing to "reset" an already established socket
> 		 * here is racy and insecure.
> 		 */
>-		if (sk->sk_state == TCP_ESTABLISHED)
>-			break;
>+		if (sk->sk_state == TCP_ESTABLISHED ||
>+		    sk->sk_state == TCP_CLOSING) {
>+			err = 0;
>+			goto out_wait;
>+		}
>
> 		/* If connection was _not_ established and a signal/timeout came
> 		 * to be, we want the socket's state reset. User space may want
> 		 * to retry.
>-		 *
>-		 * sk_state != TCP_ESTABLISHED implies that socket is not on
>-		 * vsock_connected_table. We keep the binding and the transport
>-		 * assigned.

Should we keep "We keep the binding and the transport assigned." or 
maybe add on the other phrase, "User space may want to retry, so we keep 
..."

> 		 */
> 		if (signal_pending(current) || timeout == 0) {
> 			err = timeout == 0 ? -ETIMEDOUT : sock_intr_errno(timeout);
>@@ -1874,8 +1872,8 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
> 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
> 	}
>
>-	err = sock_error(sk);
>-	if (err) {

Should we add a comment here explaining why we are doing this?

Thanks,
Stefano

>+	if (sk->sk_state != TCP_ESTABLISHED && sk->sk_state != TCP_CLOSING) {
>+		err = sock_error(sk);
> 		sk->sk_state = TCP_CLOSE;
> 		sock->state = SS_UNCONNECTED;
> 	}
>
>-- 
>2.55.0
>

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

[Stefano Garzarella]

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 <mhal@rbox.co>
>---
> 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
>

Re: [PATCH net v2 2/5] vsock/virtio: Streamline socket reset on transport/PM event

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 2 · Low: 0

- [High] Lost-update race: the new lockless store `vsk->peer_shutdown =
  SHUTDOWN_MASK;` in virtio_vsock_reset_sock()…
- [Medium] Unannotated (plain) store to a locklessly-read field:
  virtio_vsock_reset_sock() (net/vmw_vsock/virtio_transport.c:423)…
- [Medium] Commit message does not describe the bug being fixed nor the
  user-visible behaviour change, while carrying a Fixes: tag.

--

vsock/virtio: Streamline socket reset on transport/PM event

This makes virtio_vsock_reset_sock() set vsk->peer_shutdown to
SHUTDOWN_MASK in addition to setting TCP_CLOSE and ECONNRESET, matching
what vhost_vsock_reset_orphans() and vmci_transport_handle_detach() do.
The changelog describes it as groundwork for later patches in the series.

A few questions below, one about the changelog and two about the new
store itself.

> vsock/virtio: Streamline socket reset on transport/PM event
>
> Follow vhost's vhost_vsock_reset_orphans() and VMCI's
> vmci_transport_handle_detach(): set SHUTDOWN_MASK, which will come handy
> later in the series.
>
> Fixes: 0ea9e1d3a9e3 ("VSOCK: Introduce virtio_transport.ko")

[Severity: Medium]
The changelog says the new state "will come handy later in the series",
but the patch carries a Fixes: tag against the original transport commit.
Could the changelog state the failure mode that is being fixed, so this
can be judged (and backported) on its own?

The change also looks user-visible by itself. After
VIRTIO_VSOCK_EVENT_TRANSPORT_RESET or device removal/PM freeze,
vsock_poll_shutdown() now contributes EPOLLRDHUP:

	if (sk->sk_shutdown & RCV_SHUTDOWN ||
	    peer_shutdown & SEND_SHUTDOWN)
		mask |= EPOLLRDHUP;

and vsock_poll() adds EPOLLIN|EPOLLRDNORM, while the blocking send/recv
loops now terminate on the peer_shutdown condition rather than only on
sk_err.

The errno also changes for send(). In vsock_connectible_sendmsg() the
peer shutdown test runs before the state test:

	if (sk->sk_shutdown & SEND_SHUTDOWN ||
	    vsk->peer_shutdown & RCV_SHUTDOWN) {
		err = -EPIPE;
		goto out;
	}

	if (!transport || sk->sk_state != TCP_ESTABLISHED ||

For SOCK_STREAM the error is then passed through sk_stream_error(), which
consumes the pending ECONNRESET on the first call and raises SIGPIPE on
subsequent calls unless MSG_NOSIGNAL is set:

net/core/stream.c:sk_stream_error() {
	if (err == -EPIPE)
		err = sock_error(sk) ? : -EPIPE;
	if (err == -EPIPE && !(flags & MSG_NOSIGNAL))
		send_sig(SIGPIPE, current, 0);
}

Before this patch the same send() returned -ENOTCONN and no signal. Is
that behaviour change intended, and could it be mentioned in the
changelog?

> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 4f9aa9c4c3aa5..4d6991321699e 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -413,11 +413,14 @@ static void virtio_vsock_event_fill(struct virtio_vsock *vsock)
>  
>  static void virtio_vsock_reset_sock(struct sock *sk)
>  {
> +	struct vsock_sock *vsk = vsock_sk(sk);
> +
>  	/* vmci_transport.c doesn't take sk_lock here either.  At least we're
>  	 * under vsock_table_lock so the sock cannot disappear while we're
>  	 * executing.
>  	 */
>  
> +	vsk->peer_shutdown = SHUTDOWN_MASK;

[Severity: Medium]
Should this store be a WRITE_ONCE()?  As the comment right above it says,
no socket lock is held here, only vsock_table_lock via
vsock_for_each_connected_socket().  peer_shutdown is sampled locklessly
by vsock_poll():

net/vmw_vsock/af_vsock.c:vsock_poll() {
		u32 peer_shutdown = READ_ONCE(vsk->peer_shutdown);

		/* DGRAM sockets do not take lock_sock() in poll(), so use one
		 * lockless snapshot for all shutdown-derived mask bits.
		 */
	...
		peer_shutdown = READ_ONCE(vsk->peer_shutdown);
		mask |= vsock_poll_shutdown(sk, peer_shutdown);
}

and the equivalent assignment in the same transport is annotated:

net/vmw_vsock/virtio_transport_common.c:virtio_transport_do_close() {
	WRITE_ONCE(vsk->peer_shutdown, SHUTDOWN_MASK);
}

The other writers in af_vsock.c, hyperv_transport.c and vmci_transport.c
use WRITE_ONCE() too, so this looks like an unmarked/marked pair that
KCSAN would flag.  The last patch of the series also reads this field
with READ_ONCE() from vsock_connect() with the comment "Virtio/PM events
are serviced locklessly.", which makes the pairing here relevant.

[Severity: High]
Separately from the annotation: can this store be lost against the
read-modify-write in the rx path?

virtio_transport_recv_connected() handles OP_SHUTDOWN under lock_sock():

net/vmw_vsock/virtio_transport_common.c:virtio_transport_recv_connected() {
	case VIRTIO_VSOCK_OP_SHUTDOWN: {
		u32 peer_shutdown = READ_ONCE(vsk->peer_shutdown);

		if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SHUTDOWN_RCV)
			peer_shutdown |= RCV_SHUTDOWN;
		if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SHUTDOWN_SEND)
			peer_shutdown |= SEND_SHUTDOWN;
		WRITE_ONCE(vsk->peer_shutdown, peer_shutdown);
}

while the reset path holds only vsock_table_lock:

net/vmw_vsock/af_vsock.c:vsock_for_each_connected_socket() {
	spin_lock_bh(&vsock_table_lock);
	...
			fn(sk_vsock(vsk));
}

So the two lock sets are disjoint:

CPU0 (rx_work, lock_sock held)
  virtio_transport_recv_connected()
    peer_shutdown = READ_ONCE(vsk->peer_shutdown);   /* reads 0 */

CPU1 (event_work or device removal)
  virtio_vsock_reset_sock()
    vsk->peer_shutdown = SHUTDOWN_MASK;

CPU0 continues
    WRITE_ONCE(vsk->peer_shutdown, peer_shutdown);   /* stale value wins */

Are these two contexts actually serialized anywhere?  virtio_vsock_vqs_del()
runs the reset loop before rx_run is cleared and rx_work drained:

net/vmw_vsock/virtio_transport.c:virtio_vsock_vqs_del() {
	/* Reset all connected sockets when the VQs disappear */
	vsock_for_each_connected_socket(&virtio_transport.transport,
					virtio_vsock_reset_sock);

	/* Stop all work handlers ... */
	mutex_lock(&vsock->rx_lock);
	vsock->rx_run = false;
}

and the VIRTIO_VSOCK_EVENT_TRANSPORT_RESET path runs from event_work,
which shares the non-ordered virtio_vsock_workqueue with rx_work.

Since hdr->flags is host supplied and is not validated as non-zero, the
value written back by the rx path can be the stale snapshot itself, i.e.
0, which would clear the reset state entirely.  In that case
vsock_poll_shutdown() reports no EPOLLRDHUP/EPOLLHUP, the sendmsg/recvmsg
peer_shutdown tests see a stale mask, and the guard added by the last
patch of the series:

net/vmw_vsock/af_vsock.c:vsock_connect() {
		/* Virtio/PM events are serviced locklessly. */
		if (READ_ONCE(vsk->peer_shutdown)) {
			err = -ECONNRESET;
}

silently passes.  A WRITE_ONCE() alone would not close this; would an
atomic update (atomic OR / cmpxchg), or performing the reset from a
context that can take lock_sock(), be more appropriate here?

>  	sk->sk_state = TCP_CLOSE;
>  	sk->sk_err = ECONNRESET;
>  	sk_error_report(sk);

Both of the above were still present in the same form at the end of the
series, so no later patch appears to change them.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915-vsock-connect-reset-closing-v2-0-a1d9abb472f7%40rbox.co

Re: [PATCH net v2 3/5] vsock: Enforce no-transport invariant for TCP_LISTEN sockets

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete race fix: `vsk->transport != NULL` is not a reliable
  sentinel for "a connect() waiter is still in flight", so the exact bug…

--

vsock: Enforce no-transport invariant for TCP_LISTEN sockets

The patch makes vsock_listen() reject sockets that already have a transport
assigned, so that a socket cannot become a listener while a connect() wait
loop is still running on it.

> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index f840498b58afb..499e902becfad 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -1973,13 +1973,13 @@ static int vsock_listen(struct socket *sock, int backlog)
>  		goto out;
>  	}
>  
> -	if (sock->state != SS_UNCONNECTED) {
> +	vsk = vsock_sk(sk);
> +
> +	if (sock->state != SS_UNCONNECTED || vsk->transport) {
>  		err = -EINVAL;
>  		goto out;
>  	}

[Severity: High]
Is vsk->transport a reliable indicator that no connect() waiter is in
flight?

vsock_assign_transport() drops the old transport before the new one is
validated and initialized:

net/vmw_vsock/af_vsock.c:vsock_assign_transport() {
	...
	if (vsk->transport) {
		vsk->transport->release(vsk);
		vsock_deassign_transport(vsk);	/* vsk->transport = NULL */
		...
		sk->sk_state = TCP_CLOSE;
		...
	}

	if (sk->sk_type == SOCK_SEQPACKET) {
		if (!new_transport->seqpacket_allow ||
		    !new_transport->seqpacket_allow(vsk, remote_cid)) {
			module_put(new_transport->module);
			return -ESOCKTNOSUPPORT;
		}
	}

	ret = new_transport->init(vsk, psk);
	if (ret) {
		module_put(new_transport->module);
		return ret;
	}

	vsk->transport = new_transport;
	...
}

Both of those failure returns happen after vsock_deassign_transport(), so
the socket is left with vsk->transport == NULL while it is still alive and
still bound (vsock_remove_sock() keeps the binding unless SOCK_DEAD is set).

The seqpacket path looks reachable without any allocation failure: only
virtio_transport.c and vsock_loopback.c define .seqpacket_allow, so a
SOCK_SEQPACKET reassignment that resolves to vmci returns -ESOCKTNOSUPPORT.
The init path is reachable via the kzalloc in
virtio_transport_do_socket_init().

Can the following interleaving still put a listener into TCP_CLOSE?

Thread A: blocking connect(), transport T1 assigned, TCP_SYN_SENT,
	  sock->state = SS_CONNECTING, enters the wait loop
Thread B: connect() on the same fd hits case SS_CONNECTING (-EALREADY)
	  and joins the same wait loop
Thread A: timeout fires and runs the signal/timeout branch of
	  vsock_connect(), leaving TCP_CLOSE / SS_UNCONNECTED and keeping
	  T1 ("We keep the binding and the transport assigned"), while
	  thread B still sleeps with the sock lock released
Thread C: connect() to a peer that resolves to a different transport, so
	  vsock_assign_transport() deassigns T1 and then fails at the
	  seqpacket_allow check or in init(); vsk->transport is now NULL
Thread D: listen() sees sock->state == SS_UNCONNECTED and
	  vsk->transport == NULL, passes the new gate and sets TCP_LISTEN
Thread B: wakes up; TCP_LISTEN is neither TCP_ESTABLISHED nor TCP_CLOSING
	  and sk_err == 0, so it keeps looping, and on the next
	  signal/timeout it runs:

net/vmw_vsock/af_vsock.c:vsock_connect() {
	...
		if (signal_pending(current) || timeout == 0) {
			err = timeout == 0 ? -ETIMEDOUT : sock_intr_errno(timeout);
			...
			sk->sk_state = TCP_CLOSE;
			sock->state = SS_UNCONNECTED;

			vsock_transport_cancel_pkt(vsk);

			goto out_wait;
		}
	...
}

That stale waiter rewrites sk_state and sock->state without re-checking
whether the socket became a listener in the meantime, so accept() on it
would then return -EINVAL and incoming requests would no longer be queued,
while entries from the listener phase remain on pending_links/accept_queue.

The window is also user-controllable through SO_VM_SOCKETS_CONNECT_TIMEOUT,
which sets vsk->connect_timeout read at loop entry.

Reading the rest of the series, up to b93debf817d4 ("vsock: Handle sudden
TCP_CLOSE during connect"), vsock_assign_transport() is unchanged,
vsock_listen() still gates only on sock->state != SS_UNCONNECTED ||
vsk->transport, and the waiter still rewrites sk_state/sock->state, so the
gap does not appear to be closed later either.

Would it make more sense to express the invariant as "no connect() waiter
outstanding", or to make the reassignment failure paths not leave a live
socket transport-less?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915-vsock-connect-reset-closing-v2-0-a1d9abb472f7%40rbox.co

Re: [PATCH net v2 4/5] vsock: Do not reset a TCP_CLOSING socket

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)

- [Medium] Undocumented change of connect()'s return value / sk_err
  consumption for an already-established socket.

Pre-existing issues:
- [Low] Reported gap (validated as resolved later in this series,
  therefore discarded): at this commit vsock_connect() keys its "do not…

--

vsock: Do not reset a TCP_CLOSING socket

This teaches the vsock_connect() wait loop about the TCP_ESTABLISHED ->
TCP_CLOSING transition triggered by VIRTIO_VSOCK_OP_RST, so a socket that
is still linked in vsock_connected_table is not reset behind the receive
path's back. It also rewrites the tail of vsock_connect() and drops a
stale comment.

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 499e902becfad..adf3f018347ea 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c

[ ... ]

> @@ -1834,23 +1833,22 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
>  		timeout = schedule_timeout(timeout);
>  		lock_sock(sk);
>  
> -		/* Connection established. Whatever happens to socket once we
> -		 * release it, that's not connect()'s concern. No need to go
> +		/* Connection was established. Whatever happens to socket once
> +		 * we release it, that's not connect()'s concern. No need to go
>  		 * into signal and timeout handling. Call it a day.
>  		 *
>  		 * Note that allowing to "reset" an already established socket
>  		 * here is racy and insecure.
>  		 */
> -		if (sk->sk_state == TCP_ESTABLISHED)
> -			break;
> +		if (sk->sk_state == TCP_ESTABLISHED ||
> +		    sk->sk_state == TCP_CLOSING) {
> +			err = 0;
> +			goto out_wait;
> +		}

[Severity: Medium]

The old code did a plain break here, so control fell through to the
err = sock_error(sk) tail. Now it jumps straight to out_wait with err = 0.

Combined with the tail change below, does this mean a blocking connect()
that wakes up in TCP_ESTABLISHED with sk_err already set now returns 0
instead of that error, leaving sk_err queued for the next send()/recv()?

That window looks reachable via virtio_transport_recv_connected(), which
runs only for TCP_ESTABLISHED sockets:

net/vmw_vsock/virtio_transport_common.c:virtio_transport_recv_connected() {
	case VIRTIO_VSOCK_OP_RW:
		if (!virtio_transport_recv_enqueue(vsk, skb)) {
			...
			(void)virtio_transport_reset(vsk, skb);
			virtio_transport_do_close(vsk, true);
			sk->sk_err = ENOBUFS;
			...
}

and virtio_transport_do_close() only downgrades the state when the receive
queue is empty:

net/vmw_vsock/virtio_transport_common.c:virtio_transport_do_close() {
	WRITE_ONCE(vsk->peer_shutdown, SHUTDOWN_MASK);
	if (vsock_stream_has_data(vsk) <= 0)
		sk->sk_state = TCP_CLOSING;
	...
}

In the queue-full case vsock_stream_has_data(vsk) > 0, so sk_state stays
TCP_ESTABLISHED while sk_err == ENOBUFS.

The changelog mentions only the TCP_CLOSING transition plus "Drop the
redundant err=0 and the inaccurate comment above signal_pending()". Could
the changelog also spell out that connect() no longer reports or consumes a
pending sk_err for an established socket? The patch carries a Fixes:
d021c344051a tag, so it is a backport candidate and the user-visible change
of the connect() return value seems worth documenting.

>  
>  		/* If connection was _not_ established and a signal/timeout came
>  		 * to be, we want the socket's state reset. User space may want
>  		 * to retry.
> -		 *
> -		 * sk_state != TCP_ESTABLISHED implies that socket is not on
> -		 * vsock_connected_table. We keep the binding and the transport
> -		 * assigned.
>  		 */
>  		if (signal_pending(current) || timeout == 0) {
>  			err = timeout == 0 ? -ETIMEDOUT : sock_intr_errno(timeout);

[ ... ]

> @@ -1874,8 +1872,8 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
>  		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
>  	}
>  
> -	err = sock_error(sk);
> -	if (err) {
> +	if (sk->sk_state != TCP_ESTABLISHED && sk->sk_state != TCP_CLOSING) {
> +		err = sock_error(sk);
>  		sk->sk_state = TCP_CLOSE;
>  		sock->state = SS_UNCONNECTED;
>  	}

[Severity: Low]

This is a pre-existing issue and not introduced by this patch, but the new
whitelist covers only TCP_ESTABLISHED and TCP_CLOSING. A socket that is
still linked in vsock_connected_table but was moved to TCP_CLOSE by a
lockless event handler still gets reset to SS_UNCONNECTED both here and in
the signal_pending() branch above.

Two writers do that without calling vsock_remove_connected():

net/vmw_vsock/virtio_transport.c:virtio_vsock_reset_sock() {
	vsk->peer_shutdown = SHUTDOWN_MASK;
	sk->sk_state = TCP_CLOSE;
	sk->sk_err = ECONNRESET;
	sk_error_report(sk);
}

net/vmw_vsock/vmci_transport.c:vmci_transport_handle_detach() {
	WRITE_ONCE(vsk->peer_shutdown, SHUTDOWN_MASK);
	...
		sk->sk_state = TCP_CLOSE;
}

With sock->state == SS_UNCONNECTED, a retry connect() takes the default:
branch, vsock_assign_transport() returns early because the transport is
unchanged, transport->connect() is re-issued, and the next RESPONSE reaches
__vsock_insert_connected():

net/vmw_vsock/af_vsock.c:__vsock_insert_connected() {
	sock_hold(&vsk->sk);
	list_add(&vsk->connected_table, list);
}

which is the same list_add double add BUG quoted in the changelog, plus a
leaked struct sock reference.

For the record, this appears to be closed by the next patch in the series,
"vsock: Handle sudden TCP_CLOSE during connect", which adds

	/* Virtio/PM events are serviced locklessly. */
	if (READ_ONCE(vsk->peer_shutdown)) {
		err = -ECONNRESET;
		goto out;
	}

to the same default: branch, and every writer above sets
peer_shutdown = SHUTDOWN_MASK before moving the socket to TCP_CLOSE, so no
action seems needed on the series as a whole. Would it be worth noting in
this patch's changelog that the TCP_CLOSE variant of the same race is
handled by the following patch?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915-vsock-connect-reset-closing-v2-0-a1d9abb472f7%40rbox.co

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] The new consumer in vsock_connect() reads the poison flag with
  READ_ONCE(vsk->peer_shutdown) (net/vmw_vsock/af_vsock.c:1747), but the…
- [Low] Undeclared intra-series dependency: the new guard in
  vsock_connect() (net/vmw_vsock/af_vsock.c:1747) only *reads*…

--

vsock: Handle sudden TCP_CLOSE during connect

This adds a check in vsock_connect() that refuses a fresh connect attempt
with -ECONNRESET when vsk->peer_shutdown is already set, so that a socket
poisoned by a lockless virtio/PM/vhost/VMCI reset event cannot be handed a
new transport or pushed back into TCP_SYN_SENT.

> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index adf3f018347ea..972952d04a81a 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. */
> +		if (READ_ONCE(vsk->peer_shutdown)) {
> +			err = -ECONNRESET;
> +			goto out;
> +		}
> +

[Severity: Low]
The comment mentions virtio/PM events, but this hunk only reads
vsk->peer_shutdown.  For the virtio/PM path the flag is published by
virtio_vsock_reset_sock() in net/vmw_vsock/virtio_transport.c:

	vsk->peer_shutdown = SHUTDOWN_MASK;
	sk->sk_state = TCP_CLOSE;
	sk->sk_err = ECONNRESET;

That store comes from the immediately preceding patch in this series
("vsock/virtio: Streamline socket reset on transport/PM event"); at the
parent commit virtio_vsock_reset_sock() only sets sk_state and sk_err.

Given the Fixes: d021c344051a tag reaching back to 2013, would it be worth
spelling out the dependency so that a stable/AUTOSEL pick does not take this
commit alone?  On its own the new check would be a no-op for exactly the
virtio/PM scenario the changelog leads with, and only vhost's
vhost_vsock_reset_orphans() and VMCI's vmci_transport_handle_detach() (which
already set the flag) would be covered.

[Severity: Medium]
Should the writers of this flag be marked to match the new READ_ONCE()
reader?  The two handlers named in the changelog store it plainly:

net/vmw_vsock/virtio_transport.c:virtio_vsock_reset_sock() {
	vsk->peer_shutdown = SHUTDOWN_MASK;
	sk->sk_state = TCP_CLOSE;
	sk->sk_err = ECONNRESET;
	sk_error_report(sk);
}

drivers/vhost/vsock.c:vhost_vsock_reset_orphans() {
	sock_set_flag(sk, SOCK_DONE);
	vsk->peer_shutdown = SHUTDOWN_MASK;
	sk->sk_state = TCP_CLOSE;
	sk->sk_err = ECONNRESET;
}

Both run from vsock_for_each_connected_socket() under
spin_lock_bh(&vsock_table_lock) only, never under lock_sock(), so they share
no lock with vsock_connect().  Every other lockless writer of the field uses
WRITE_ONCE(), for example vmci_transport_handle_detach(),
virtio_transport_recv_pkt() in virtio_transport_common.c and
hvs_close_connection() in hyperv_transport.c.  Is the plain store racing the
new marked read something KCSAN would flag here?

There is also no barrier pairing the poison store with the sk_state/sk_err
stores that connect() actually leaves its wait loop on.  Can the following
still happen on a weakly ordered machine?

	connect() wait loop exits on timeout/signal:
		sk->sk_state = TCP_CLOSE;
		sock->state = SS_UNCONNECTED;
		vsock_transport_cancel_pkt(vsk);

	userspace retries connect(), reads peer_shutdown == 0,
	falls through to vsock_assign_transport() and
		sk->sk_state = TCP_SYN_SENT;

while the socket is still hashed in connected_table.  A later OP_RESPONSE
then reaches virtio_transport_recv_connecting(), which calls
vsock_insert_connected() unconditionally, and __vsock_insert_connected() does
a bare list_add() on an already-linked node.  That is the list_add double add
splat described in the sibling commit 6f06a44e.

Would a WRITE_ONCE() for both stores, plus explicit ordering between the
peer_shutdown store and the sk_state/sk_err stores, close that window?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915-vsock-connect-reset-closing-v2-0-a1d9abb472f7%40rbox.co