[PATCH 20/29] drm/i915/gem: Make an alignment check more sensible

[Daniel Vetter]

On Thu, May 27, 2021 at 11:26:41AM -0500, Jason Ekstrand wrote:
> What we really want to check is that size of the engines array, i.e.
> args->size - sizeof(*user) is divisible by the element size, i.e.
> sizeof(*user->engines) because that's what's required for computing the
> array length right below the check.  However, we're currently not doing
> this and instead doing a compile-time check that sizeof(*user) is
> divisible by sizeof(*user->engines) and avoiding the subtraction.  As
> far as I can tell, the only reason for the more confusing pair of checks
> is to avoid a single subtraction of a constant.
> 
> The other thing the BUILD_BUG_ON might be trying to implicitly check is
> that offsetof(user->engines) == sizeof(*user) and we don't have any
> weird padding throwing us off.  However, that's not the check it's doing
> and it's not even a reliable way to do that check.
> 
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>

Yeah a non-dense compiler should be able to figure this out, plus
set_engines isn't a hotpath.

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 12a148ba421b6..cf7c281977a3e 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1758,9 +1758,8 @@ set_engines(struct i915_gem_context *ctx,
>  		goto replace;
>  	}
>  
> -	BUILD_BUG_ON(!IS_ALIGNED(sizeof(*user), sizeof(*user->engines)));
>  	if (args->size < sizeof(*user) ||
> -	    !IS_ALIGNED(args->size, sizeof(*user->engines))) {
> +	    !IS_ALIGNED(args->size -  sizeof(*user), sizeof(*user->engines))) {
>  		drm_dbg(&i915->drm, "Invalid size for engine array: %d\n",
>  			args->size);
>  		return -EINVAL;
> -- 
> 2.31.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch