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

Pre-existing issues:
- [High] Scheduling an uninitialized thermal workqueue in 
kv_dpm_process_interrupt triggers a kernel panic when amdgpu.dpm=0.
- [High] Scheduling an uninitialized thermal workqueue in cik_irq_process 
triggers a kernel panic when radeon.dpm=0.
--

commit 8aa1c10862ca264ac27db63ef5b31d5ef1328d80
Author: Timur Kristóf <[email protected]>

drm/amd/pm/si: Don't schedule thermal work when queue isn't initialized

When DPM is turned off with the amdgpu.dpm=0 module parameter, the thermal
work queue isn't initialized so we shouldn't schedule any work on it.

> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c 
> b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> @@ -7659,7 +7659,7 @@ static int si_dpm_process_interrupt(struct 
> amdgpu_device *adev,
>               break;
>       }
>  
> -     if (queue_thermal)
> +     if (queue_thermal && amdgpu_dpm)
>               schedule_work(&adev->pm.dpm.thermal.work);

[Severity: High]
This is a pre-existing issue, and not a bug introduced by this patch, but
does the same uninitialized workqueue crash also happen in kv_dpm.c?

Looking at kv_dpm_process_interrupt() in
drivers/gpu/drm/amd/pm/legacy-dpm/kv_dpm.c, it still has the unguarded
schedule_work call:

    if (queue_thermal)
        schedule_work(&adev->pm.dpm.thermal.work);

And similar to the issue you fixed in si_dpm.c, kv_dpm_sw_init() skips the
INIT_WORK call when DPM is disabled:

    if (amdgpu_dpm == 0)
        return 0;

    INIT_WORK(&adev->pm.dpm.thermal.work, amdgpu_dpm_thermal_work_handler);

If a thermal interrupt fires on KV/CI hardware while booted with
amdgpu.dpm=0, wouldn't this queue a zero-initialized work struct and attempt
to execute a NULL function pointer? Should the same amdgpu_dpm guard be
added to kv_dpm_process_interrupt()?


[Severity: High]
This is also a pre-existing issue, but it looks like the radeon driver
might share the exact same vulnerability in cik.c.

In drivers/gpu/drm/radeon/radeon_pm.c, radeon_pm_init() skips INIT_WORK
when DPM is disabled, falling back to the old method:

    if (rdev->pm.pm_method == PM_METHOD_DPM)
        return radeon_pm_init_dpm(rdev);
    else
        return radeon_pm_init_old(rdev);

But cik_irq_process() in drivers/gpu/drm/radeon/cik.c unconditionally
schedules the work if a thermal interrupt arrives:

    if (queue_thermal)
        schedule_work(&rdev->pm.dpm.thermal.work);

If a user boots with radeon.dpm=0 on CIK hardware and a thermal interrupt
triggers, could this result in the same workqueue panic? Does
cik_irq_process() need a rdev->pm.dpm_enabled guard added to its
schedule_work call?

>  
>       return 0;
>  }

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

Reply via email to