[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