Skip to content

Commit

Permalink
drm/i915/gem: Make an alignment check more sensible
Browse files Browse the repository at this point in the history
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@jlekstrand.net>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-21-jason@jlekstrand.net
  • Loading branch information
Jason Ekstrand authored and Daniel Vetter committed Jul 8, 2021
1 parent bc2ceb7 commit def25b7
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions drivers/gpu/drm/i915/gem/i915_gem_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -1723,9 +1723,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;
Expand Down

0 comments on commit def25b7

Please sign in to comment.