Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Incomplete Fix / NULL Pointer Dereference or Use-After-Free on Error Path -- commit 2ee86f69ec7f41352206d6f701597e5f22e81241 Author: Thomas Hellström <[email protected]> drm/amdgpu: Fix init ordering in amdgpu_vram_mgr_init() drmm_cgroup_register_region() is called before INIT_LIST_HEAD() and gpu_buddy_init() in amdgpu_vram_mgr_init(). If it fails, the function returns early and bypasses those initializations. Closes: https://sashiko.dev/#/patchset/[email protected]?part=4 > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > index 2a241a5b12c42..ac3f71d77140a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c [ ... ] > @@ -935,6 +932,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 does this still leave a use-after-free or NULL pointer dereference on the error path if gpu_buddy_init() fails? If gpu_buddy_init() fails, it cleans up its partial allocations but leaves mgr->mm.free_trees as a dangling pointer or NULL. Because adev->mman.initialized is set to true before amdgpu_vram_mgr_init() is called, the error teardown path unconditionally calls amdgpu_ttm_fini(), which calls amdgpu_vram_mgr_fini(): drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:amdgpu_vram_mgr_fini() { ... if (!adev->gmc.is_app_apu) gpu_buddy_fini(&mgr->mm); ... } This calls gpu_buddy_fini() which accesses mm->free_trees: drivers/gpu/buddy.c:gpu_buddy_fini() { ... for_each_free_tree(i) kfree(mm->free_trees[i]); ... } Could this dereference the uninitialized or dangling mm->free_trees pointer and cause a kernel panic during module load error handling? > + 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/[email protected]?part=1
