Re: [PATCH] drm/amdgpu: Fix use-after-free in amdgpu_vram_mgr_fini() on init failure

On 7/22/26 11:52, Arunpravin Paneer Selvam wrote:
> adev->mman.initialized is set to true before amdgpu_vram_mgr_init() is
> called, so any failure in amdgpu_vram_mgr_init() still leads to the full
> teardown path amdgpu_ttm_fini() -> amdgpu_vram_mgr_fini() ->
> gpu_buddy_fini() being run.
> 
> When gpu_buddy_init() fails, it frees its partial allocations but leaves
> mgr->mm.free_trees, mgr->mm.roots and the scoreboards as dangling
> pointers. gpu_buddy_fini() then unconditionally dereferences these
> (iterating mm->roots, kfree(mm->free_trees[i]), mm->used_scoreboard[i]),
> resulting in a double-free / use-after-free and a kernel panic during
> probe error handling.
> 
> The manager is only marked as used as the final step of a fully
> successful amdgpu_vram_mgr_init(), i.e. after gpu_buddy_init() has
> succeeded, and nothing in between can fail. Use that state to guard the
> buddy teardown so gpu_buddy_fini() is only called when the buddy
> allocator was actually initialized.
> 
> Reported-by: Sashiko-bot <[email protected]>
> Closes: https://sashiko.dev/#/patchset/[email protected]?part=1
> Cc: Christian König <[email protected]>
> Cc: Thomas Hellström <[email protected]>
> Cc: Maarten Lankhorst <[email protected]>
> Assisted-by: GitHub_Copilot:claude-opus-4.8
> Signed-off-by: Arunpravin Paneer Selvam <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index 2a241a5b12c42..38529a904deb6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -954,6 +954,9 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device *adev)
>  	struct ttm_resource_manager *man = &mgr->manager;
>  	int ret;
>  	struct amdgpu_vram_reservation *rsv, *temp;
> +	bool was_used;
> +
> +	was_used = ttm_resource_manager_used(man);

The used flag indicates if TTM can use this allocation back end but doesn't indicates if initialization was successfully or not.

So that approach here is clearly not correct.

Regards,
Christian.

>  
>  	ttm_resource_manager_set_used(man, false);
>  
> @@ -969,7 +972,7 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device *adev)
>  		gpu_buddy_free_list(&mgr->mm, &rsv->allocated, 0);
>  		kfree(rsv);
>  	}
> -	if (!adev->gmc.is_app_apu)
> +	if (!adev->gmc.is_app_apu && was_used)
>  		gpu_buddy_fini(&mgr->mm);
>  	mutex_unlock(&mgr->lock);
>