Re: [PATCH net v2] vsock: keep poll shutdown state consistent

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Concurrent non-atomic read-modify-write on vsk->peer_shutdown without synchronization leads to lost shutdown flags, potentially hanging host processes.
--

commit d7feaefbab86483ded481345d096a59fb79a2b29
Author: Ziyu Zhang <ziyuzhang201@gmail.com>

vsock: keep poll shutdown state consistent

This patch updates vsock polling to use a consistent snapshot of the
peer_shutdown state to avoid race conditions between checking for
HUP/RDHUP and EOF readability. It introduces lockless READ_ONCE and
WRITE_ONCE accessors for peer_shutdown in datagram and stream sockets.

> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
[ ... ]
> @@ -1542,7 +1542,9 @@ static int vmci_transport_recv_connected(struct sock *sk,
>  		if (pkt->u.mode) {
>  			vsk = vsock_sk(sk);
>  
> -			vsk->peer_shutdown |= pkt->u.mode;
> +			WRITE_ONCE(vsk->peer_shutdown,
> +				   READ_ONCE(vsk->peer_shutdown) |
> +				   pkt->u.mode);

[Severity: High]
This is a pre-existing issue, but does this non-atomic read-modify-write
sequence risk losing concurrent updates to the shutdown mask?

While vmci_transport_recv_connected() executes under lock_sock(),
vmci_transport_handle_detach() can be called from softirq context via
the queue pair detach event.

The detach event uses bh_lock_sock() without checking sock_owned_by_user()
to defer execution to the socket backlog. Because lock_sock() releases
the inner spinlock while setting sk_lock.owned=1, bh_lock_sock() can
successfully acquire the lock and execute concurrently.

If the softirq executes exactly between the READ_ONCE() and WRITE_ONCE()
calls here:

CPU 1 (vmci_transport_recv_connected):
    read = READ_ONCE(vsk->peer_shutdown);

CPU 2 (softirq detach callback):
    vmci_transport_handle_detach()
        WRITE_ONCE(vsk->peer_shutdown, SHUTDOWN_MASK);

CPU 1 (vmci_transport_recv_connected):
    WRITE_ONCE(vsk->peer_shutdown, read | pkt->u.mode);

Could CPU 1 overwrite the SHUTDOWN_MASK written by CPU 2? If the
SHUTDOWN_MASK is lost, it appears host processes blocking in poll()
could be left sleeping indefinitely because the connection closure flags
won't be fully recognized.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519165636.62542-1-ziyuzhang201@gmail.com?part=1