Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH net] vhost/vsock: Fix socket state constant
@ 2026-11-12 10:34 Michal Luczaj
  2026-11-12 10:36 ` netdev-bot+sinfo
  2026-11-13 10:34 ` sashiko-bot
  0 siblings, 2 replies; 5+ messages in thread
From: Michal Luczaj @ 2026-11-12 10:34 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Bobby Eshleman,
	Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
	David S. Miller
  Cc: kvm, virtualization, netdev, linux-kernel, 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")
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
Salvaging a single already-reviewed patch from a series[1] that was
superseded.

[1]: https://lore.kernel.org/all/20260922-vsock-connect-reset-closing-v3-0-78907b8200d4@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 88a310390eb0..fd224f7681d9 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -789,7 +789,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);
 }

---
base-commit: 145480b28ca82e9fae789427cc4569e596515a32
change-id: 20261112-transport-reset-state-fix-42363b46d2a7

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net] vhost/vsock: Fix socket state constant
  2026-11-12 10:34 [PATCH net] vhost/vsock: Fix socket state constant Michal Luczaj
@ 2026-11-12 10:36 ` netdev-bot+sinfo
  2026-11-12 10:52   ` Michal Luczaj
  2026-11-13 10:34 ` sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: netdev-bot+sinfo @ 2026-11-12 10:36 UTC (permalink / raw)
  To: Michal Luczaj
  Cc: Stefan Hajnoczi, Stefano Garzarella, Bobby Eshleman,
	Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
	David S. Miller, kvm, virtualization, netdev, linux-kernel

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by LLM and not proven at runtime). If it was triggered
   please include the symptoms, like the stack trace or kernel messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] vhost/vsock: Fix socket state constant
  2026-11-12 10:36 ` netdev-bot+sinfo
@ 2026-11-12 10:52   ` Michal Luczaj
  2026-11-13  8:44     ` Michal Luczaj
  0 siblings, 1 reply; 5+ messages in thread
From: Michal Luczaj @ 2026-11-12 10:52 UTC (permalink / raw)
  To: netdev-bot+sinfo
  Cc: Stefan Hajnoczi, Stefano Garzarella, Bobby Eshleman,
	Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
	David S. Miller, kvm, virtualization, netdev, linux-kernel

On 11/12/26 11:36, netdev-bot+sinfo@kernel.org wrote:

>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.

Discovered by reading the code and not getting it.

>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by LLM and not proven at runtime). If it was triggered
>    please include the symptoms, like the stack trace or kernel messages.

Not triggered at runtime. It's a correctness fix, I didn't try to work out
what could/would actually go wrong.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] vhost/vsock: Fix socket state constant
  2026-11-12 10:52   ` Michal Luczaj
@ 2026-11-13  8:44     ` Michal Luczaj
  0 siblings, 0 replies; 5+ messages in thread
From: Michal Luczaj @ 2026-11-13  8:44 UTC (permalink / raw)
  To: netdev-bot+sinfo
  Cc: Stefan Hajnoczi, Stefano Garzarella, Bobby Eshleman,
	Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
	David S. Miller, kvm, virtualization, netdev, linux-kernel

>>  - Whether the issue was actually triggered, or is only theoretical
>>    (e.g. found by LLM and not proven at runtime). If it was triggered
>>    please include the symptoms, like the stack trace or kernel messages.
> 
> Not triggered at runtime. It's a correctness fix, I didn't try to work out
> what could/would actually go wrong.

I don't see any way this could really affect the kernel. Will re-post to
net-next along with some other cleanups. Sorry for the noise.

pw-bot: cr

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] vhost/vsock: Fix socket state constant
  2026-11-12 10:34 [PATCH net] vhost/vsock: Fix socket state constant Michal Luczaj
  2026-11-12 10:36 ` netdev-bot+sinfo
@ 2026-11-13 10:34 ` sashiko-bot
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-13 10:34 UTC (permalink / raw)
  To: Michal Luczaj; +Cc: kvm, virtualization, Michael S. Tsirkin, Eugenio Perez

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] vhost/vsock: vhost_vsock_reset_orphans() TCP_CLOSE breaks
  vsock_connect()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] vhost/vsock: data race in vhost_vsock_reset_orphans() [1]
--

commit 5aff0946f70c88424d509989526e2bc91c501e
Author: Michal Luczaj <mhal@rbox.co>

vhost/vsock: Fix socket state constant

This commit updates vhost_vsock_reset_orphans() to set sk->sk_state to
TCP_CLOSE instead of SS_UNCONNECTED when resetting orphaned connections.

> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 88a310390eb0a..fd224f7681d92 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -789,7 +789,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);
>  }

[Severity: High]
Can setting sk->sk_state to TCP_CLOSE here cause vsock_connect() to reset
sock->state to SS_UNCONNECTED while vsk remains in vsock_connected_table?

 Previously, SS_UNCONNECTED (1) happened to match TCP_ESTABLISHED (1), so
vsock_connect() treated the socket as once-established.  With TCP_CLOSE, a
vhost device release racing with a completing connection can hit the error
path in vsock_connect():

CPU0                              CPU1
vsock_connect()
  release_sock(sk)
  schedule_timeout()
                                  virtio_transport_recv_connecting()
                                    sk->sk_state = TCP_ESTABLISHED
                                    sock->state = SS_CONNECTED
                                    vsock_insert_connected(vsk)
                                  vhost_vsock_dev_release()
                                    ...
                                      vhost_vsock_reset_orphans(sk)
                                        sk->sk_state = TCP_CLOSE
                                        sk->sk_err = ECONNRESET
  lock_sock(sk)
  if (sk->sk_state == TCP_ESTABLISHED ||
      sk->sk_state == TCP_CLOSING)        <-- false
  ...
  err = sock_error(sk)
  if (err)
    sock->state = SS_UNCONNECTED          <-- still in vsock_connected_table

net/vmw_vsock/af_vsock.c:vsock_connect() {
    ...
		if (sk->sk_state == TCP_ESTABLISHED ||
		    sk->sk_state == TCP_CLOSING) {
			err = 0;
			goto out_wait;
		}
    ...
	err = sock_error(sk);
	if (err) {
		sk->sk_state = TCP_CLOSE;
		sock->state = SS_UNCONNECTED;
	}
}

Because sock->state is reset to SS_UNCONNECTED while vsk is still linked in
vsock_connected_table, a subsequent connect() on the same socket can
reassign its transport or, if shutdown() clears SOCK_DONE and a new
VIRTIO_VSOCK_OP_RESPONSE arrives, call vsock_insert_connected(vsk) a second
time and corrupt vsock_connected_table.  Setting sk->sk_state to
TCP_CLOSING instead (or checking sock_flag(sk, SOCK_DONE) in
vsock_connect()) would preserve the once-established check.

[Severity: High]
This problem wasn't introduced by this patch, but vhost_vsock_reset_orphans()
modifies sk->sk_flags, vsk->peer_shutdown, sk->sk_state, and sk->sk_err
under vsock_table_lock without holding lock_sock(sk) [1].  Concurrent
socket operations such as vsock_connect(), sendmsg(), or recvmsg() holding
the socket lock can race with these updates and observe inconsistent socket
state.

[1] https://sashiko.dev/#/bug/linux-f1ccca0e-e7ae-4926-a24d-72f61680c524

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-transport-reset-state-fix-v1-1-5698375f52a9@rbox.co?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-13 10:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-12 10:34 [PATCH net] vhost/vsock: Fix socket state constant Michal Luczaj
2026-11-12 10:36 ` netdev-bot+sinfo
2026-11-12 10:52   ` Michal Luczaj
2026-11-13  8:44     ` Michal Luczaj
2026-11-13 10:34 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox