All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/virtio: fix object leak when drm_gem_handle_create() fails
@ 2026-08-15  6:16 ` Junrui Luo
  0 siblings, 0 replies; 3+ messages in thread
From: Junrui Luo via B4 Relay @ 2026-08-15  6:16 UTC (permalink / raw)
  To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
	Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Dave Airlie, Michael S. Tsirkin
  Cc: dri-devel, virtualization, linux-kernel, Yuhao Jiang, Junrui Luo

From: Junrui Luo <moonafterrain@outlook.com>

virtio_gpu_gem_create() owns the reference taken by
virtio_gpu_object_create(). On the drm_gem_handle_create() error path it
calls drm_gem_object_release() instead of dropping that reference.

drm_gem_object_release() is the inverse of drm_gem_object_init() and does
not touch the reference count or call obj->funcs->free(), so it is only
correct as the last step of a destructor, as in
virtio_gpu_cleanup_object(). Using it here leaves the bo at refcount 1
with no remaining reference, so virtio_gpu_free_object() never runs and
the shmem pages, sg table and virtio_gpu_object are leaked. Since
virtio_gpu_object_create() has already set bo->created,
VIRTIO_GPU_CMD_RESOURCE_UNREF is not queued either, leaking the host-side
resource and the resource id.

drm_gem_handle_create_tail() drops the handle reference on all of its
internal error paths, so the caller only has to drop its own. Use
drm_gem_object_put(), matching the success path below.

Fixes: dc5698e80cf7 ("Add virtio gpu driver.")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/gpu/drm/virtio/virtgpu_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 66c3f6f74e9c..d2f0b8a3f172 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -45,7 +45,7 @@ static int virtio_gpu_gem_create(struct drm_file *file,
 
 	ret = drm_gem_handle_create(file, &obj->base.base, &handle);
 	if (ret) {
-		drm_gem_object_release(&obj->base.base);
+		drm_gem_object_put(&obj->base.base);
 		return ret;
 	}
 

---
base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
change-id: 20260815-virtgpu-gem-create-leak-8620531cd3d8

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH] drm/virtio: fix object leak when drm_gem_handle_create() fails
@ 2026-08-15  6:16 ` Junrui Luo
  0 siblings, 0 replies; 3+ messages in thread
From: Junrui Luo @ 2026-08-15  6:16 UTC (permalink / raw)
  To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
	Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Dave Airlie, Michael S. Tsirkin
  Cc: dri-devel, virtualization, linux-kernel, Yuhao Jiang, Junrui Luo

virtio_gpu_gem_create() owns the reference taken by
virtio_gpu_object_create(). On the drm_gem_handle_create() error path it
calls drm_gem_object_release() instead of dropping that reference.

drm_gem_object_release() is the inverse of drm_gem_object_init() and does
not touch the reference count or call obj->funcs->free(), so it is only
correct as the last step of a destructor, as in
virtio_gpu_cleanup_object(). Using it here leaves the bo at refcount 1
with no remaining reference, so virtio_gpu_free_object() never runs and
the shmem pages, sg table and virtio_gpu_object are leaked. Since
virtio_gpu_object_create() has already set bo->created,
VIRTIO_GPU_CMD_RESOURCE_UNREF is not queued either, leaking the host-side
resource and the resource id.

drm_gem_handle_create_tail() drops the handle reference on all of its
internal error paths, so the caller only has to drop its own. Use
drm_gem_object_put(), matching the success path below.

Fixes: dc5698e80cf7 ("Add virtio gpu driver.")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/gpu/drm/virtio/virtgpu_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 66c3f6f74e9c..d2f0b8a3f172 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -45,7 +45,7 @@ static int virtio_gpu_gem_create(struct drm_file *file,
 
 	ret = drm_gem_handle_create(file, &obj->base.base, &handle);
 	if (ret) {
-		drm_gem_object_release(&obj->base.base);
+		drm_gem_object_put(&obj->base.base);
 		return ret;
 	}
 

---
base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
change-id: 20260815-virtgpu-gem-create-leak-8620531cd3d8

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/virtio: fix object leak when drm_gem_handle_create() fails
  2026-08-15  6:16 ` Junrui Luo
  (?)
@ 2026-08-15  6:26 ` sashiko-bot
  -1 siblings, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-15  6:26 UTC (permalink / raw)
  To: Junrui Luo; +Cc: dri-devel

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

Pre-existing issues:
- [High] The patch fixes an object leak when drm_gem_handle_create() fails, but misses identical preexisting leaks in virtio_gpu_resource_create_ioctl() and virtio_gpu_resource_create_blob_ioctl().
- [High] Missing cleanup of GEM object internals and fake mmap offset on error paths in virtio_gpu_vram_create().
--

--- Patch [1]: [PATCH] drm/virtio: fix object leak when drm_gem_handle_create() fails ---
commit f5c2e8ad38dcc08de3708efd9e20d6ef52e2a1e8
Author: Junrui Luo <moonafterrain@outlook.com>

drm/virtio: fix object leak when drm_gem_handle_create() fails

virtio_gpu_gem_create() owns the reference taken by
virtio_gpu_object_create(). On the drm_gem_handle_create() error path it
calls drm_gem_object_release() instead of dropping that reference.

> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index 66c3f6f74e9c6..d2f0b8a3f172b 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -45,7 +45,7 @@ static int virtio_gpu_gem_create(struct drm_file *file,
>  
>  	ret = drm_gem_handle_create(file, &obj->base.base, &handle);
>  	if (ret) {
> -		drm_gem_object_release(&obj->base.base);
> +		drm_gem_object_put(&obj->base.base);
>  		return ret;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but there are other locations with
this exact same pattern. Does virtio_gpu_resource_create_ioctl() also leak the
object when drm_gem_handle_create() fails?

drivers/gpu/drm/virtio/virtgpu_ioctl.c:virtio_gpu_resource_create_ioctl() {
    ...
    ret = drm_gem_handle_create(file_priv, obj, &handle);
    if (ret) {
        drm_gem_object_release(obj);
        return ret;
    }
    ...
}

Similar leaks appear to be present in virtio_gpu_resource_create_blob_ioctl()
where drm_gem_object_release() is called instead of drm_gem_object_put() on
multiple error paths. Should those be updated to use drm_gem_object_put()
as well to ensure the free hooks are properly executed?

[Severity: High]
This is a pre-existing issue, but might we be missing cleanup of GEM internals
and the mmap offset on error paths in virtio_gpu_vram_create()?

drivers/gpu/drm/virtio/virtgpu_vram.c:virtio_gpu_vram_create() {
    ...
    ret = drm_gem_create_mmap_offset(&vram->base.base.base);
    if (ret) {
        kfree(vram);
        return ret;
    }

    ret = virtio_gpu_resource_id_get(vgdev, &vram->base.hw_res_handle);
    if (ret) {
        kfree(vram);
        return ret;
    }
    ...
}

If virtio_gpu_resource_id_get() fails, drm_gem_create_mmap_offset() has
already added the fake offset to the DRM VMA offset manager's interval tree.
Would calling kfree(vram) here leave a dangling pointer in the interval tree,
potentially causing a use-after-free or tree corruption upon subsequent
accesses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815-virtgpu-gem-create-leak-v1-1-a4e9fb18caa3@outlook.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-15  6:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-15  6:16 [PATCH] drm/virtio: fix object leak when drm_gem_handle_create() fails Junrui Luo via B4 Relay
2026-08-15  6:16 ` Junrui Luo
2026-08-15  6:26 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.