On Thu, 8 Jul 2021 12:56:19 +0200 Christian König christian.koenig@amd.com wrote:
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?
Yes, I will send v2 soon. Thank you!
With regards, Pavel Skripkin