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

Pre-existing issues:
- [High] If late initialization steps in `amdgpu_pci_probe()` fail, the error 
path skips hardware teardown and fails to unwind earlier registrations, leading 
to resource leaks, zombie devices, and potential use-after-free.
--

--- Patch [1]: [PATCH] drm/amdgpu: fix autosuspend cleanup during removal ---
commit c007137d62506190886d6bf0f4180137897883b4
Author: Guangshuo Li <[email protected]>

drm/amdgpu: fix autosuspend cleanup during removal

This commit adds a missing pm_runtime_dont_use_autosuspend() call to the
remove path. This ensures that the usage count is properly balanced and
dropped, preventing issues where the runtime PM core keeps the device from
suspending when the autosuspend delay is set to a negative value.

> amdgpu_pci_probe() calls pm_runtime_use_autosuspend(), but
> amdgpu_pci_remove() does not call the matching
> pm_runtime_dont_use_autosuspend().

[Severity: High]
This is a pre-existing issue, but while looking at amdgpu_pci_probe(), if late
initialization steps fail, the error path skips hardware teardown:

drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:amdgpu_pci_probe() {
    ...
        } else if (ret) {
                goto err_pci;
        }

        ret = amdgpu_xcp_dev_register(adev, ent);
        if (ret)
                goto err_pci;

        ret = amdgpu_amdkfd_drm_client_create(adev);
        if (ret)
                goto err_pci;
    ...
err_pci:
        pci_disable_device(pdev);
        return ret;
}

Can this leave a zombie DRM device exposed to userspace?

Because adev is allocated via devm_drm_dev_alloc(), devres handles cleanup on
probe failure by calling drm_dev_put(). The release callback
(amdgpu_driver_release_kms) only calls amdgpu_device_fini_sw(), which frees
memory and software state.

Since the error path does not call amdgpu_driver_unload_kms(),
amdgpu_device_fini_hw() is never executed. Hardware clocks, interrupts, and
internal engines remain active while the backing memory is freed, which could
lead to memory corruption.

Furthermore, if drm_dev_register() succeeds but a subsequent step (like
amdgpu_xcp_dev_register()) fails, the error path fails to call
drm_dev_unregister(). Could this leave dangling sysfs nodes and device files
that might trigger a use-after-free when userspace attempts to open them?

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 0ab380ca7e645..5c33c19fd9bc5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2557,6 +2557,7 @@ amdgpu_pci_remove(struct pci_dev *pdev)
>       if (adev->pm.rpm_mode != AMDGPU_RUNPM_NONE) {
>               pm_runtime_get_sync(dev->dev);
>               pm_runtime_forbid(dev->dev);
> +             pm_runtime_dont_use_autosuspend(dev->dev);
>       }
>  
>       amdgpu_driver_unload_kms(dev);

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to