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]>
