virtio_gpu_queue_ctrl_sgs() drops the command when drm_dev_enter() fails
or when vqs_released is set. It unlocks the reservation and frees the
vbuf, but never releases the references that the caller handed over in
vbuf->objs. Those references are normally dropped from the dequeue path,
which never sees a command that was not queued.

If a framebuffer flush races with device removal, the GEM object backing
the fbdev buffer keeps a stale reference, is never freed, and its node is
still in the vma offset manager when the drm_device is released:

  Memory manager not clean during takedown.
  WARNING: drivers/gpu/drm/drm_mm.c:965 at drm_mm_takedown
  Call Trace:
   drm_managed_release
   drm_dev_put
   virtio_dev_remove
   virtio_pci_remove
   pci_device_remove
   remove_store

Drop the object references together with the vbuf on both early-exit
paths.

Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=d93b9064fe5f74af3aa3
Fixes: b7170f9457f2 ("drm/virtio: return virtio_gpu_queue errors")
Signed-off-by: Nguyen Ngoc Thang <[email protected]>
---
 drivers/gpu/drm/virtio/virtgpu_vq.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c 
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index c02c03c10d92..b36c4da04b18 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -208,6 +208,17 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
        kmem_cache_free(vgdev->vbufs, vbuf);
 }
 
+static void virtio_gpu_drop_vbuf(struct virtio_gpu_device *vgdev,
+                                struct virtio_gpu_vbuffer *vbuf, bool locked)
+{
+       if (vbuf->objs) {
+               if (locked)
+                       virtio_gpu_array_unlock_resv(vbuf->objs);
+               virtio_gpu_array_put_free(vbuf->objs);
+       }
+       free_vbuf(vgdev, vbuf);
+}
+
 void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev)
 {
        struct virtio_gpu_vbuffer *vbuf;
@@ -410,9 +421,7 @@ static int virtio_gpu_queue_ctrl_sgs(struct 
virtio_gpu_device *vgdev,
        int ret, idx;
 
        if (!drm_dev_enter(vgdev->ddev, &idx)) {
-               if (fence && vbuf->objs)
-                       virtio_gpu_array_unlock_resv(vbuf->objs);
-               free_vbuf(vgdev, vbuf);
+               virtio_gpu_drop_vbuf(vgdev, vbuf, fence);
                return -ENODEV;
        }
 
@@ -432,9 +441,7 @@ static int virtio_gpu_queue_ctrl_sgs(struct 
virtio_gpu_device *vgdev,
                 * synchronize_srcu() wait in drm_dev_unplug().
                 */
                if (vgdev->vqs_released) {
-                       if (fence && vbuf->objs)
-                               virtio_gpu_array_unlock_resv(vbuf->objs);
-                       free_vbuf(vgdev, vbuf);
+                       virtio_gpu_drop_vbuf(vgdev, vbuf, fence);
                        drm_dev_exit(idx);
                        return -ENODEV;
                }
-- 
2.43.0


Reply via email to