Re: Patch "drm/amdgpu: Fix init ordering in amdgpu_vram_mgr_init()" has been added to the 6.18-stable tree

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] If `drm_buddy_init()` fails, it frees its internal structures but leaves dangling pointers and a non-zero `mm->n_roots`. `amdgpu_vram_mgr_fini()` then unconditionally calls `drm_buddy_fini()`, which dereferences these pointers, causing a Use-After-Free or NULL pointer dereference crash.
--

commit 952cbc7ae0fee615a078c5b60b31a61af5185d75
Author: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>

Patch "drm/amdgpu: Fix init ordering in amdgpu_vram_mgr_init()" has been added to the 6.18-stable tree

This patch fixes an initialization ordering issue in amdgpu_vram_mgr_init().
It moves drmm_cgroup_register_region() to after the list and buddy allocator
are fully initialized. This prevents the module load error path from
dereferencing uninitialized list pointers or NULL pointers.

Link: https://patch.msgid.link/20260725100036.2372-2-thomas.hellstrom@linux.intel.com

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index 9d934c07fa6b8..2e457a0388a32 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c

[ ... ]

> @@ -934,6 +931,10 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
>  	if (err)
>  		return err;

[Severity: High]
This isn't a bug introduced by this patch, but can we still encounter a
kernel panic on the error path if drm_buddy_init() fails?

If drm_buddy_init() fails, it frees mm->roots on its error path but leaves
mm->n_roots > 0:

drivers/gpu/drm/drm_buddy.c:drm_buddy_init() {
    ...
    mm->n_roots = hweight64(size);
    ...
    do {
        ...
        root = drm_block_alloc(mm, NULL, order, offset);
        if (!root)
            goto out_free_roots;
    ...
out_free_roots:
    while (i--)
        drm_block_free(mm, mm->roots[i]);
    kfree(mm->roots);
    ...
}

Because the driver teardown path calls amdgpu_vram_mgr_fini() which
unconditionally calls drm_buddy_fini(), doesn't this result in
dereferencing the freed mm->roots array since mm->n_roots is still > 0?

drivers/gpu/drm/drm_buddy.c:drm_buddy_fini() {
    ...
    for (i = 0; i < mm->n_roots; ++i) {
        order = ilog2(size) - ilog2(mm->chunk_size);
        start = drm_buddy_block_offset(mm->roots[i]);
    ...
}

Could the error path in drm_buddy_init() reset mm->n_roots to 0, or could
amdgpu_vram_mgr_fini() skip calling drm_buddy_fini() if initialization failed?

> +	man->cg = drmm_cgroup_register_region(adev_to_drm(adev), "vram", adev->gmc.real_vram_size);
> +	if (IS_ERR(man->cg))
> +		return PTR_ERR(man->cg);
> +
>  	ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_VRAM, &mgr->manager);
>  	ttm_resource_manager_set_used(man, true);
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2026090937-mangle-cinema-f62f@gregkh?part=1