| Message ID | 1457086284-14054-1-git-send-email-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 04, 2016 at 10:11:24AM +0000, Matthew Auld wrote: > When binding pages for a partial view we should check that the offset + > size is valid relative to the size of the gem object. > > v2: Don't use pages->nents to determine the page count (Tvrtko Ursulin) > v3: Handle potential overflow (Chris Wilson) > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 7b8de85..596692b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -3493,6 +3493,13 @@ intel_partial_pages(const struct i915_ggtt_view *view, > struct sg_page_iter obj_sg_iter; > int ret = -ENOMEM; > > + if (U64_MAX - view->params.partial.offset < view->params.partial.size) > + return ERR_PTR(-ERANGE); Idiomatically is this how we test for offset+size overflows? > + if (view->params.partial.offset + view->params.partial.size > > + obj->base.size >> PAGE_SHIFT) > + return ERR_PTR(-EINVAL); This is still idiotic (placement, choice of runtime errors for a programmer error). If this concerns you that, please look at the API, and please review the outstanding patches. -Chris
> If this concerns you that, please look at the API,
and please review the outstanding patches.
Could you elaborate on this please?
What patches are you referring to?
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 7b8de85..596692b 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3493,6 +3493,13 @@ intel_partial_pages(const struct i915_ggtt_view *view, struct sg_page_iter obj_sg_iter; int ret = -ENOMEM; + if (U64_MAX - view->params.partial.offset < view->params.partial.size) + return ERR_PTR(-ERANGE); + + if (view->params.partial.offset + view->params.partial.size > + obj->base.size >> PAGE_SHIFT) + return ERR_PTR(-EINVAL); + st = kmalloc(sizeof(*st), GFP_KERNEL); if (!st) goto err_st_alloc;
When binding pages for a partial view we should check that the offset + size is valid relative to the size of the gem object. v2: Don't use pages->nents to determine the page count (Tvrtko Ursulin) v3: Handle potential overflow (Chris Wilson) Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++++++ 1 file changed, 7 insertions(+)