Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] vsock: lost peer_shutdown update in lockless reset --
commit c52d0f36e0857217fee2a98454aaab9edf069cc3 Author: Michal Luczaj <[email protected]> vsock/virtio: Streamline socket reset on transport/PM event This commit streamlines socket resets on transport or power management events by setting SHUTDOWN_MASK in virtio_vsock_reset_sock(). It aligns the virtio transport with similar behavior in vhost and VMCI. > diff --git a/net/vmw_vsock/virtio_transport.c > b/net/vmw_vsock/virtio_transport.c > --- a/net/vmw_vsock/virtio_transport.c > +++ b/net/vmw_vsock/virtio_transport.c [ ... ] > 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] Could this lockless assignment race with the lock-protected read-modify-write in the RX path? Looking at virtio_transport_recv_connected(), it processes VIRTIO_VSOCK_OP_SHUTDOWN while holding sk_lock. It reads vsk->peer_shutdown via READ_ONCE(), applies a bitwise OR (such as peer_shutdown |= RCV_SHUTDOWN), and writes it back with WRITE_ONCE(). If the lockless assignment here occurs between the RX thread's read and write, could the RX thread overwrite SHUTDOWN_MASK with its local value, dropping the SEND_SHUTDOWN state? If this lost update occurs, it seems the socket would lose its full shutdown state. Would this cause vsock_poll() to miss setting EPOLLHUP and EPOLLRDHUP flags, breaking API expectations? Additionally, might this plain write trigger KCSAN data race warnings against concurrent READ_ONCE() calls? > sk->sk_state = TCP_CLOSE; > sk->sk_err = ECONNRESET; > sk_error_report(sk); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
