Re: [PATCH v3 6/6] drm/amdgpu/ras: Cancel delayed work before ACA teardown in amdgpu_ras_fini

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Race condition and Locking Violation / Use-After-Free in `amdgpu_ras_recovery_fini` where `page_rsv_lock` is destroyed before `page_retirement_dwork` is reliably canceled.
--

--- Patch [6]: [PATCH v3 6/6] drm/amdgpu/ras: Cancel delayed work before ACA teardown in amdgpu_ras_fini ---
commit dd2776768892778a29776f233fd342f5f59be385
Author: Sreeraj S Kurup <sreekuttan2156239@gmail.com>

drm/amdgpu/ras: Cancel delayed work before ACA teardown in amdgpu_ras_fini
    
In amdgpu_ras_fini(), cancel_delayed_work_sync() is currently called
after ACA subsystems and locks are destroyed. This patch moves it to the
beginning to ensure all pending work is flushed and stopped before resource
deallocation starts, preventing a use-after-free and locking violation.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 764cd49504083..118716ad04379 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -4751,6 +4751,7 @@ int amdgpu_ras_fini(struct amdgpu_device *adev)
>  	if (!adev->ras_enabled || !con)
>  		return 0;
>  
> +	cancel_delayed_work_sync(&con->ras_counte_delay_work);
>  	amdgpu_ras_critical_region_fini(adev);
>  	mutex_destroy(&con->critical_region_lock);

[Severity: High]
This is a pre-existing issue, but while looking at this fix for early delayed
work cancellation, I noticed a very similar pattern in
amdgpu_ras_recovery_fini():

drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:amdgpu_ras_recovery_fini() {
    ...
	do {
		flush_delayed_work(&con->page_retirement_dwork);
		ret = amdgpu_ras_schedule_retirement_dwork(con, 0);
	} while (ret && max_flush_timeout--);

	...

	mutex_destroy(&con->page_rsv_lock);

	cancel_work_sync(&con->recovery_work);

	cancel_delayed_work_sync(&con->page_retirement_dwork);
    ...
}

If the loop exhausts its max_flush_timeout, it exits while the delayed work
is still pending. The code then proceeds to call
mutex_destroy(&con->page_rsv_lock) before calling
cancel_delayed_work_sync(&con->page_retirement_dwork).

If the delayed work (page_retirement_dwork) runs after the page_rsv_lock
mutex is destroyed, could it attempt to acquire the destroyed mutex in
amdgpu_ras_reserve_page(), leading to a locking violation and
use-after-free?

Should cancel_delayed_work_sync(&con->page_retirement_dwork) be moved above
mutex_destroy(&con->page_rsv_lock) here as well?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809051200.3276-1-sreekuttan2156239@gmail.com?part=6