Re: [PATCH] drm/tegra: Finalize GEM object on initialization failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] A racing userspace thread can trigger a use-after-free by guessing the GEM handle during creation, leading to premature object destruction.
--

--- Patch [1]: [PATCH] drm/tegra: Finalize GEM object on initialization failure ---
commit dfc541dc5f8719918460cc54c0dfa389b27272fd
Author: Guangshuo Li <lgs201920130244@gmail.com>

drm/tegra: Finalize GEM object on initialization failure

The commit introduces a call to drm_gem_private_object_fini() on the failure 
path of drm_gem_object_init() inside tegra_bo_alloc_object() before freeing
the buffer object, ensuring proper cleanup of the partially initialized GEM
object.

> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> index ace3e5a805cf7..9ce8ecf1a2335 100644
> --- a/drivers/gpu/drm/tegra/gem.c
> +++ b/drivers/gpu/drm/tegra/gem.c
> @@ -312,8 +312,10 @@ static struct tegra_bo *tegra_bo_alloc_object(struct drm_device *drm,
>  	size = round_up(size, PAGE_SIZE);
>  
>  	err = drm_gem_object_init(drm, &bo->gem, size);
> -	if (err < 0)
> +	if (err < 0) {
> +		drm_gem_private_object_fini(&bo->gem);
>  		goto free;
> +	}
>  
>  	err = drm_gem_create_mmap_offset(&bo->gem);
>  	if (err < 0)

[Severity: High]
This is a pre-existing issue, but while reviewing this area of the code
I noticed a potential use-after-free in the related
tegra_bo_create_with_handle() function.

If drm_gem_handle_create() fails, the error path unconditionally calls
tegra_bo_free_object():

drivers/gpu/drm/tegra/gem.c:tegra_bo_create_with_handle() {
	...
	err = drm_gem_handle_create(file, &bo->gem, handle);
	if (err) {
		tegra_bo_free_object(&bo->gem);
		return ERR_PTR(err);
	}
	...
}

Inside drm_gem_handle_create(), the handle is inserted into the IDR before
secondary operations like drm_vma_node_allow() are checked. If that check
fails, the handle is removed.

During the brief window where the handle is in the IDR, could a racing
userspace thread guess the handle and issue an ioctl (like
DRM_IOCTL_GEM_CLOSE) to look up the object and increment its refcount?

If that happens and tegra_bo_create_with_handle() receives the error, it
appears to ignore the elevated refcount and unconditionally free the object
by calling tegra_bo_free_object(). When the racing thread later calls
drm_gem_object_put(), does this result in dereferencing freed memory?

Would it be safer to use drm_gem_object_put() instead of directly calling
tegra_bo_free_object() here so that the object is only freed when the
racing thread drops its reference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714131923.1575860-1-lgs201920130244@gmail.com?part=1