Messages in this thread | | | Subject | Re: [PATCH] gpu: ttm: fix GPF in ttm_bo_release | From | Christian König <> | Date | Thu, 8 Jul 2021 12:56:19 +0200 |
| |
Am 08.07.21 um 12:09 schrieb Pavel Skripkin: > On Thu, 8 Jul 2021 11:37:01 +0300 > Pavel Skripkin <paskripkin@gmail.com> wrote: > >> On Thu, 8 Jul 2021 08:49:48 +0200 >> Christian König <christian.koenig@amd.com> 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
| |