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 <[email protected]> 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/[email protected]?part=1
