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
>