Re: [PATCH] drm/gem: Try to fix change_handle ioctl, attempt 4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The ioctl returns 0 on `args->handle == args->new_handle` without validating if the handle actually exists in the IDR.
--

commit c59eb6aaceabfd08a4835d7c2c0133017afcb072
Author: Simona Vetter <simona.vetter@ffwll.ch>

drm/gem: Try to fix change_handle ioctl, attempt 4

This commit attempts to fix a race condition in the change_handle ioctl by
reworking the two-stage idr replacement logic and renaming variables for
clarity. It also unifies the object lookup and idr_replace steps and disables
the ioctl pending testing.

> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index ac751a42db281..55caad80a3503 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
[ ... ]
> @@ -1028,52 +1028,36 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data,
>  	/* idr_alloc() limitation. */
>  	if (args->new_handle > INT_MAX)
>  		return -EINVAL;
> -	handle = args->new_handle;
> -
> -	obj = drm_gem_object_lookup(file_priv, args->handle);
> -	if (!obj)
> -		return -ENOENT;
> +	new_handle = args->new_handle;
>  
> -	if (args->handle == handle) {
> -		ret = 0;
> -		goto out;
> -	}
> +	if (args->handle == new_handle)
> +		return 0;

[Severity: High]
Does this new early return skip handle validation?

Previously, drm_gem_object_lookup() was called before this check, returning
-ENOENT if the given handle was invalid. In the new code, if a user passes
a non-existent handle but sets new_handle to the exact same value, the ioctl
will return 0 (success) instead of -ENOENT. 

Should the handle existence still be verified before returning success here?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604194437.1725314-1-simona.vetter@ffwll.ch?part=1