Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] xe: TOCTOU race in xe_device_declare_wedged() overwrites recovery method --
commit 8a75aea67f604c5a996849e643c64617844ddad3 Author: Arvind Yadav <[email protected]> Subject: drm/xe: Send wedged notification from a worker This commit moves the wedged event to a sleepable device worker to allow isolation work to complete before userspace is notified. It updates the wedge bookkeeping to rescan the GT state on later declarations and tracks the last reported method to avoid lost updates. > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c > --- a/drivers/gpu/drm/xe/xe_device.c > +++ b/drivers/gpu/drm/xe/xe_device.c [ ... ] > @@ -1435,14 +1482,22 @@ void xe_device_declare_wedged(struct xe_device *xe) > */ > if (xe->wedged.mode == XE_WEDGED_MODE_UPON_ANY_HANG_NO_RESET) > xe_device_set_wedged_method(xe, > DRM_WEDGE_RECOVERY_NONE); > } > + > + /* Re-scan GT submission state on every declaration. */ > + for_each_gt(gt, xe, id) > + xe_gt_declare_wedged(gt); > + > + /* If no wedge recovery method is set, use default */ > + if (!READ_ONCE(xe->wedged.method)) > + xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_REBIND | > + DRM_WEDGE_RECOVERY_BUS_RESET); [Severity: Medium] Is there a Time-of-Check to Time-of-Use (TOCTOU) race condition here in xe_device_declare_wedged()? If multiple threads invoke xe_device_declare_wedged() concurrently (for example, during simultaneous GT hangs), could Thread A evaluate the !READ_ONCE(xe->wedged.method) condition as true, get preempted, and then allow Thread B to successfully set a specific method like DRM_WEDGE_RECOVERY_NONE? If Thread A then resumes, it seems it would unconditionally call xe_device_set_wedged_method() with the default REBIND | BUS_RESET flags, silently overwriting Thread B's legitimately set method. Does this check-and-set sequence need atomic protection, such as cmpxchg, to prevent concurrent threads from overwriting a newly set recovery method with the default fallback? > + > + if (!atomic_read(&xe->wedged.stopping) && > + (first || > + READ_ONCE(xe->wedged.method) != > + READ_ONCE(xe->wedged.reported_method))) > + queue_work(xe->unordered_wq, &xe->wedged.work); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
