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: [email protected] <[email protected]> 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/[email protected] > 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
