[PATCH] drm/i915/gt: Fix request use-after-free in heartbeat()

[PATCH] drm/i915/gt: Fix request use-after-free in heartbeat()

[Shuangpeng Bai]

heartbeat() detaches engine->heartbeat.systole with xchg() before
checking whether the request has completed. If the request is complete,
dropping the detached systole reference may free it. Concurrently,
idle_pulse() can observe the empty slot and publish a newer request.

The worker then sees a non-NULL engine->heartbeat.systole, but continues
to dereference its stale local rq. The newer request can therefore make
the shared-slot check succeed after the old request has been freed,
causing use-after-free accesses to emitted_jiffies, submit, and sched.

If the detached request is incomplete, heartbeat() can also overwrite a
newer request published by idle_pulse() when it unconditionally restores
the old pointer.

Use cmpxchg() both when idle_pulse() publishes a request and when
heartbeat() restores the detached request. Drop the detached reference
when restoration loses the race, and reload rq from the shared slot
before dereferencing it.

A KASAN-enabled i915 mock selftest reproduces the use-after-free before
this change and completes without a report after the fix.

Fixes: bf9bd6a5128a ("drm/i915/gt: Track the most recent pulse for the heartbeat")
Cc: stable@vger.kernel.org # v5.11+
Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
---
 .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 22 ++++++++++++++-----
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 6424ecce8bcb..fc6c53c93cf5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -81,10 +81,18 @@ heartbeat_create(struct intel_context *ce, gfp_t gfp)
 
 static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
 {
+	struct i915_request *systole;
+
 	engine->wakeref_serial = READ_ONCE(engine->serial) + 1;
 	i915_request_add_active_barriers(rq);
-	if (!engine->heartbeat.systole && intel_engine_has_heartbeat(engine))
-		engine->heartbeat.systole = i915_request_get(rq);
+	if (READ_ONCE(engine->heartbeat.systole) ||
+	    !intel_engine_has_heartbeat(engine))
+		return;
+
+	systole = i915_request_get(rq);
+	/* The worker may have restored its detached systole in the meantime. */
+	if (cmpxchg(&engine->heartbeat.systole, NULL, systole))
+		i915_request_put(systole);
 }
 
 static void heartbeat_commit(struct i915_request *rq,
@@ -152,9 +160,11 @@ static void heartbeat(struct work_struct *wrk)
 	if (rq) {
 		if (i915_request_completed(rq))
 			i915_request_put(rq);
-		else
-			engine->heartbeat.systole = rq;
+		/* Keep a newer pulse that raced with the detached systole. */
+		else if (cmpxchg(&engine->heartbeat.systole, NULL, rq))
+			i915_request_put(rq);
 	}
+	rq = READ_ONCE(engine->heartbeat.systole);
 
 	if (!intel_engine_pm_get_if_awake(engine))
 		return;
@@ -163,11 +173,11 @@ static void heartbeat(struct work_struct *wrk)
 		goto out;
 
 	if (i915_sched_engine_disabled(engine->sched_engine)) {
-		reset_engine(engine, engine->heartbeat.systole);
+		reset_engine(engine, rq);
 		goto out;
 	}
 
-	if (engine->heartbeat.systole) {
+	if (rq) {
 		long delay = READ_ONCE(engine->props.heartbeat_interval_ms);
 
 		/* Safeguard against too-fast worker invocations */

base-commit: a59f57e2aa127c5354168d2ec4bac920df1be4f4
-- 
2.43.0

✗ LGCI.VerificationFailed: failure for drm/i915/gt: Fix request use-after-free in heartbeat()

[Patchwork]

== Series Details ==

Series: drm/i915/gt: Fix request use-after-free in heartbeat()
URL   : https://patchwork.freedesktop.org/series/171942/
State : failure

== Summary ==

Series author address 'shuangpeng.kernel@gmail.com' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://gfx-ci.igk.intel.com:8080/job/CI_PW_kernel/183928/ (on built-in)

Re: [PATCH] drm/i915/gt: Fix request use-after-free in heartbeat()

[Krzysztof Karas]

Hi Shuangpeng,

On 2026-08-08 at 14:43:02 -0400, Shuangpeng Bai wrote:
> heartbeat() detaches engine->heartbeat.systole with xchg() before
> checking whether the request has completed. If the request is complete,
> dropping the detached systole reference may free it. Concurrently,
> idle_pulse() can observe the empty slot and publish a newer request.
> 
> The worker then sees a non-NULL engine->heartbeat.systole, but continues
> to dereference its stale local rq. The newer request can therefore make
> the shared-slot check succeed after the old request has been freed,
> causing use-after-free accesses to emitted_jiffies, submit, and sched.
> 
> If the detached request is incomplete, heartbeat() can also overwrite a
> newer request published by idle_pulse() when it unconditionally restores
> the old pointer.
> 
> Use cmpxchg() both when idle_pulse() publishes a request and when
> heartbeat() restores the detached request. Drop the detached reference
> when restoration loses the race, and reload rq from the shared slot
> before dereferencing it.
> 
> A KASAN-enabled i915 mock selftest reproduces the use-after-free before
> this change and completes without a report after the fix.
What is this mock selftest? Can I see it somewhere and confirm
if it works myself? A link to a public issue would be cool too,
something that gives more context as to how this was discovered.

> 
> Fixes: bf9bd6a5128a ("drm/i915/gt: Track the most recent pulse for the heartbeat")
> Cc: stable@vger.kernel.org # v5.11+
> Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
> ---
>  .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 22 ++++++++++++++-----
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> index 6424ecce8bcb..fc6c53c93cf5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> @@ -81,10 +81,18 @@ heartbeat_create(struct intel_context *ce, gfp_t gfp)
>  
>  static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
>  {
> +	struct i915_request *systole;
> +
>  	engine->wakeref_serial = READ_ONCE(engine->serial) + 1;
>  	i915_request_add_active_barriers(rq);
> -	if (!engine->heartbeat.systole && intel_engine_has_heartbeat(engine))
> -		engine->heartbeat.systole = i915_request_get(rq);
> +	if (READ_ONCE(engine->heartbeat.systole) ||
> +	    !intel_engine_has_heartbeat(engine))
> +		return;
> +
> +	systole = i915_request_get(rq);
> +	/* The worker may have restored its detached systole in the meantime. */
> +	if (cmpxchg(&engine->heartbeat.systole, NULL, systole))
> +		i915_request_put(systole);
>  }
>  
>  static void heartbeat_commit(struct i915_request *rq,
> @@ -152,9 +160,11 @@ static void heartbeat(struct work_struct *wrk)
>  	if (rq) {
>  		if (i915_request_completed(rq))
>  			i915_request_put(rq);
> -		else
> -			engine->heartbeat.systole = rq;
> +		/* Keep a newer pulse that raced with the detached systole. */
> +		else if (cmpxchg(&engine->heartbeat.systole, NULL, rq))
> +			i915_request_put(rq);
>  	}
> +	rq = READ_ONCE(engine->heartbeat.systole);
>  
>  	if (!intel_engine_pm_get_if_awake(engine))
>  		return;
> @@ -163,11 +173,11 @@ static void heartbeat(struct work_struct *wrk)
>  		goto out;
>  
>  	if (i915_sched_engine_disabled(engine->sched_engine)) {
> -		reset_engine(engine, engine->heartbeat.systole);
> +		reset_engine(engine, rq);
>  		goto out;
>  	}
>  
> -	if (engine->heartbeat.systole) {
> +	if (rq) {
>  		long delay = READ_ONCE(engine->props.heartbeat_interval_ms);
>  
>  		/* Safeguard against too-fast worker invocations */
> 
> base-commit: a59f57e2aa127c5354168d2ec4bac920df1be4f4
> -- 
> 2.43.0
> 

The fix looks sane at first glance to me, but I'd like to see
how it was spotted before diving in further.

-- 
Best Regards,
Krzysztof

Re: [PATCH] drm/i915/gt: Fix request use-after-free in heartbeat()

[Shuangpeng]

> On Aug 11, 2026, at 06:50, Krzysztof Karas <krzysztof.karas@intel.com> wrote:
> 
> Hi Shuangpeng,
> 
> On 2026-08-08 at 14:43:02 -0400, Shuangpeng Bai wrote:
>> heartbeat() detaches engine->heartbeat.systole with xchg() before
>> checking whether the request has completed. If the request is complete,
>> dropping the detached systole reference may free it. Concurrently,
>> idle_pulse() can observe the empty slot and publish a newer request.
>> 
>> The worker then sees a non-NULL engine->heartbeat.systole, but continues
>> to dereference its stale local rq. The newer request can therefore make
>> the shared-slot check succeed after the old request has been freed,
>> causing use-after-free accesses to emitted_jiffies, submit, and sched.
>> 
>> If the detached request is incomplete, heartbeat() can also overwrite a
>> newer request published by idle_pulse() when it unconditionally restores
>> the old pointer.
>> 
>> Use cmpxchg() both when idle_pulse() publishes a request and when
>> heartbeat() restores the detached request. Drop the detached reference
>> when restoration loses the race, and reload rq from the shared slot
>> before dereferencing it.
>> 
>> A KASAN-enabled i915 mock selftest reproduces the use-after-free before
>> this change and completes without a report after the fix.
> What is this mock selftest? Can I see it somewhere and confirm
> if it works myself? A link to a public issue would be cool too,
> something that gives more context as to how this was discovered.
> 

Hi Krzysztof,

I've put the reproducer here:

https://gist.github.com/shuangpengbai/1410664bc909e8c834311576d6242103

Tested on commit a59f57e2aa127c5354168d2ec4bac920df1be4f4 (2026-08-07).

It uses i915's mock selftest path, so no Intel GPU hardware is required. 
Please apply 1_repro.patch, build with 2_config, and boot with:
i915.mock_selftests=1 i915.igt__36__mock_heartbeat_uaf=1

Expected result:
BUG: KASAN: slab-use-after-free in heartbeat

Best,
Shuangpeng

>> 
>> Fixes: bf9bd6a5128a ("drm/i915/gt: Track the most recent pulse for the heartbeat")
>> Cc: stable@vger.kernel.org # v5.11+
>> Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
>> ---
>> .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 22 ++++++++++++++-----
>> 1 file changed, 16 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
>> index 6424ecce8bcb..fc6c53c93cf5 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
>> @@ -81,10 +81,18 @@ heartbeat_create(struct intel_context *ce, gfp_t gfp)
>> 
>> static void idle_pulse(struct intel_engine_cs *engine, struct i915_request *rq)
>> {
>> + struct i915_request *systole;
>> +
>> engine->wakeref_serial = READ_ONCE(engine->serial) + 1;
>> i915_request_add_active_barriers(rq);
>> - if (!engine->heartbeat.systole && intel_engine_has_heartbeat(engine))
>> - engine->heartbeat.systole = i915_request_get(rq);
>> + if (READ_ONCE(engine->heartbeat.systole) ||
>> +     !intel_engine_has_heartbeat(engine))
>> + return;
>> +
>> + systole = i915_request_get(rq);
>> + /* The worker may have restored its detached systole in the meantime. */
>> + if (cmpxchg(&engine->heartbeat.systole, NULL, systole))
>> + i915_request_put(systole);
>> }
>> 
>> static void heartbeat_commit(struct i915_request *rq,
>> @@ -152,9 +160,11 @@ static void heartbeat(struct work_struct *wrk)
>> if (rq) {
>> if (i915_request_completed(rq))
>> i915_request_put(rq);
>> - else
>> - engine->heartbeat.systole = rq;
>> + /* Keep a newer pulse that raced with the detached systole. */
>> + else if (cmpxchg(&engine->heartbeat.systole, NULL, rq))
>> + i915_request_put(rq);
>> }
>> + rq = READ_ONCE(engine->heartbeat.systole);
>> 
>> if (!intel_engine_pm_get_if_awake(engine))
>> return;
>> @@ -163,11 +173,11 @@ static void heartbeat(struct work_struct *wrk)
>> goto out;
>> 
>> if (i915_sched_engine_disabled(engine->sched_engine)) {
>> - reset_engine(engine, engine->heartbeat.systole);
>> + reset_engine(engine, rq);
>> goto out;
>> }
>> 
>> - if (engine->heartbeat.systole) {
>> + if (rq) {
>> long delay = READ_ONCE(engine->props.heartbeat_interval_ms);
>> 
>> /* Safeguard against too-fast worker invocations */
>> 
>> base-commit: a59f57e2aa127c5354168d2ec4bac920df1be4f4
>> -- 
>> 2.43.0
>> 
> 
> The fix looks sane at first glance to me, but I'd like to see
> how it was spotted before diving in further.
> 
> -- 
> Best Regards,
> Krzysztof

Re: [PATCH] drm/i915/gt: Fix request use-after-free in heartbeat()

[Andi Shyti]

Hi Shuangpeng,

On Sat, Aug 08, 2026 at 02:43:02PM -0400, Shuangpeng Bai wrote:
> heartbeat() detaches engine->heartbeat.systole with xchg() before
> checking whether the request has completed. If the request is complete,
> dropping the detached systole reference may free it. Concurrently,
> idle_pulse() can observe the empty slot and publish a newer request.
> 
> The worker then sees a non-NULL engine->heartbeat.systole, but continues
> to dereference its stale local rq. The newer request can therefore make
> the shared-slot check succeed after the old request has been freed,
> causing use-after-free accesses to emitted_jiffies, submit, and sched.
> 
> If the detached request is incomplete, heartbeat() can also overwrite a
> newer request published by idle_pulse() when it unconditionally restores
> the old pointer.
> 
> Use cmpxchg() both when idle_pulse() publishes a request and when
> heartbeat() restores the detached request. Drop the detached reference
> when restoration loses the race, and reload rq from the shared slot
> before dereferencing it.
> 
> A KASAN-enabled i915 mock selftest reproduces the use-after-free before
> this change and completes without a report after the fix.
> 
> Fixes: bf9bd6a5128a ("drm/i915/gt: Track the most recent pulse for the heartbeat")
> Cc: stable@vger.kernel.org # v5.11+

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Your patch looks correct, although I don't think the Fixes tag
is.  I believe the actual fix is:

Fixes: 4c71fd099513 ("drm/i915/gt: fix refcount underflow in intel_engine_park_heartbeat")
Cc: <stable@vger.kernel.org> # v7.0+

That commit introduced the early xchg() which hides the old
request from idle_pulse() and replaces it with a new one.

I'm going to resend the patch so that it can be picked up by our
CI.

Thanks,
Andi

Re: [PATCH] drm/i915/gt: Fix request use-after-free in heartbeat()

[Shuangpeng]

Hi Andi,


> On Aug 30, 2026, at 18:44, Andi Shyti <andi.shyti@kernel.org> wrote:
> 
> Hi Shuangpeng,
> 
> On Sat, Aug 08, 2026 at 02:43:02PM -0400, Shuangpeng Bai wrote:
>> heartbeat() detaches engine->heartbeat.systole with xchg() before
>> checking whether the request has completed. If the request is complete,
>> dropping the detached systole reference may free it. Concurrently,
>> idle_pulse() can observe the empty slot and publish a newer request.
>> 
>> The worker then sees a non-NULL engine->heartbeat.systole, but continues
>> to dereference its stale local rq. The newer request can therefore make
>> the shared-slot check succeed after the old request has been freed,
>> causing use-after-free accesses to emitted_jiffies, submit, and sched.
>> 
>> If the detached request is incomplete, heartbeat() can also overwrite a
>> newer request published by idle_pulse() when it unconditionally restores
>> the old pointer.
>> 
>> Use cmpxchg() both when idle_pulse() publishes a request and when
>> heartbeat() restores the detached request. Drop the detached reference
>> when restoration loses the race, and reload rq from the shared slot
>> before dereferencing it.
>> 
>> A KASAN-enabled i915 mock selftest reproduces the use-after-free before
>> this change and completes without a report after the fix.
>> 
>> Fixes: bf9bd6a5128a ("drm/i915/gt: Track the most recent pulse for the heartbeat")
>> Cc: stable@vger.kernel.org # v5.11+
> 
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> 

> Your patch looks correct, although I don't think the Fixes tag
> is.  I believe the actual fix is:
> 
> Fixes: 4c71fd099513 ("drm/i915/gt: fix refcount underflow in intel_engine_park_heartbeat")
> Cc: <stable@vger.kernel.org> # v7.0+
> 
> That commit introduced the early xchg() which hides the old
> request from idle_pulse() and replaces it with a new one.
> 

Thanks for the review and for correcting the Fixes tag. 

> I'm going to resend the patch so that it can be picked up by our
> CI.
> 

Thanks for resending it.

> Thanks,
> Andi


Best,
Shuangpeng

Re: [PATCH] drm/i915/gt: Fix request use-after-free in heartbeat()

[Joonas Lahtinen]

Quoting Shuangpeng (2026-08-31 03:13:28)
> Hi Andi,
> 
> 
> > On Aug 30, 2026, at 18:44, Andi Shyti <andi.shyti@kernel.org> wrote:
> > 
> > Hi Shuangpeng,
> > 
> > On Sat, Aug 08, 2026 at 02:43:02PM -0400, Shuangpeng Bai wrote:
> >> heartbeat() detaches engine->heartbeat.systole with xchg() before
> >> checking whether the request has completed. If the request is complete,
> >> dropping the detached systole reference may free it. Concurrently,
> >> idle_pulse() can observe the empty slot and publish a newer request.
> >> 
> >> The worker then sees a non-NULL engine->heartbeat.systole, but continues
> >> to dereference its stale local rq. The newer request can therefore make
> >> the shared-slot check succeed after the old request has been freed,
> >> causing use-after-free accesses to emitted_jiffies, submit, and sched.
> >> 
> >> If the detached request is incomplete, heartbeat() can also overwrite a
> >> newer request published by idle_pulse() when it unconditionally restores
> >> the old pointer.
> >> 
> >> Use cmpxchg() both when idle_pulse() publishes a request and when
> >> heartbeat() restores the detached request. Drop the detached reference
> >> when restoration loses the race, and reload rq from the shared slot
> >> before dereferencing it.
> >> 
> >> A KASAN-enabled i915 mock selftest reproduces the use-after-free before
> >> this change and completes without a report after the fix.

The test should be properly submitted and reviewed first before we even
consider merging this, especially with such bad commit message. The commit
message doesn't mention a word about real world usage scenario that
could lead to this.

There are probably hundreds of ways to cause kernel to crash by crafting
selftests, and if they don't map to any real real hardware usage behavior,
that's just noise.

Regards, Joonas

Re: [PATCH] drm/i915/gt: Fix request use-after-free in heartbeat()

[Jani Nikula]

On Mon, 31 Aug 2026, Andi Shyti <andi.shyti@kernel.org> wrote:
> I'm going to resend the patch so that it can be picked up by our CI.

Nowadays, there's no need to do that. It's possible to request retest in
patchwork.

BR,
Jani.

-- 
Jani Nikula, Intel