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

Pre-existing issues:
- [High] Premature exposure of the object to the IDR in `ttm_base_object_init` 
allows concurrent threads to acquire a reference before initialization 
completes, leading to a Use-After-Free if initialization fails.
--

commit 2a16041f7a57ed8c7355b93de9b4469d336c4166
Author: WenTao Liang <[email protected]>

fix: drm/vmwgfx: ttm_base_object_init: fix tfile reference leak on error paths

This commit fixes a tfile reference leak on error paths in
ttm_base_object_init by ensuring ttm_object_file_unref is called when
idr_alloc or ttm_ref_object_add fails.

Cc: [email protected]
Fixes: 0b8762e997df ("drm/ttm, drm/vmwgfx: Move the lock- and object 
functionality to the vmwgfx driver")

> diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.c 
> b/drivers/gpu/drm/vmwgfx/ttm_object.c
> index 2421b0dd057c0..93ae5a07d70ab 100644
> --- a/drivers/gpu/drm/vmwgfx/ttm_object.c
> +++ b/drivers/gpu/drm/vmwgfx/ttm_object.c

[ ... ]

> @@ -218,6 +218,10 @@ int ttm_base_object_init(struct ttm_object_file *tfile,
>       spin_lock(&tdev->object_lock);
>       idr_remove(&tdev->idr, base->handle);
>       spin_unlock(&tdev->object_lock);

[Severity: High]
This is a pre-existing issue, but does returning an error here after
briefly exposing the object in the IDR lead to a use-after-free?

In ttm_base_object_init(), idr_alloc() makes the object globally
visible before initialization completes. If ttm_ref_object_add() then
fails, this error path removes the object from the IDR and returns the
error to the caller, which will typically free the object's memory.

Could a concurrent thread grab a reference to this object from the IDR
between the idr_alloc() and this idr_remove()? For example, a thread
could guess the handle and use an ioctl to fetch the object via
ttm_base_object_lookup_for_ref(). 

If that concurrent thread successfully increments the reference count
during this window, and the original thread returns an error that causes
the caller to forcefully free the memory, wouldn't the concurrent thread
be left holding a dangling pointer to freed memory?

> +     ttm_object_file_unref(&base->tfile);
> +     return ret;
> +err_unref_tfile:
> +     ttm_object_file_unref(&base->tfile);
>       return ret;
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to