| Message ID | 20200116192809.843138-2-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/userptr: add user_size limit check | expand |
Quoting Matthew Auld (2020-01-16 19:28:09) > If we create a rather large userptr object(e.g 1ULL << 32) we might > shift past the type-width of num_pages: (int)num_pages << PAGE_SHIFT, > resulting in a totally bogus sg_table, which fortunately will eventually > manifest as: > > gen8_ppgtt_insert_huge:463 GEM_BUG_ON(iter->sg->length < page_size) > kernel BUG at drivers/gpu/drm/i915/gt/gen8_ppgtt.c:463! > > Fixes: 5cc9ed4b9a7a ("drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl") > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 ++- > drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > index fef96a303d9d..81fa53495c9d 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > @@ -405,6 +405,7 @@ static struct sg_table * > __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object *obj, > struct page **pvec, int num_pages) Well that answers that question. Why not use unsigned long? That's what the comment is all about, fixing the code! Not continuing to bodge it. -Chris
Quoting Matthew Auld (2020-01-16 19:28:09) > diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > index 077b8f7cf6cb..0d7820c49f5b 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c > @@ -379,6 +379,7 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, > pd = i915_pd_entry(pdp, gen8_pd_index(idx, 2)); > vaddr = kmap_atomic_px(i915_pt_entry(pd, gen8_pd_index(idx, 1))); > do { > + GEM_BUG_ON(iter->sg->length < PAGE_SIZE); s/PAGE_SIZE/I915_GTT_PAGE_SIZE/ > vaddr[gen8_pd_index(idx, 0)] = pte_encode | iter->dma; > > iter->dma += I915_GTT_PAGE_SIZE; > -- > 2.20.1 >
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index fef96a303d9d..81fa53495c9d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -405,6 +405,7 @@ static struct sg_table * __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object *obj, struct page **pvec, int num_pages) { + unsigned long size = (unsigned long)num_pages << PAGE_SHIFT; unsigned int max_segment = i915_sg_segment_size(); struct sg_table *st; unsigned int sg_page_sizes; @@ -416,7 +417,7 @@ __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object *obj, alloc_table: ret = __sg_alloc_table_from_pages(st, pvec, num_pages, - 0, num_pages << PAGE_SHIFT, + 0, size, max_segment, GFP_KERNEL); if (ret) { diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index 077b8f7cf6cb..0d7820c49f5b 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -379,6 +379,7 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, pd = i915_pd_entry(pdp, gen8_pd_index(idx, 2)); vaddr = kmap_atomic_px(i915_pt_entry(pd, gen8_pd_index(idx, 1))); do { + GEM_BUG_ON(iter->sg->length < PAGE_SIZE); vaddr[gen8_pd_index(idx, 0)] = pte_encode | iter->dma; iter->dma += I915_GTT_PAGE_SIZE;
If we create a rather large userptr object(e.g 1ULL << 32) we might shift past the type-width of num_pages: (int)num_pages << PAGE_SHIFT, resulting in a totally bogus sg_table, which fortunately will eventually manifest as: gen8_ppgtt_insert_huge:463 GEM_BUG_ON(iter->sg->length < page_size) kernel BUG at drivers/gpu/drm/i915/gt/gen8_ppgtt.c:463! Fixes: 5cc9ed4b9a7a ("drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl") Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 ++- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)