[PATCH v2] drm/virtio: reclaim pending vbufs before tearing down vqs

[PATCH v2] drm/virtio: reclaim pending vbufs before tearing down vqs

[Anuj Bolewar via B4 Relay]

From: Anuj Bolewar <bolewara@gmail.com>

virtio_gpu_free_vbufs() destroys the vbufs kmem_cache after the virtqueues
have already been released. Commands that were queued but never completed
by the device leave their vbuffers stranded in the virtqueue, so the cache
still holds live objects when virtio_gpu_deinit() tears everything down.
This triggers a WARNING in virtio_gpu_free_vbufs:

    BUG virtio-gpu-vbufs (Not tainted): Objects remaining in cache
    on __kmem_cache_shutdown()

Drain any buffers still sitting in the control and cursor virtqueues in
virtio_gpu_deinit() after the device has been reset and before the
virtqueues are deleted, following the same pattern used by virtio_console's
remove_vqs(). Each reclaimed buffer is released with free_vbuf(), dropping
the reference on any GEM objects it holds. Pending RESOURCE_UNREF
commands are handled as well: their resp_cb_data still references a GEM
object, so it is cleaned up with virtio_gpu_cleanup_object() to avoid
leaking it on teardown.

Reported-by: syzbot+06f9b2a53ba4a5a47644@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=06f9b2a53ba4a5a47644
Signed-off-by: Anuj Bolewar <bolewara@gmail.com>
---
This series fixes a syzbot-triggered WARNING in virtio_gpu_free_vbufs
(cache object: virtio-gpu-vbufs) seen on device removal. Commands that
are queued but never complete leave vbuffers stranded in the control and
cursor virtqueues. virtio_gpu_deinit() reset the device and deleted the
virtqueues without draining them, so a later kmem_cache_destroy() in
virtio_gpu_release() ran with live objects still allocated.

Patch 1 drains the queues in virtio_gpu_deinit(): after the device reset
and before del_vqs(), virtio_gpu_reclaim_vbufs() detaches every unused
buffer from both virtqueues, releases their object arrays, and runs the
pending resource-unref cleanup so the referenced GEM objects are freed
rather than leaked. This mirrors the drain pattern used by
virtio_console's remove_vqs().

Link: https://syzkaller.appspot.com/bug?extid=06f9b2a53ba4a5a47644
---
Changes in v2:
- Also release the GEM object referenced by vbuf->resp_cb_data when
  reclaiming stranded buffers, so pending RESOURCE_UNREF commands do not
  leak their underlying objects on teardown.
- Link to v1: https://patch.msgid.link/20260802-virtio-gpu-reclaim-vbufs-v1-1-9947f18b20e2@gmail.com
---
 drivers/gpu/drm/virtio/virtgpu_drv.h |  1 +
 drivers/gpu/drm/virtio/virtgpu_kms.c |  1 +
 drivers/gpu/drm/virtio/virtgpu_vq.c  | 15 +++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 7449907754a..3e491c80873 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -332,6 +332,7 @@ void virtio_gpu_array_put_free_work(struct work_struct *work);
 /* virtgpu_vq.c */
 int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
 void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev);
+void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev);
 void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev,
 				    struct virtio_gpu_object *bo,
 				    struct virtio_gpu_object_params *params,
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index b4329f28e97..e5a6ae679f3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -298,6 +298,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
 	flush_work(&vgdev->cursorq.dequeue_work);
 	flush_work(&vgdev->config_changed_work);
 	virtio_reset_device(vgdev->vdev);
+	virtio_gpu_reclaim_vbufs(vgdev);
 	vgdev->vdev->config->del_vqs(vgdev->vdev);
 }
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index e5e1af8b8e8..ab6106f4bdf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -208,6 +208,21 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
 	kmem_cache_free(vgdev->vbufs, vbuf);
 }
 
+void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev)
+{
+	struct virtio_gpu_vbuffer *vbuf;
+
+	while ((vbuf = virtqueue_detach_unused_buf(vgdev->ctrlq.vq))) {
+		if (vbuf->objs)
+			virtio_gpu_array_put_free(vbuf->objs);
+		if (vbuf->resp_cb_data)
+			virtio_gpu_cleanup_object(vbuf->resp_cb_data);
+		free_vbuf(vgdev, vbuf);
+	}
+	while ((vbuf = virtqueue_detach_unused_buf(vgdev->cursorq.vq)))
+		free_vbuf(vgdev, vbuf);
+}
+
 static void reclaim_vbufs(struct virtqueue *vq, struct list_head *reclaim_list)
 {
 	struct virtio_gpu_vbuffer *vbuf;

---
base-commit: 2d2338c93da79b3bfe4b6099a931d9468d539952
change-id: 20260802-virtio-gpu-reclaim-vbufs-4816cc8a5bf9

Best regards,
--  
Anuj Bolewar <bolewara@gmail.com>

[PATCH v2] drm/virtio: reclaim pending vbufs before tearing down vqs

[Anuj Bolewar]

virtio_gpu_free_vbufs() destroys the vbufs kmem_cache after the virtqueues
have already been released. Commands that were queued but never completed
by the device leave their vbuffers stranded in the virtqueue, so the cache
still holds live objects when virtio_gpu_deinit() tears everything down.
This triggers a WARNING in virtio_gpu_free_vbufs:

    BUG virtio-gpu-vbufs (Not tainted): Objects remaining in cache
    on __kmem_cache_shutdown()

Drain any buffers still sitting in the control and cursor virtqueues in
virtio_gpu_deinit() after the device has been reset and before the
virtqueues are deleted, following the same pattern used by virtio_console's
remove_vqs(). Each reclaimed buffer is released with free_vbuf(), dropping
the reference on any GEM objects it holds. Pending RESOURCE_UNREF
commands are handled as well: their resp_cb_data still references a GEM
object, so it is cleaned up with virtio_gpu_cleanup_object() to avoid
leaking it on teardown.

Reported-by: syzbot+06f9b2a53ba4a5a47644@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=06f9b2a53ba4a5a47644
Signed-off-by: Anuj Bolewar <bolewara@gmail.com>
---
This series fixes a syzbot-triggered WARNING in virtio_gpu_free_vbufs
(cache object: virtio-gpu-vbufs) seen on device removal. Commands that
are queued but never complete leave vbuffers stranded in the control and
cursor virtqueues. virtio_gpu_deinit() reset the device and deleted the
virtqueues without draining them, so a later kmem_cache_destroy() in
virtio_gpu_release() ran with live objects still allocated.

Patch 1 drains the queues in virtio_gpu_deinit(): after the device reset
and before del_vqs(), virtio_gpu_reclaim_vbufs() detaches every unused
buffer from both virtqueues, releases their object arrays, and runs the
pending resource-unref cleanup so the referenced GEM objects are freed
rather than leaked. This mirrors the drain pattern used by
virtio_console's remove_vqs().

Link: https://syzkaller.appspot.com/bug?extid=06f9b2a53ba4a5a47644
---
Changes in v2:
- Also release the GEM object referenced by vbuf->resp_cb_data when
  reclaiming stranded buffers, so pending RESOURCE_UNREF commands do not
  leak their underlying objects on teardown.
- Link to v1: https://patch.msgid.link/20260802-virtio-gpu-reclaim-vbufs-v1-1-9947f18b20e2@gmail.com
---
 drivers/gpu/drm/virtio/virtgpu_drv.h |  1 +
 drivers/gpu/drm/virtio/virtgpu_kms.c |  1 +
 drivers/gpu/drm/virtio/virtgpu_vq.c  | 15 +++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 7449907754a..3e491c80873 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -332,6 +332,7 @@ void virtio_gpu_array_put_free_work(struct work_struct *work);
 /* virtgpu_vq.c */
 int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
 void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev);
+void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev);
 void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev,
 				    struct virtio_gpu_object *bo,
 				    struct virtio_gpu_object_params *params,
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index b4329f28e97..e5a6ae679f3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -298,6 +298,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
 	flush_work(&vgdev->cursorq.dequeue_work);
 	flush_work(&vgdev->config_changed_work);
 	virtio_reset_device(vgdev->vdev);
+	virtio_gpu_reclaim_vbufs(vgdev);
 	vgdev->vdev->config->del_vqs(vgdev->vdev);
 }
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index e5e1af8b8e8..ab6106f4bdf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -208,6 +208,21 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
 	kmem_cache_free(vgdev->vbufs, vbuf);
 }
 
+void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev)
+{
+	struct virtio_gpu_vbuffer *vbuf;
+
+	while ((vbuf = virtqueue_detach_unused_buf(vgdev->ctrlq.vq))) {
+		if (vbuf->objs)
+			virtio_gpu_array_put_free(vbuf->objs);
+		if (vbuf->resp_cb_data)
+			virtio_gpu_cleanup_object(vbuf->resp_cb_data);
+		free_vbuf(vgdev, vbuf);
+	}
+	while ((vbuf = virtqueue_detach_unused_buf(vgdev->cursorq.vq)))
+		free_vbuf(vgdev, vbuf);
+}
+
 static void reclaim_vbufs(struct virtqueue *vq, struct list_head *reclaim_list)
 {
 	struct virtio_gpu_vbuffer *vbuf;

---
base-commit: 2d2338c93da79b3bfe4b6099a931d9468d539952
change-id: 20260802-virtio-gpu-reclaim-vbufs-4816cc8a5bf9

Best regards,
--  
Anuj Bolewar <bolewara@gmail.com>

Re: [PATCH v2] drm/virtio: reclaim pending vbufs before tearing down vqs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] Concurrent execution of workqueue and virtio_gpu_reclaim_vbufs causes data races and potential use-after-free.

Pre-existing issues:
- [High] Tasks waiting on vgdev->resp_wq hang forever because stranded vbufs are not responded to.
- [High] Incorrect flush order in virtio_gpu_deinit() can cause vgdev->obj_free_work to execute after driver unload.
- [High] Fences associated with pending commands are never signaled or removed from the fence_drv.fences list on teardown.
- [High] OOM during fence allocation or SG table mapping leaves BOs locked indefinitely and leaks command buffers.
--

--- Patch [1]: [PATCH v2] drm/virtio: reclaim pending vbufs before tearing down vqs ---
commit deb7b9f45646f92497e6f83adda8ac1c7224985d
Author: Anuj Bolewar <bolewara@gmail.com>

drm/virtio: reclaim pending vbufs before tearing down vqs

virtio_gpu_free_vbufs() destroys the vbufs kmem_cache after the virtqueues
have already been released. Commands that were queued but never completed
by the device leave their vbuffers stranded in the virtqueue, so the cache
still holds live objects when virtio_gpu_deinit() tears everything down.

Link: https://syzkaller.appspot.com/bug?extid=06f9b2a53ba4a5a47644

> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -292,10 +292,11 @@ void virtio_gpu_deinit(struct drm_device *dev)
>  {
>  	struct virtio_gpu_device *vgdev = dev->dev_private;
>  
>  	flush_work(&vgdev->obj_free_work);
>  	flush_work(&vgdev->ctrlq.dequeue_work);

[Severity: High]
This is a pre-existing issue, but does this flush order allow
obj_free_work to execute after driver unload?

If dequeue_work runs and processes command completions, it calls
virtio_gpu_array_put_free_delayed() which can schedule new obj_free_work.

Since obj_free_work is flushed first, any new work queued during the
dequeue_work flush will never be waited upon. This delayed work can then
execute after the device is freed, potentially causing a use-after-free on
vgdev->obj_free_lock.

>  	flush_work(&vgdev->cursorq.dequeue_work);
>  	flush_work(&vgdev->config_changed_work);
>  	virtio_reset_device(vgdev->vdev);
> +	virtio_gpu_reclaim_vbufs(vgdev);

[Severity: High]
Could this lead to concurrent execution of dequeue_work and
virtio_gpu_reclaim_vbufs()?

Because flush_work() is called before virtio_reset_device(), if a device
interrupt fires in the window after the flush but before the reset,
dequeue_work could be rescheduled. 

virtio_gpu_reclaim_vbufs() then executes and locklessly calls
virtqueue_detach_unused_buf(). If dequeue_work executes concurrently, it
takes vgdev->ctrlq.qlock and operates on the same virtqueue. This might
cause a data race corrupting the virtqueue or a use-after-free if it
accesses it after del_vqs().

>  	vgdev->vdev->config->del_vqs(vgdev->vdev);
>  }

> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
[ ... ]
> +void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev)
> +{
> +	struct virtio_gpu_vbuffer *vbuf;
> +
> +	while ((vbuf = virtqueue_detach_unused_buf(vgdev->ctrlq.vq))) {
> +		if (vbuf->objs)
> +			virtio_gpu_array_put_free(vbuf->objs);
> +		if (vbuf->resp_cb_data)
> +			virtio_gpu_cleanup_object(vbuf->resp_cb_data);
> +		free_vbuf(vgdev, vbuf);
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but could tasks waiting on
vgdev->resp_wq hang forever because stranded vbufs are not responded to?

The loop manually cleans up objs and resp_cb_data but does not invoke
vbuf->resp_cb or wake up vgdev->resp_wq.

For example, in virtgpu_virtio_get_uuid():

    wait_event(vgdev->resp_wq, bo->uuid_state != STATE_INITIALIZING);

A task waiting here will never be woken up if the device is removed and
the stranded UUID command is reclaimed without calling the response callback.

[Severity: High]
This is also a pre-existing issue, but are fences associated with pending
commands left unsignaled and leaked on teardown?

When commands are queued, virtio_gpu_fence_emit() adds the fence:

    list_add_tail(&fence->node, &drv->fences);

This reclaim loop frees the command buffers but completely ignores the fences.
Does this leave the fences permanently in the driver's list, causing
memory leaks and indefinite hangs for any userspace tasks waiting on them?

> +	while ((vbuf = virtqueue_detach_unused_buf(vgdev->cursorq.vq)))
> +		free_vbuf(vgdev, vbuf);
> +}

> --- a/drivers/gpu/drm/virtio/virtgpu_submit.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_submit.c
[ ... ]
>  		out_fence = virtio_gpu_fence_alloc(vgdev, fence_ctx, ring_idx);
>  	else
>  		out_fence = NULL;
>  
>  	if (drm_fence_event) {
>  		err = virtio_gpu_fence_event_create(dev, file, out_fence, ring_idx);

[Severity: High]
This is a pre-existing issue, but can an OOM during fence allocation leave
buffer objects locked indefinitely?

If virtio_gpu_fence_alloc() fails and returns NULL, it proceeds without
checking the error. If drm_fence_event is true, it passes the NULL out_fence
to virtio_gpu_fence_event_create(), causing a NULL pointer dereference.

Otherwise, it locks BOs but later in virtio_gpu_queue_ctrl_sgs():

    if (fence) {
        virtio_gpu_fence_emit(vgdev, virtio_gpu_vbuf_ctrl_hdr(vbuf),
                              fence);
        if (vbuf->objs) {
            virtio_gpu_array_add_fence(vbuf->objs, &fence->f);
            virtio_gpu_array_unlock_resv(vbuf->objs);
        }
    }

Because the fence allocation failed and fence is NULL, the BOs are never
unlocked, causing a permanent deadlock on the next access.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802-virtio-gpu-reclaim-vbufs-v2-1-5767fb860691@gmail.com?part=1