Re: [PATCH] vsock/virtio: prevent workers from using deleted virtqueues
Stefano Garzarella <[email protected]> Wed, 29 Jul 2026 16:34:55 +0200
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.kvm |
|---|---|
| Message-ID | <amoNWGhKa78r3Kzo@sgarzare-redhat> |
On Tue, Jul 28, 2026 at 03:58:23AM +0000, [email protected] wrote: >Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > >Pre-existing issues: >- [High] Incorrect flush_work() ordering in virtio_vsock_remove() allows rx_work to be queued by a delayed send_pkt_work after rx_work has already been flushed, leading to a use-after-free when vsock is destroyed. >-- > >--- Patch [1]: [PATCH] vsock/virtio: prevent workers from using deleted virtqueues --- >commit fac9b600b273994b7593c604a6d3d0352d8dbee3 >Author: Weiming Shi <[email protected]> > >vsock/virtio: prevent workers from using deleted virtqueues > >The RX, TX and event workers read their virtqueue pointers before taking >the mutex that protects the queue and its run flag. This patch reads each >worker's virtqueue under its mutex after confirming the queue is running, >preventing use-after-free of deleted virtqueues. > >Cc: [email protected] > >> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c >> index 57f2d6ec3ffc6..79cf19f589439 100644 >> --- a/net/vmw_vsock/virtio_transport.c >> +++ b/net/vmw_vsock/virtio_transport.c >[ ... ] >> @@ -689,7 +690,8 @@ static void virtio_transport_rx_work(struct work_struct *work) >> } while (!virtqueue_enable_cb(vq)); >> >> out: >> - if (vsock->rx_buf_nr < vsock->rx_buf_max_nr / 2) >> + if (vsock->rx_run && >> + vsock->rx_buf_nr < vsock->rx_buf_max_nr / 2) >> virtio_vsock_rx_fill(vsock); >> mutex_unlock(&vsock->rx_lock); >> } > >[Severity: High] >This is a pre-existing issue, but is there a use-after-free risk in >virtio_vsock_remove() due to the flush_work() ordering? > >In virtio_vsock_remove(), the flush sequence drains rx_work before >send_pkt_work: > > flush_work(&vsock->rx_work); > flush_work(&vsock->tx_work); > flush_work(&vsock->event_work); > flush_work(&vsock->send_pkt_work); > ... > kfree(vsock); > >Meanwhile, virtio_transport_send_pkt_work() can queue rx_work outside >the tx_lock: > >out: > mutex_unlock(&vsock->tx_lock); > > if (restart_rx) > queue_work(virtio_vsock_workqueue, &vsock->rx_work); > >If virtio_vsock_remove() flushes rx_work (which returns immediately if it >has not been queued yet), and then blocks waiting on send_pkt_work to finish, >could send_pkt_work queue rx_work just before completing? > >This would leave rx_work pending in the workqueue after virtio_vsock_remove() >frees the vsock object, leading to memory corruption when the work executes. This seems a real issue, would you like to fix it? In a separate patch, since it's pre-existing. Thanks, Stefano > >-- >Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1 >