Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Race condition in `i915_pmu_unregister()` allowing the timer to be rearmed after cancellation, leading to a use-after-free. -- --- Patch [2]: [PATCH 2/6] drm/i915/pmu: use the expiry injecting hrtimer callback --- commit 3544b866c523f970d22c7a2ae5ea50d0bc15978f Author: Andreas Hindborg <[email protected]> drm/i915/pmu: use the expiry injecting hrtimer callback The commit converts the i915 PMU sampling timer to use the expiry injecting callback variant. This prevents a race condition between the timer callback forwarding itself and concurrent timer restarts when the GT is unparked. > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index 1c3bafda9c708..c18587e200c9f 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c [ ... ] > @@ -1157,7 +1159,7 @@ void i915_pmu_register(struct drm_i915_private *i915) > int ret = -ENOMEM; > > spin_lock_init(&pmu->lock); > - hrtimer_setup(&pmu->timer, i915_sample, CLOCK_MONOTONIC, > HRTIMER_MODE_REL); > + hrtimer_setup_ext(&pmu->timer, i915_sample, CLOCK_MONOTONIC, > HRTIMER_MODE_REL); [Severity: High] This is a pre-existing issue, but is there a race condition between the PMU unregistration and GT unparking that can lead to a use-after-free? Looking at i915_pmu_gt_unparked() in drivers/gpu/drm/i915/i915_pmu.c, it performs a lockless check on pmu->registered before acquiring the lock: drivers/gpu/drm/i915/i915_pmu.c:i915_pmu_gt_unparked() { if (!pmu->registered) return; spin_lock_irq(&pmu->lock); /* * Re-enable sampling timer when GPU goes active. */ if (pmu->unparked == 0) __i915_pmu_maybe_start_timer(pmu); ... } Concurrently, i915_pmu_unregister() sets pmu->registered to false and cancels the timer: drivers/gpu/drm/i915/i915_pmu.c:i915_pmu_unregister() { ... /* Disconnect the PMU callbacks */ pmu->registered = false; hrtimer_cancel(&pmu->timer); ... } If a user unparks the GT (e.g., by submitting a request) while the driver is being removed, could i915_pmu_gt_unparked() read pmu->registered as true, wait for the lock, and then rearm the timer after i915_pmu_unregister() has already cancelled it? When the drm_i915_private struct is later freed, would the rearmed timer fire on freed memory? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
