| Message ID | 1456765862-25589-1-git-send-email-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 29, 2016 at 05:11:02PM +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. > > 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 | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 49e4f26..a477bb2 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -3500,6 +3500,10 @@ intel_partial_pages(const struct i915_ggtt_view *view, > struct sg_page_iter obj_sg_iter; > int ret = -ENOMEM; > > + if (view->params.partial.offset + view->params.partial.size > > + obj->pages->nents) > + return ERR_PTR(-EINVAL); It seems to me that if we hit this, there must a bug somewhere higher up. > + > st = kmalloc(sizeof(*st), GFP_KERNEL); > if (!st) > goto err_st_alloc; > -- > 2.4.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On ma, 2016-02-29 at 17:11 +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. > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 49e4f26..a477bb2 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -3500,6 +3500,10 @@ intel_partial_pages(const struct i915_ggtt_view *view, > struct sg_page_iter obj_sg_iter; > int ret = -ENOMEM; > > + if (view->params.partial.offset + view->params.partial.size > > + obj->pages->nents) > + return ERR_PTR(-EINVAL); > + > st = kmalloc(sizeof(*st), GFP_KERNEL); > if (!st) > goto err_st_alloc;
On 29/02/16 17:11, 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. > > 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 | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 49e4f26..a477bb2 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -3500,6 +3500,10 @@ intel_partial_pages(const struct i915_ggtt_view *view, > struct sg_page_iter obj_sg_iter; > int ret = -ENOMEM; > > + if (view->params.partial.offset + view->params.partial.size > > + obj->pages->nents) > + return ERR_PTR(-EINVAL); > + obj->pages->nents is not guaranteed to be equal to number of pages but can be less than due sg entry coalescing. I suggest replacing with a check against "obj->base.size >> PAGE_SHIFT". > st = kmalloc(sizeof(*st), GFP_KERNEL); > if (!st) > goto err_st_alloc; > Regards, Tvrtko
On Wed, Mar 02, 2016 at 03:29:12PM +0200, Joonas Lahtinen wrote: > On ma, 2016-02-29 at 17:11 +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. > > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 49e4f26..a477bb2 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -3500,6 +3500,10 @@ intel_partial_pages(const struct i915_ggtt_view *view, > > struct sg_page_iter obj_sg_iter; > > int ret = -ENOMEM; > > > > + if (view->params.partial.offset + view->params.partial.size > > > + obj->pages->nents) > > + return ERR_PTR(-EINVAL); Wrong. Tell me again what nents has to do with the object size? -Chris
On ma, 2016-02-29 at 19:57 +0200, Ville Syrjälä wrote: > On Mon, Feb 29, 2016 at 05:11:02PM +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. > > > > 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 | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 49e4f26..a477bb2 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -3500,6 +3500,10 @@ intel_partial_pages(const struct i915_ggtt_view *view, > > struct sg_page_iter obj_sg_iter; > > int ret = -ENOMEM; > > > > + if (view->params.partial.offset + view->params.partial.size > > > + obj->pages->nents) > > + return ERR_PTR(-EINVAL); > It seems to me that if we hit this, there must a bug somewhere higher > up. > Currently yes. This is in preparation of the more widespread support for partial views and was chosen as a good get-to-know-GEM-code candidate. Regards, Joonas > > > > + > > st = kmalloc(sizeof(*st), GFP_KERNEL); > > if (!st) > > goto err_st_alloc; > > -- > > 2.4.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 49e4f26..a477bb2 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3500,6 +3500,10 @@ intel_partial_pages(const struct i915_ggtt_view *view, struct sg_page_iter obj_sg_iter; int ret = -ENOMEM; + if (view->params.partial.offset + view->params.partial.size > + obj->pages->nents) + 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. 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 | 4 ++++ 1 file changed, 4 insertions(+)