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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] If `amdgpu_vram_mgr_init()` fails early, structures like `ttm_resource_manager`, mutexes, and list heads remain zero-initialized. `amdgpu_vram_mgr_fini()` runs unconditionally and accesses these, causing NULL pointer dereferences and kernel panics.
--

--- Patch [1]: [PATCH] drm/amdgpu: Fix use-after-free in amdgpu_vram_mgr_fini() on init failure ---
commit 48887fb6b109845d375cd21461abab0a51f3196c
Author: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>

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

The patch tries to fix a double-free or use-after-free when gpu_buddy_init()
fails by guarding gpu_buddy_fini() with ttm_resource_manager_used().

Closes: https://sashiko.dev/#/patchset/20260611173301.17473-1-thomas.hellstrom@linux.intel.com?part=1

> 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);

[Severity: High]
This isn't a bug introduced by this patch, but does this function still
crash if amdgpu_vram_mgr_init() fails early?

If amdgpu_vram_mgr_init() fails (for example, if drmm_cgroup_register_region()
returns an error), it bails out before ttm_resource_manager_init() can
initialize the man->lru list heads or mutex_init() runs.

When the error path unwinds through amdgpu_vram_mgr_fini(), the code
unconditionally calls:

	ttm_resource_manager_set_used(man, false);
	ret = ttm_resource_manager_evict_all(&adev->mman.bdev, man);

This traverses the uninitialized man->lru lists in ttm_bo_evict_first(),
where list_empty() evaluates to false on zeroed memory (since NULL != head),
causing list_first_entry() to return an invalid offset and eventually
dereferencing a NULL pointer.

If that doesn't crash, the function later unconditionally locks the
zero-initialized mutex and iterates over the zero-initialized list head:

	mutex_lock(&mgr->lock);
	list_for_each_entry_safe(rsv, temp, &mgr->reservations_pending, blocks)

Could we return early from amdgpu_vram_mgr_fini() if the manager wasn't
initialized, perhaps by using the was_used flag to skip these teardown
steps as well?

>  
>  	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);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722095201.658119-1-Arunpravin.PaneerSelvam@amd.com?part=1