Re: [PATCH 4/7] drm/syncobj: Use put_user in drm_syncobj_query_ioctl

["Maíra Canal" <mcanal@igalia.com>]

Hi Tvrtko,

On 18/03/25 12:54, Tvrtko Ursulin wrote:
> Since the query loop is using copy_to_user() to write out a single u64 at
> a time it feels more natural (and is a tiny bit more compact) to replace
> it with put_user().
> 
> Access_ok() check is added to the input checking for an early bailout in
> case of a bad buffer passed in.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Reviewed-by: Maíra Canal <mcanal@igalia.com>

Best Regards,
- Maíra

> ---
>   drivers/gpu/drm/drm_syncobj.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index cdda2df06bec..74d1dc0d1f8b 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1643,6 +1643,9 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>   	if (args->count_handles == 0)
>   		return -EINVAL;
>   
> +	if (!access_ok(points, args->count_handles * sizeof(*points)))
> +		return -EFAULT;
> +
>   	ret = drm_syncobj_array_find(file_private,
>   				     u64_to_user_ptr(args->handles),
>   				     args->count_handles,
> @@ -1684,10 +1687,10 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>   			point = 0;
>   		}
>   		dma_fence_put(fence);
> -		ret = copy_to_user(&points[i], &point, sizeof(uint64_t));
> -		ret = ret ? -EFAULT : 0;
> -		if (ret)
> +		if (__put_user(point, points++)) {
> +			ret = -EFAULT;
>   			break;
> +		}
>   	}
>   	drm_syncobj_array_free(syncobjs, args->count_handles);
>