Re: [PATCH] gpu: ttm: fix GPF in ttm_bo_release

[Christian König]

Am 08.07.21 um 12:09 schrieb Pavel Skripkin:
> On Thu, 8 Jul 2021 11:37:01 +0300
> Pavel Skripkin <paskripkin@xxxxxxxxx> wrote:
>
> > On Thu, 8 Jul 2021 08:49:48 +0200
> > Christian König <christian.koenig@xxxxxxx> wrote:
> >
> > > Am 07.07.21 um 20:51 schrieb Pavel Skripkin:
> > > > My local syzbot instance hit GPF in ttm_bo_release().
> > > > Unfortunately, syzbot didn't produce a reproducer for this, but I
> > > > found out possible scenario:
> > > >
> > > > drm_gem_vram_create()            <-- drm_gem_vram_object kzalloced
> > > > 				     (bo embedded in this object)
> > > >     ttm_bo_init()
> > > >       ttm_bo_init_reserved()
> > > >         ttm_resource_alloc()
> > > >           man->func->alloc()       <-- allocation failure
> > > >         ttm_bo_put()
> > > > 	ttm_bo_release()
> > > > 	  ttm_mem_io_free()      <-- bo->resource == NULL passed
> > > > 				     as second argument
> > > > 	     *GPF*
> > > >
> > > > So, I've added check in ttm_bo_release() to avoid passing
> > > > NULL as second argument to ttm_mem_io_free().
> > Hi, Christian!
> >
> > Thank you for quick feedback :)
> >
> > > There is another ocassion of this a bit down before we call
> > > ttm_bo_move_to_lru_tail() apart from that good catch.
> > Did you mean, that ttm_bo_move_to_lru_tail() should have NULL check
> > too?

Yes, exactly that.

> >   I checked it's realization, and, I think, NULL check is necessary
> > there, since mem pointer is dereferenced w/o any checking
> >
> > > But I'm wondering if we should make the functions NULL save instead
> > > of the external check.
> > I tried to find more possible scenarios of GPF in ttm_bo_release(),
> > but I didn't find one. But, yes, moving NULL check inside
> > ttm_mem_io_free() is more general approach and it will defend this
> > function from GPFs in the future.
> >
> >
> >
> > With regards,
> > Pavel Skripkin
> I misclicked and sent this email to Christian privately :(
>
> Added all thread participants back, sorry.

No problem.

Do you want to update your patch or should I take care of this?

Thanks,
Christian.

> With regards,
> Pavel Skripkin