[PATCH] s390/pai: Handle multiple PMU stop callback invocations

[PATCH] s390/pai: Handle multiple PMU stop callback invocations

[Thomas Richter]

Handle the following scenario:
The kernel protects itself against a very high sampling load and
throttles the sampling using:

  perf_event_throttle() --> PMU->stop()

Shortly later the scheduler may terminate the task and removes it from the
CPU. It again calls

  PMU->stop()

which results in two invocations of PMU->stop() called back to back.
Protect against this and check the PERF_HES_STOPPED bit on function
entry.  If it is already set return.
Clear bit PERF_HES_STOPPED in PMU->start().

 # Cc: stable@vger.kernel.org # v6.19+
Fixes: ac03223f0723 ("s390/pai_crypto: Add common pai_stop() function")
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
---
 arch/s390/kernel/perf_pai.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
index 1e5a1f1cc68a..f6790f658f21 100644
--- a/arch/s390/kernel/perf_pai.c
+++ b/arch/s390/kernel/perf_pai.c
@@ -484,6 +484,7 @@ static void pai_start(struct perf_event *event, int flags,
 			cpump->event = event;
 		}
 	}
+	event->hw.state &= ~PERF_HES_STOPPED;
 }
 
 static void paicrypt_start(struct perf_event *event, int flags)
@@ -530,6 +531,13 @@ static void pai_stop(struct perf_event *event, int flags)
 	struct pai_mapptr *mp = this_cpu_ptr(pai_root[idx].mapptr);
 	struct pai_map *cpump = mp->mapptr;
 
+	/* Cope with multiple invocations:
+	 *   1. perf_event_throttle() --> PMU->stop()
+	 *   2. task schedules out --> PMU->stop()
+	 * Check for event already stopped.
+	 */
+	if (event->hw.state & PERF_HES_STOPPED)
+		return;
 	if (!event->attr.sample_period) {	/* Counting */
 		pai_pmu[idx].pmu->read(event);
 	} else {				/* Sampling */
-- 
2.55.0

Re: [PATCH] s390/pai: Handle multiple PMU stop callback invocations

[sashiko-bot]

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

Pre-existing issues:
- [High] Unsafe iteration over `cpump->syswide_list` in `pai_have_samples()` leads to a kernel crash (LIST_POISON dereference) if an event overflows and is synchronously throttled during the loop.
--

--- Patch [1]: [PATCH] s390/pai: Handle multiple PMU stop callback invocations ---
commit 8c88016c23029bda1d214155e79ad2671b8e3eb5
Author: Thomas Richter <tmricht@linux.ibm.com>

s390/pai: Handle multiple PMU stop callback invocations

The kernel protects itself against high sampling load by throttling it via
perf_event_throttle(), calling PMU->stop(). Shortly after, the task may be
scheduled out, resulting in a second PMU->stop() invocation. This patch adds
a check for the PERF_HES_STOPPED bit to prevent duplicate handling.

Cc: stable@vger.kernel.org # v6.19+
Fixes: ac03223f0723 ("s390/pai_crypto: Add common pai_stop() function")

> diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
> index cdb8006220ca0..a377f04296563 100644
> --- a/arch/s390/kernel/perf_pai.c
> +++ b/arch/s390/kernel/perf_pai.c

[ ... ]

> @@ -510,6 +511,13 @@ static void pai_stop(struct perf_event *event, int flags)
>  	struct pai_mapptr *mp = this_cpu_ptr(pai_root[idx].mapptr);
>  	struct pai_map *cpump = mp->mapptr;
>  
> +	/* Cope with multiple invocations:
> +	 *   1. perf_event_throttle() --> PMU->stop()
> +	 *   2. task schedules out --> PMU->stop()
> +	 * Check for event already stopped.
> +	 */
> +	if (event->hw.state & PERF_HES_STOPPED)
> +		return;

[Severity: High]
This is a pre-existing issue, but can this synchronous throttle event
corrupt the list iteration during sample processing?

Looking at the iteration in pai_have_samples():

arch/s390/kernel/perf_pai.c:pai_have_samples() {
    ...
    list_for_each_entry(event, &cpump->syswide_list, hw.tp_list)
        pai_have_sample(event, cpump);
    ...
}

If an event reaches its sample limit, perf_event_overflow() triggers
synchronously, invoking perf_event_throttle() which then calls pai_stop().

When pai_stop() is called for a system-wide sampling event, it executes:

    list_del(PAI_SWLIST(event));

Will deleting the event currently being iterated leave the list pointer
at a poison value, causing a panic when the loop in pai_have_samples()
continues? Does pai_have_samples() need to use list_for_each_entry_safe()
instead?

>  	if (!event->attr.sample_period) {	/* Counting */
>  		pai_pmu[idx].pmu->read(event);
>  	} else {				/* Sampling */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813070815.2692398-1-tmricht@linux.ibm.com?part=1

Re: [PATCH] s390/pai: Handle multiple PMU stop callback invocations

[Thomas Richter]

On 8/13/26 09:21, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Unsafe iteration over `cpump->syswide_list` in `pai_have_samples()` leads to a kernel crash (LIST_POISON dereference) if an event overflows and is synchronously throttled during the loop.

This can not happen. The syswide_list is anchored by per-CPU pai_map::syswide_list and tracks
all events which are bound to that particular CPU. The event was installed only on that particular
CPU and does not move with the process/task. Each CPU has its own list anchored by
pai_map::syswide_list.

The call chains are either

 paicrypt_sched_task() or paiext_sched_task
 +--> pai_have_samples() iterates over syswide_list and calls
      +--> pai_have_sample
      
or    
 pai_stop()
 +--> list_del() to remove the event from that CPU.

 Since a CPU can only execute one given code path at any one time:
 - either schedule out
 - or stop the event
 both can not happen at the same time. PAI PMU does not install an interrupt handler,
 there is no supoprt for interrupts.

 I think this is safe.


> --
> 
> --- Patch [1]: [PATCH] s390/pai: Handle multiple PMU stop callback invocations ---
> commit 8c88016c23029bda1d214155e79ad2671b8e3eb5
> Author: Thomas Richter <tmricht@linux.ibm.com>
> 
> s390/pai: Handle multiple PMU stop callback invocations
> 
> The kernel protects itself against high sampling load by throttling it via
> perf_event_throttle(), calling PMU->stop(). Shortly after, the task may be
> scheduled out, resulting in a second PMU->stop() invocation. This patch adds
> a check for the PERF_HES_STOPPED bit to prevent duplicate handling.
> 
> Cc: stable@vger.kernel.org # v6.19+
> Fixes: ac03223f0723 ("s390/pai_crypto: Add common pai_stop() function")
> 
>> diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
>> index cdb8006220ca0..a377f04296563 100644
>> --- a/arch/s390/kernel/perf_pai.c
>> +++ b/arch/s390/kernel/perf_pai.c
> 
> [ ... ]
> 
>> @@ -510,6 +511,13 @@ static void pai_stop(struct perf_event *event, int flags)
>>  	struct pai_mapptr *mp = this_cpu_ptr(pai_root[idx].mapptr);
>>  	struct pai_map *cpump = mp->mapptr;
>>  
>> +	/* Cope with multiple invocations:
>> +	 *   1. perf_event_throttle() --> PMU->stop()
>> +	 *   2. task schedules out --> PMU->stop()
>> +	 * Check for event already stopped.
>> +	 */
>> +	if (event->hw.state & PERF_HES_STOPPED)
>> +		return;
> 
> [Severity: High]
> This is a pre-existing issue, but can this synchronous throttle event
> corrupt the list iteration during sample processing?
> 
> Looking at the iteration in pai_have_samples():
> 
> arch/s390/kernel/perf_pai.c:pai_have_samples() {
>     ...
>     list_for_each_entry(event, &cpump->syswide_list, hw.tp_list)
>         pai_have_sample(event, cpump);
>     ...
> }
> 
> If an event reaches its sample limit, perf_event_overflow() triggers
> synchronously, invoking perf_event_throttle() which then calls pai_stop().
> 
> When pai_stop() is called for a system-wide sampling event, it executes:
> 
>     list_del(PAI_SWLIST(event));
> 
> Will deleting the event currently being iterated leave the list pointer
> at a poison value, causing a panic when the loop in pai_have_samples()
> continues? Does pai_have_samples() need to use list_for_each_entry_safe()
> instead?
> 
>>  	if (!event->attr.sample_period) {	/* Counting */
>>  		pai_pmu[idx].pmu->read(event);
>>  	} else {				/* Sampling */
> 


-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH] s390/pai: Handle multiple PMU stop callback invocations

[Thomas Richter]

On 8/13/26 09:21, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Unsafe iteration over `cpump->syswide_list` in `pai_have_samples()` leads to a kernel crash (LIST_POISON dereference) if an event overflows and is synchronously throttled during the loop.
> --

This can not happen. The syswide_list is anchored by per-CPU pai_map::syswide_list and tracks
all events which are bound to that particular CPU. The event was installed only on that particular
CPU and does not move with the process/task. Each CPU has its own list anchored by
 pai_map::syswide_list.

The call chains are either

 paicrypt_sched_task() or paiext_sched_task
 +--> pai_have_samples() iterates over syswide_list and calls
      +--> pai_have_sample

or
 pai_stop()
 +--> list_del() to remove the event from that CPU.

 Since a CPU can only execute one given code path at any one time:
 - either schedule out
 - or stop the event
 both can not happen at the same time. PAI PMU does not install an interrupt handler,
 there is no supoprt for interrupts.

 I think this is safe or am I mistaken?


...

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

[PATCH] s390/pai: Handle multiple PMU stop callback invocations

[Thomas Richter]

Handle the following scenario:
The kernel protects itself against a very high sampling load and
throttles the sampling using:

  perf_event_throttle() --> PMU->stop()

Shortly later the scheduler may terminate the task and removes it from the
CPU. It again calls

  PMU->stop()

which results in two invocations of PMU->stop() called back to back.
Protect against this and check the PERF_HES_STOPPED bit on function
entry.  If it is already set return.
Clear bit PERF_HES_STOPPED in PMU->start().

Cc: stable@vger.kernel.org # v6.19+
Fixes: 9f66572f2889 ("s390/pai_crypto: Enable per-task and system-wide sampling event")
Fixes: 582cc1b28e8c ("s390/pai_ext: Enable per-task and system-wide sampling event")

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Sumanth Korikkar <sumanthk@linux.ibm.com>
---
 arch/s390/kernel/perf_pai.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
index 1e5a1f1cc68a..f6790f658f21 100644
--- a/arch/s390/kernel/perf_pai.c
+++ b/arch/s390/kernel/perf_pai.c
@@ -484,6 +484,7 @@ static void pai_start(struct perf_event *event, int flags,
 			cpump->event = event;
 		}
 	}
+	event->hw.state &= ~PERF_HES_STOPPED;
 }
 
 static void paicrypt_start(struct perf_event *event, int flags)
@@ -530,6 +531,13 @@ static void pai_stop(struct perf_event *event, int flags)
 	struct pai_mapptr *mp = this_cpu_ptr(pai_root[idx].mapptr);
 	struct pai_map *cpump = mp->mapptr;
 
+	/* Cope with multiple invocations:
+	 *   1. perf_event_throttle() --> PMU->stop()
+	 *   2. task schedules out --> PMU->stop()
+	 * Check for event already stopped.
+	 */
+	if (event->hw.state & PERF_HES_STOPPED)
+		return;
 	if (!event->attr.sample_period) {	/* Counting */
 		pai_pmu[idx].pmu->read(event);
 	} else {				/* Sampling */
-- 
2.55.0

Re: [PATCH] s390/pai: Handle multiple PMU stop callback invocations

[sashiko-bot]

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

Pre-existing issues:
- [High] Preexisting list poisoning crash during system-wide event throttling due to unsafe list traversal.
--

--- Patch [1]: [PATCH] s390/pai: Handle multiple PMU stop callback invocations ---
commit 7f302db53fed202e75cc42832375513bbf0c824b
Author: Thomas Richter <tmricht@linux.ibm.com>

s390/pai: Handle multiple PMU stop callback invocations

This commit adds a PERF_HES_STOPPED check in pai_stop() to prevent
back-to-back invocations when throttling and scheduling out occur closely.

> diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
> index cdb8006220ca0..a377f04296563 100644
> --- a/arch/s390/kernel/perf_pai.c
> +++ b/arch/s390/kernel/perf_pai.c

[ ... ]

> @@ -510,6 +511,13 @@ static void pai_stop(struct perf_event *event, int flags)
>  	struct pai_mapptr *mp = this_cpu_ptr(pai_root[idx].mapptr);
>  	struct pai_map *cpump = mp->mapptr;
>  
> +	/* Cope with multiple invocations:
> +	 *   1. perf_event_throttle() --> PMU->stop()
> +	 *   2. task schedules out --> PMU->stop()
> +	 * Check for event already stopped.
> +	 */
> +	if (event->hw.state & PERF_HES_STOPPED)
> +		return;
>  	if (!event->attr.sample_period) {	/* Counting */
>  		pai_pmu[idx].pmu->read(event);
>  	} else {				/* Sampling */

[Severity: High]
This is a pre-existing issue, but does this throttling path cause list
poisoning during system-wide sampling?

When processing system-wide sampling events during context switches,
pai_have_samples() iterates over syswide_list using list_for_each_entry()
instead of the safe variant:

arch/s390/kernel/perf_pai.c:pai_have_samples() {
    ...
    list_for_each_entry(event, &cpump->syswide_list, hw.tp_list)
        pai_have_sample(event, cpump);
    ...
}

If the sampling limit is exceeded, pai_have_sample() can trigger
perf_event_throttle(), which synchronously calls pai_stop().

Just below the diff hunk above, pai_stop() deletes the currently
iterated event from the list:

arch/s390/kernel/perf_pai.c:pai_stop() {
    ...
    } else {                                /* Sampling */
        if (!(event->attach_state & PERF_ATTACH_TASK)) {
            perf_sched_cb_dec(event->pmu);
            list_del(PAI_SWLIST(event));
        ...
}

Since list_for_each_entry() does not protect against node removal, would
returning to the loop in pai_have_samples() dereference the poisoned pointer
and crash the kernel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814121958.2408863-1-tmricht@linux.ibm.com?part=1

Re: [PATCH] s390/pai: Handle multiple PMU stop callback invocations

[Heiko Carstens]

On Thu, Aug 13, 2026 at 11:33:59AM +0200, Thomas Richter wrote:
> On 8/13/26 09:21, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [High] Unsafe iteration over `cpump->syswide_list` in `pai_have_samples()` leads to a kernel crash (LIST_POISON dereference) if an event overflows and is synchronously throttled during the loop.
> > --
> 
> This can not happen. The syswide_list is anchored by per-CPU pai_map::syswide_list and tracks
> all events which are bound to that particular CPU. The event was installed only on that particular
> CPU and does not move with the process/task. Each CPU has its own list anchored by
>  pai_map::syswide_list.
> 
> The call chains are either
> 
>  paicrypt_sched_task() or paiext_sched_task
>  +--> pai_have_samples() iterates over syswide_list and calls
>       +--> pai_have_sample
...
>  I think this is safe or am I mistaken?

Looks like this is not safe. The following can happen:

-> pai_have_sample()
 -> pai_push_sample()
  -> perf_event_overflow()
   -> event->pmu->stop()
    -> pai_stop()
     -> list_del() modifies the list being traversed
      -> potential crash when list traversal continues

As sashiko proposed: using list_for_each_entry_safe() instead of
list_for_each_entry() would avoid that potential scenario.