[Intel-xe] [PATCH 2/2] drm/xe: properly check bounds for xe_wait_user_fence_ioctl()

[Souza, Jose]

On Thu, 2023-06-22 at 15:42 -0700, Paulo Zanoni wrote:
> If !no_engines, then we use copy_from_user to copy to the 'eci' array,
> which has XE_HW_ENGINE_MAX_INSTANCE members. The amount of members
> copied is given by the user in args->num_engines, so add code to check
> that args->num_engines does not exceed XE_HW_ENGINE_MAX_INSTANCE. It's
> an unsigned value so there's no need to check for negative values.
> 
> Fixes error messages such as:
> 
>     Buffer overflow detected (54 < 18446744073709551520)!
> 
> Very simple reproducer:
> 
>     https://people.freedesktop.org/~pzanoni/wait-user-fence-bug/

Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_wait_user_fence.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> 
> On a side note, I noticed the timeout is in jiffies. That is a very weird thing
> to ask from user space.

There is under review patches to change that to nsec.

> 
> Also, shouldn't we be using mostly copy_from_user() instead of
> __copy_from_user()?
> 
> diff --git a/drivers/gpu/drm/xe/xe_wait_user_fence.c b/drivers/gpu/drm/xe/xe_wait_user_fence.c
> index 3122374341d6..098e2a4cff3f 100644
> --- a/drivers/gpu/drm/xe/xe_wait_user_fence.c
> +++ b/drivers/gpu/drm/xe/xe_wait_user_fence.c
> @@ -121,6 +121,9 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>  			 addr & 0x7))
>  		return -EINVAL;
>  
> +	if (XE_IOCTL_ERR(xe, args->num_engines > XE_HW_ENGINE_MAX_INSTANCE))
> +		return -EINVAL;
> +
>  	if (!no_engines) {
>  		err = copy_from_user(eci, user_eci,
>  				     sizeof(struct drm_xe_engine_class_instance) *