Re: [PATCH] drm/i915/query: nospec expects no more than an unsigned long

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Chris Wilson (2018-05-22 13:17:06)
> Quoting Lionel Landwerlin (2018-05-22 13:13:03)
> > On 22/05/18 13:10, Chris Wilson wrote:
> > > nospec quite reasonably asserts that it will never be used with an index
> > > larger than unsigned long (that being the largest possibly index into an
> > > C array). However, our ubi uses the convention of u64 for any large
> > > integer, running afoul of the assertion on 32b. Reduce our index to an
> > > unsigned long, checking for type overflow first.
> > >
> > >    drivers/gpu/drm/i915/i915_query.c: In function 'i915_query_ioctl':
> > >    include/linux/compiler.h:339:38: error: call to '__compiletime_assert_119' declared with attribute error: BUILD_BUG_ON failed: sizeof(_s) > sizeof(long)
> > >
> > > Reported-by: kbuild-all@xxxxxx
> > > Fixes: 84b510e22da7 ("drm/i915/query: Protect tainted function pointer lookup")
> > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > > ---
> > >   drivers/gpu/drm/i915/i915_query.c | 5 ++++-
> > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> > > index 95f9d179afc4..3f502eef2431 100644
> > > --- a/drivers/gpu/drm/i915/i915_query.c
> > > +++ b/drivers/gpu/drm/i915/i915_query.c
> > > @@ -102,7 +102,7 @@ int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> > >   
> > >       for (i = 0; i < args->num_items; i++, user_item_ptr++) {
> > >               struct drm_i915_query_item item;
> > > -             u64 func_idx;
> > > +             unsigned long func_idx;
> > >               int ret;
> > >   
> > >               if (copy_from_user(&item, user_item_ptr, sizeof(item)))
> > > @@ -111,6 +111,9 @@ int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> > I guess you can get rid of this if (item.query_id == 0) then :
> 
> Hmm, we could indeed. The choice is whether we want to make it clear
> that id=0 is illegal (making it easier to add debug later?)

On second thoughts, I don't think so since u64==unsigned long on 64b, so
(u64)-1 should fit inside unsigned long.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux