[PATCH 0/2] drm/virtio: fix control-buffer leaks on queueing errors

[PATCH 0/2] drm/virtio: fix control-buffer leaks on queueing errors

[Yao Kai]

Fix two independent cleanup omissions for control buffers that do not
reach the virtqueue.

Patch 1 fixes the reproduced GEM/VMA leak when fbdev damage work races
with device unplug.

Patch 2 fixes the vmalloc_to_sgt() allocation-failure path, found by
code inspection.

Testing:
- Repeated virtio-gpu unbind/rebind no longer triggers the
  drm_mm_takedown warning.
- Built drivers/gpu/drm/virtio/virtgpu_vq.o with W=1.

Yao Kai (2):
  drm/virtio: release object array when device is unplugged
  drm/virtio: free control buffer when scatterlist allocation fails

 drivers/gpu/drm/virtio/virtgpu_vq.c | 3 +++
 1 file changed, 3 insertions(+)


base-commit: 8ba098e6b6ff0db8edf28528d1552be261af30d4
-- 
2.43.0

[PATCH 1/2] drm/virtio: release object array when device is unplugged

[Yao Kai]

Unbinding virtio-gpu while fbdev damage work is active can leave the VMA
offset manager non-empty when the DRM device is released:

  Console: switching to colour VGA+ 80x25
  ------------[ cut here ]------------
  Memory manager not clean during takedown.
  WARNING: drivers/gpu/drm/drm_mm.c:965 at drm_mm_takedown+0x28/0xc0
  Call Trace:
   drm_managed_release+0x95/0x130
   drm_dev_put+0x54/0x70
   virtio_dev_remove+0x3c/0x90
   device_release_driver_internal+0x19a/0x200
   unbind_store+0x9c/0xb0

The remaining node belongs to the fbdev GEM object:

  [drm:drm_mm_takedown] *ERROR* node [00100000 + 00000300]:
   drm_mm_insert_node_in_range+0x2bf/0x530
   drm_vma_offset_add+0x58/0x60
   __drm_gem_shmem_create+0x71/0x150
   virtio_gpu_object_create+0x41/0x3a0
   virtio_gpu_mode_dumb_create+0xdc/0x190
   drm_client_buffer_create_dumb+0x75/0xf0
   drm_fbdev_shmem_driver_fbdev_probe+0x8b/0x230
   __drm_fb_helper_initial_config_and_unlock+0x359/0x600
   drm_fbdev_client_hotplug+0x61/0xb0
   drm_client_register+0x5c/0x90
   drm_fbdev_client_setup+0xd6/0x1d0
   drm_client_setup+0x81/0xb0
   virtio_gpu_probe+0xd6/0x180

fbdev damage work can race with drm_dev_unplug(). It may build
vbuf->objs and take GEM references after the device has been marked
unplugged. drm_dev_enter() then fails, and the existing failure path
unlocks fenced reservations and frees the vbuffer without releasing the
object array. The unreachable array keeps the GEM object, and therefore
its VMA offset node, alive through device release.

Release the object array before freeing a control buffer rejected by
drm_dev_enter().

Fixes: b1df3a2b24a9 ("drm/virtio: add drm_driver.release callback.")
Cc: stable@vger.kernel.org
Reported-by: Ji Fa <jifa@huawei.com>
Signed-off-by: Yao Kai <yaokai34@huawei.com>
---
 drivers/gpu/drm/virtio/virtgpu_vq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index e5e1af8b8e8a..a470339b6747 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -383,6 +383,7 @@ static int virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
 	if (!drm_dev_enter(vgdev->ddev, &idx)) {
 		if (fence && vbuf->objs)
 			virtio_gpu_array_unlock_resv(vbuf->objs);
+		virtio_gpu_array_put_free(vbuf->objs);
 		free_vbuf(vgdev, vbuf);
 		return -ENODEV;
 	}
-- 
2.43.0

[PATCH 2/2] drm/virtio: free control buffer when scatterlist allocation fails

[Yao Kai]

A vmalloc_to_sgt() failure leaks the control vbuffer, its data buffer,
and any GEM references held by vbuf->objs.

The queueing code owns the vbuffer by this point, but the failure path
only unlocks fenced reservations before returning -ENOMEM. No caller
reclaims the unqueued buffer.

Drop the object-array references and free the vbuffer before returning.

Fixes: e1218b8c0cc1 ("drm/virtio: Use vmalloc for command buffer allocations.")
Cc: stable@vger.kernel.org
Signed-off-by: Yao Kai <yaokai34@huawei.com>
---
 drivers/gpu/drm/virtio/virtgpu_vq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index a470339b6747..7ccc487e3332 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -477,6 +477,8 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
 			if (!sgt) {
 				if (fence && vbuf->objs)
 					virtio_gpu_array_unlock_resv(vbuf->objs);
+				virtio_gpu_array_put_free(vbuf->objs);
+				free_vbuf(vgdev, vbuf);
 				return -ENOMEM;
 			}
 
-- 
2.43.0

Re: [PATCH 0/2] drm/virtio: fix control-buffer leaks on queueing errors

[Yao Kai]

Gentle ping. Any comments on this series?

Thanks,
Yao