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 <[email protected]>

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

Reply via email to