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? 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. With regards, Pavel Skripkin
