Stefano Garzarella <[email protected]> 于2026年7月29日周三 22:47写道： > > On Tue, Jul 28, 2026 at 10:17:55AM -0700, Bobby Eshleman wrote: > >On Sun, Jul 26, 2026 at 08:58:01PM -0700, Weiming Shi wrote: > >> The RX, TX and event workers read their virtqueue pointers before taking > >> the mutex that protects the queue and its run flag. A work item delayed > >> across freeze and restore can therefore retain a pointer deleted by > >> virtio_vsock_vqs_del(), observe the run flag for the replacement queues, > >> and use the freed pointer. > >> > >> RX has an additional path: when rx_run is clear, the common exit still > >> refills the RX queue. A queued worker can consequently call > >> virtqueue_add_sgs() immediately after freeze deletes the virtqueues. > >> > >> BUG: KASAN: slab-use-after-free in virtqueue_add_sgs > >> Read of size 4 by task kworker/2:1 > >> Workqueue: virtio_vsock virtio_transport_rx_work > >> Call Trace: > >> virtqueue_add_sgs (drivers/virtio/virtio_ring.c:2796) > >> virtio_vsock_rx_fill (net/vmw_vsock/virtio_transport.c:332) > >> virtio_transport_rx_work (net/vmw_vsock/virtio_transport.c:701) > >> process_one_work (kernel/workqueue.c:3314) > >> worker_thread (kernel/workqueue.c:3478) > >> kthread (kernel/kthread.c:436) > >> ret_from_fork (arch/x86/kernel/process.c:158) > >> ret_from_fork_asm (arch/x86/entry/entry_64.S:245) > >> ... > >> Freed by task 141: > >> kfree (mm/slub.c:6566) > >> vp_del_vq (drivers/virtio/virtio_pci_common.c:259) > >> vp_del_vqs (drivers/virtio/virtio_pci_common.c:285) > >> virtio_vsock_freeze (net/vmw_vsock/virtio_transport.c:912) > >> virtio_device_freeze (drivers/virtio/virtio.c:658) > >> virtio_pci_freeze (drivers/virtio/virtio_pci_common.c:601) > >> pci_pm_freeze (drivers/pci/pci-driver.c:1098) > >> device_suspend (drivers/base/power/main.c:1968) > >> Kernel panic - not syncing: KASAN: panic_on_warn set ... > >> > >> Read each worker's virtqueue under its mutex after confirming that the > >> queue is running, and only refill RX while RX is running. > >> > >> Fixes: b917507e5ad9 ("vsock/virtio: stop workers during the .remove()") > >> Cc: [email protected] > >> Reported-by: Xiang Mei <[email protected]> > >> Assisted-by: OpenAI-Codex:gpt-5 > >> Signed-off-by: Weiming Shi <[email protected]> > >> --- > >> net/vmw_vsock/virtio_transport.c | 14 ++++++++------ > >> 1 file changed, 8 insertions(+), 6 deletions(-) > >> > >> diff --git a/net/vmw_vsock/virtio_transport.c > >> b/net/vmw_vsock/virtio_transport.c > >> index 57f2d6ec3ffc..79cf19f58943 100644 > >> --- a/net/vmw_vsock/virtio_transport.c > >> +++ b/net/vmw_vsock/virtio_transport.c > >> @@ -346,12 +346,13 @@ static void virtio_transport_tx_work(struct > >> work_struct *work) > >> struct virtqueue *vq; > >> bool added = false; > >> > >> - vq = vsock->vqs[VSOCK_VQ_TX]; > >> mutex_lock(&vsock->tx_lock); > >> > >> if (!vsock->tx_run) > >> goto out; > >> > >> + vq = vsock->vqs[VSOCK_VQ_TX]; > >> + > >> do { > >> struct sk_buff *skb; > >> unsigned int len; > >> @@ -451,13 +452,13 @@ static void virtio_transport_event_work(struct > >> work_struct *work) > >> container_of(work, struct virtio_vsock, event_work); > >> struct virtqueue *vq; > >> > >> - vq = vsock->vqs[VSOCK_VQ_EVENT]; > >> - > >> mutex_lock(&vsock->event_lock); > >> > >> if (!vsock->event_run) > >> goto out; > >> > >> + vq = vsock->vqs[VSOCK_VQ_EVENT]; > >> + > >> do { > >> struct virtio_vsock_event *event; > >> unsigned int len; > >> @@ -634,13 +635,13 @@ static void virtio_transport_rx_work(struct > >> work_struct *work) > >> container_of(work, struct virtio_vsock, rx_work); > >> struct virtqueue *vq; > >> > >> - vq = vsock->vqs[VSOCK_VQ_RX]; > >> - > >> mutex_lock(&vsock->rx_lock); > >> > >> if (!vsock->rx_run) > >> goto out; > >> > >> + vq = vsock->vqs[VSOCK_VQ_RX]; > >> + > >> do { > >> virtqueue_disable_cb(vq); > >> for (;;) { > >> @@ -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); > >> } > >> -- > >> 2.55.0 > > > >Not a strong opinion from me, but since this last hunk is the one that > >fixes the bug and the other hunks are moreso hardening, maybe break > >these out into two patches? > > I think also the other hunks fix an issue, but I agree on the split > since IMO we are fixing 2 different commits. Commit b917507e5ad9 > ("vsock/virtio: stop workers during the .remove()") was before > freeze/resume added by commit bd50c5dc182b ("vsock/virtio: add support > for device suspend/resume"). Only after that one we can have the false > -> true transition of *_run variables. > > So IMO hunks 1-3 should have Fixes: bd50c5dc182b ... and hunk 4 should > have Fixes: b917507e5ad9 ... > > That said, it's not a strong opinion here too, but if you prefer a > single patch, please add both Fixes. > > Thanks, > Stefano > > > > >Besides, that all looks good to me. > > > >Reviewed-by: Bobby Eshleman <[email protected]> > > >
Thanks for your review. v2 sent. Best, Weiming Shi
