| Message ID | 20221028130635.465839-1-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/userptr: restore probe_range behaviour | expand |
* Matthew Auld <matthew.auld@intel.com> [221028 09:07]: > The conversion looks harmless, however the addr value is updated inside > the loop with the previous vm_end, which then incorrectly leads to > for_each_vma_range() iterating over stuff outside the range we care > about. Fix this by storing the end value separately. Also fix the case > where the range doesn't intersect with any vma, or if the vma itself > doesn't extend the entire range, which must mean we have hole at the > end. Both should result in an error, as per the previous behaviour. > > v2: Fix the cases where the range is empty, or if there's a hole at > the end of the range > Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7247 > Testcase: igt@gem_userptr_blits@probe > Fixes: f683b9d61319 ("i915: use the VMA iterator") > Reported-by: kernel test robot <oliver.sang@intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Cc: Matthew Wilcox (Oracle) <willy@infradead.org> > Cc: Liam R. Howlett <Liam.Howlett@Oracle.com> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Yu Zhao <yuzhao@google.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > index 1b1a22716722..ca7a388ba2bf 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > @@ -427,9 +427,10 @@ probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len) > { > VMA_ITERATOR(vmi, mm, addr); > struct vm_area_struct *vma; > + unsigned long end = addr + len; > > mmap_read_lock(mm); > - for_each_vma_range(vmi, vma, addr + len) { > + for_each_vma_range(vmi, vma, end) { > /* Check for holes, note that we also update the addr below */ > if (vma->vm_start > addr) > break; > @@ -441,7 +442,7 @@ probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len) > } > mmap_read_unlock(mm); > > - if (vma) > + if (vma || addr < end) > return -EFAULT; > return 0; > } > -- > 2.37.3 >
On 28.10.2022 15:06, Matthew Auld wrote: > The conversion looks harmless, however the addr value is updated inside > the loop with the previous vm_end, which then incorrectly leads to > for_each_vma_range() iterating over stuff outside the range we care > about. Fix this by storing the end value separately. Also fix the case > where the range doesn't intersect with any vma, or if the vma itself > doesn't extend the entire range, which must mean we have hole at the > end. Both should result in an error, as per the previous behaviour. > > v2: Fix the cases where the range is empty, or if there's a hole at > the end of the range > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7247 > Testcase: igt@gem_userptr_blits@probe > Fixes: f683b9d61319 ("i915: use the VMA iterator") > Reported-by: kernel test robot <oliver.sang@intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Cc: Matthew Wilcox (Oracle) <willy@infradead.org> > Cc: Liam R. Howlett <Liam.Howlett@Oracle.com> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Yu Zhao <yuzhao@google.com> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Regards Andrzej > --- > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > index 1b1a22716722..ca7a388ba2bf 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > @@ -427,9 +427,10 @@ probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len) > { > VMA_ITERATOR(vmi, mm, addr); > struct vm_area_struct *vma; > + unsigned long end = addr + len; > > mmap_read_lock(mm); > - for_each_vma_range(vmi, vma, addr + len) { > + for_each_vma_range(vmi, vma, end) { > /* Check for holes, note that we also update the addr below */ > if (vma->vm_start > addr) > break; > @@ -441,7 +442,7 @@ probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len) > } > mmap_read_unlock(mm); > > - if (vma) > + if (vma || addr < end) > return -EFAULT; > return 0; > }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 1b1a22716722..ca7a388ba2bf 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -427,9 +427,10 @@ probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len) { VMA_ITERATOR(vmi, mm, addr); struct vm_area_struct *vma; + unsigned long end = addr + len; mmap_read_lock(mm); - for_each_vma_range(vmi, vma, addr + len) { + for_each_vma_range(vmi, vma, end) { /* Check for holes, note that we also update the addr below */ if (vma->vm_start > addr) break; @@ -441,7 +442,7 @@ probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len) } mmap_read_unlock(mm); - if (vma) + if (vma || addr < end) return -EFAULT; return 0; }
The conversion looks harmless, however the addr value is updated inside the loop with the previous vm_end, which then incorrectly leads to for_each_vma_range() iterating over stuff outside the range we care about. Fix this by storing the end value separately. Also fix the case where the range doesn't intersect with any vma, or if the vma itself doesn't extend the entire range, which must mean we have hole at the end. Both should result in an error, as per the previous behaviour. v2: Fix the cases where the range is empty, or if there's a hole at the end of the range Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7247 Testcase: igt@gem_userptr_blits@probe Fixes: f683b9d61319 ("i915: use the VMA iterator") Reported-by: kernel test robot <oliver.sang@intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Matthew Wilcox (Oracle) <willy@infradead.org> Cc: Liam R. Howlett <Liam.Howlett@Oracle.com> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: Yu Zhao <yuzhao@google.com> --- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)