Weiming Shi <[email protected]> 于2026年7月30日周四 03:05写道： > > 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
Sorry for the noise; I resent the series as v3 with proper threading and no code changes. https://lore.kernel.org/all/[email protected]/
