Re: [Intel-gfx] [PATCH] drm/i915: Fix possible null ptr dereferences

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Fri, 2021-12-03 at 13:02 +0200, Ville Syrjälä wrote:
> On Fri, Dec 03, 2021 at 12:02:57PM +0530, Pallavi Mishra wrote:
> > add null ptr checks to prevent crash/exceptions.
> 
> BUG_ON()s aren't going to fix anything.
> 
> > Signed-off-by: Pallavi Mishra <pallavi.mishra@intel.com>

Pallavi, 

The NULL pointer dereferences here are probably all false positives
from a static analyzer. However the GEM_BUG_ONs are fine to assert that
the assumption really holds and to clearly point out what's going wrong
if they are hit in CI tests.

But the commit message must reflect that.

/Thomas.


> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_ttm.c      | 3 +++
> >  drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 3 ++-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > index 218a9b3037c7..997fe73c205b 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > @@ -906,6 +906,8 @@ vm_access_ttm(struct vm_area_struct *area,
> > unsigned long addr,
> >         struct drm_i915_gem_object *obj =
> >                 i915_ttm_to_gem(area->vm_private_data);
> >  
> > +       GEM_BUG_ON(!obj);
> > +
> >         if (i915_gem_object_is_readonly(obj) && write)
> >                 return -EACCES;
> >  
> > @@ -966,6 +968,7 @@ static const struct drm_i915_gem_object_ops
> > i915_gem_ttm_obj_ops = {
> >  void i915_ttm_bo_destroy(struct ttm_buffer_object *bo)
> >  {
> >         struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> > +       GEM_BUG_ON(!obj);
> >  
> >         i915_gem_object_release_memory_region(obj);
> >         mutex_destroy(&obj->ttm.get_io_page.lock);
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> > index 80df9f592407..2b684903a9f5 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> > @@ -371,6 +371,7 @@ int i915_ttm_move_notify(struct
> > ttm_buffer_object *bo)
> >         struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> >         int ret;
> >  
> > +       GEM_BUG_ON(!obj);
> >         ret = i915_gem_object_unbind(obj,
> > I915_GEM_OBJECT_UNBIND_ACTIVE);
> >         if (ret)
> >                 return ret;
> > @@ -506,7 +507,7 @@ static void i915_ttm_memcpy_init(struct
> > i915_ttm_memcpy_arg *arg,
> >  
> >         dst_reg = i915_ttm_region(bo->bdev, dst_mem->mem_type);
> >         src_reg = i915_ttm_region(bo->bdev, bo->resource-
> > >mem_type);
> > -       GEM_BUG_ON(!dst_reg || !src_reg);
> > +       GEM_BUG_ON(!dst_reg || !src_reg || !obj);
> >  
> >         arg->dst_iter = !i915_ttm_cpu_maps_iomem(dst_mem) ?
> >                 ttm_kmap_iter_tt_init(&arg->_dst_iter.tt, dst_ttm)
> > :
> > -- 
> > 2.25.1
>