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 <[email protected]> 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/[email protected]?part=6
