Message ID | 20180521210530.26008-1-chris@chris-wilson.co.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 21/05/18 22:05, Chris Wilson wrote: > Smatch identifies i915_query_ioctl() as being a potential victim of > Spectre due to it use of a tainted user index into a function pointer > array. Use array_index_nospec() to defang the user index before using it > to lookup the function pointer. > > Fixes: a446ae2c6e65 ("drm/i915: add query uAPI") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Okay. Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > --- > drivers/gpu/drm/i915/i915_query.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c > index 3ace929dd90f..95f9d179afc4 100644 > --- a/drivers/gpu/drm/i915/i915_query.c > +++ b/drivers/gpu/drm/i915/i915_query.c > @@ -4,6 +4,8 @@ > * Copyright © 2018 Intel Corporation > */ > > +#include <linux/nospec.h> > + > #include "i915_drv.h" > #include "i915_query.h" > #include <uapi/drm/i915_drm.h> > @@ -111,10 +113,12 @@ int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file) > > func_idx = item.query_id - 1; > > - if (func_idx < ARRAY_SIZE(i915_query_funcs)) > + ret = -EINVAL; > + if (func_idx < ARRAY_SIZE(i915_query_funcs)) { > + func_idx = array_index_nospec(func_idx, > + ARRAY_SIZE(i915_query_funcs)); > ret = i915_query_funcs[func_idx](dev_priv, &item); > - else > - ret = -EINVAL; > + } > > /* Only write the length back to userspace if they differ. */ > if (ret != item.length && put_user(ret, &user_item_ptr->length))
Quoting Lionel Landwerlin (2018-05-21 23:20:56) > On 21/05/18 22:05, Chris Wilson wrote: > > Smatch identifies i915_query_ioctl() as being a potential victim of > > Spectre due to it use of a tainted user index into a function pointer > > array. Use array_index_nospec() to defang the user index before using it > > to lookup the function pointer. > > > > Fixes: a446ae2c6e65 ("drm/i915: add query uAPI") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Okay. > > Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Indeed, it's all magic to me. Thanks for checking, I'll apply shortly. -Chris
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c index 3ace929dd90f..95f9d179afc4 100644 --- a/drivers/gpu/drm/i915/i915_query.c +++ b/drivers/gpu/drm/i915/i915_query.c @@ -4,6 +4,8 @@ * Copyright © 2018 Intel Corporation */ +#include <linux/nospec.h> + #include "i915_drv.h" #include "i915_query.h" #include <uapi/drm/i915_drm.h> @@ -111,10 +113,12 @@ int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file) func_idx = item.query_id - 1; - if (func_idx < ARRAY_SIZE(i915_query_funcs)) + ret = -EINVAL; + if (func_idx < ARRAY_SIZE(i915_query_funcs)) { + func_idx = array_index_nospec(func_idx, + ARRAY_SIZE(i915_query_funcs)); ret = i915_query_funcs[func_idx](dev_priv, &item); - else - ret = -EINVAL; + } /* Only write the length back to userspace if they differ. */ if (ret != item.length && put_user(ret, &user_item_ptr->length))
Smatch identifies i915_query_ioctl() as being a potential victim of Spectre due to it use of a tainted user index into a function pointer array. Use array_index_nospec() to defang the user index before using it to lookup the function pointer. Fixes: a446ae2c6e65 ("drm/i915: add query uAPI") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_query.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)