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

[PATCH v2] 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().

Also fix unsafe iteration over syswide_list in pai_have_samples()
which might lead to a kernel crash (LIST_POISON dereference) if an
event overflows and is synchronously throttled during the loop.

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>
Suggested-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/perf_pai.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
index cdb8006220ca..6d0bf4ddec8f 100644
--- a/arch/s390/kernel/perf_pai.c
+++ b/arch/s390/kernel/perf_pai.c
@@ -464,6 +464,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)
@@ -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 */
@@ -672,9 +680,9 @@ static void pai_have_samples(int idx)
 {
 	struct pai_mapptr *mp = this_cpu_ptr(pai_root[idx].mapptr);
 	struct pai_map *cpump = mp->mapptr;
-	struct perf_event *event;
+	struct perf_event *event, *e2;
 
-	list_for_each_entry(event, &cpump->syswide_list, hw.tp_list)
+	list_for_each_entry_safe(event, e2, &cpump->syswide_list, hw.tp_list)
 		pai_have_sample(event, cpump);
 }
 
-- 
2.55.0

Re: [PATCH v2] 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] The patch uses `list_for_each_entry_safe()` to fix local iteration of `syswide_list` in `pai_have_samples()`, but fails to address a critical core perf list corruption. Synchronously calling `perf_sched_cb_dec()` inside `pai_stop()` during throttling deletes the PMU from the core perf's `sched_cb_list` while it is being actively iterated, causing a `LIST_POISON1` dereference.
--

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

s390/pai: Handle multiple PMU stop callback invocations

This commit adds a check for PERF_HES_STOPPED in PMU->stop() to handle
multiple back-to-back invocations from throttling and scheduling. It also
changes pai_have_samples() to safely iterate syswide_list to prevent list
corruption during synchronous throttling.

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

[ ... ]

> @@ -672,9 +680,9 @@ static void pai_have_samples(int idx)
>  {
>  	struct pai_mapptr *mp = this_cpu_ptr(pai_root[idx].mapptr);
>  	struct pai_map *cpump = mp->mapptr;
> -	struct perf_event *event;
> +	struct perf_event *event, *e2;
>  
> -	list_for_each_entry(event, &cpump->syswide_list, hw.tp_list)
> +	list_for_each_entry_safe(event, e2, &cpump->syswide_list, hw.tp_list)
>  		pai_have_sample(event, cpump);
>  }

[Severity: High]
This is a pre-existing issue, but does fixing the local syswide_list
iteration here still leave a similar list corruption in the core perf code?

If a system-wide PAI sampling event with a low sample period overflows
during a context switch (while inside this pai_have_samples() loop), it
can trigger perf_event_throttle(), which synchronously calls pmu->stop().

In that path, pai_stop() calls perf_sched_cb_dec(event->pmu), which mutates
the core sched_cb_list:

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

However, perf_pmu_sched_task() in kernel/events/core.c iterates that same
sched_cb_list using an unsafe list_for_each_entry() loop:

kernel/events/core.c:perf_pmu_sched_task() {
    ...
    list_for_each_entry(cpc, this_cpu_ptr(&sched_cb_list), sched_cb_entry)
        __perf_pmu_sched_task(cpc, sched_in ? next : prev, sched_in);
    ...
}

Could this synchronous perf_sched_cb_dec() call from pai_stop() during
throttling still cause a LIST_POISON1 dereference crash inside
perf_pmu_sched_task()?

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