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

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

Prohibit ioctl(fd, PERF_EVENT_IOC_PERIOD, ...) call for this event.
It sets perf_event::event_limit to a positive value and causes
perf_event_overflow() to invoke pai_stop() call back function when
perf_event::event_limit hits zero. This is not supported because the
sample events CRYPTO_ALL and NNPA_ALL are only taken at schedule out
of a task.

Use list_for_each_entry_safe() for safe iteration over syswide_list
in pai_have_samples().

 #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 | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
index cdb8006220ca..05f74d74fad1 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);
 }
 
@@ -691,6 +699,17 @@ static void paicrypt_sched_task(struct perf_event_pmu_context *pmu_ctx,
 		pai_have_samples(PAI_PMU_CRYPTO);
 }
 
+/* Prevent ioctl(fd, PERF_EVENT_IOC_PERIOD, ...) call.
+ * It sets perf_event::event_limit to a positive value and causes
+ * perf_event_overflow() to invoke pai_stop() call back function when
+ * perf_event::event_limit hits zero. This is not supported because the
+ * sample events CRYPTO_ALL and NNPA_ALL are always taken at schedule out
+ * of a task.
+ */
+static int pai_check_period(struct perf_event *event, u64 value)
+{
+	return -EINVAL;
+}
 /* ============================= paiext ====================================*/
 
 static void paiext_event_destroy(struct perf_event *event)
@@ -804,6 +823,7 @@ static struct pmu paicrypt = {
 	.stop	      = paicrypt_stop,
 	.read	      = paicrypt_read,
 	.sched_task   = paicrypt_sched_task,
+	.check_period = pai_check_period,
 	.attr_groups  = paicrypt_attr_groups
 };
 
@@ -1015,6 +1035,7 @@ static struct pmu paiext = {
 	.stop	      = paiext_stop,
 	.read	      = paiext_read,
 	.sched_task   = paiext_sched_task,
+	.check_period = pai_check_period,
 	.attr_groups  = paiext_attr_groups,
 };
 
-- 
2.55.0

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

[sashiko-bot]

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

Pre-existing issues:
- [Medium] `list_for_each_entry_safe` in `pai_have_samples()` is insufficient to prevent list corruption (dereferencing `LIST_POISON1`) when group throttling removes sibling events concurrently.
- [Medium] The patch fails to prohibit setting `event_limit`, leaving the unsupported `pai_stop()` invocation pathway open via `PERF_EVENT_IOC_REFRESH`.
--

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

s390/pai: Handle multiple PMU stop callback invocations

This commit handles back-to-back PMU stop invocations during task
scheduling and event throttling. It also attempts to prevent the
PERF_EVENT_IOC_PERIOD ioctl for specific events and switches to safe
list iteration in pai_have_samples().

> diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
> index cdb8006220ca0..05f74d74fad13 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)

[Severity: Medium]
This is a pre-existing issue, but is list_for_each_entry_safe()
sufficient here if group throttling occurs concurrently?

Since this only protects against the removal of the current entry, if
perf_event_throttle_group() synchronously removes the sibling event
(which is temporarily held in e2), could this result in dereferencing
LIST_POISON1 on the next iteration?

>  		pai_have_sample(event, cpump);
>  }
>  
> @@ -691,6 +699,17 @@ static void paicrypt_sched_task(struct perf_event_pmu_context *pmu_ctx,
>  		pai_have_samples(PAI_PMU_CRYPTO);
>  }
>  
> +/* Prevent ioctl(fd, PERF_EVENT_IOC_PERIOD, ...) call.
> + * It sets perf_event::event_limit to a positive value and causes
> + * perf_event_overflow() to invoke pai_stop() call back function when
> + * perf_event::event_limit hits zero. This is not supported because the
> + * sample events CRYPTO_ALL and NNPA_ALL are always taken at schedule out
> + * of a task.
> + */
> +static int pai_check_period(struct perf_event *event, u64 value)
> +{
> +	return -EINVAL;
> +}

[Severity: Medium]
This isn't a bug introduced by this patch, but does this fully prevent
userspace from setting event_limit to a positive value?

While this correctly blocks PERF_EVENT_IOC_PERIOD, could a user still
trigger the unsupported behavior by using PERF_EVENT_IOC_REFRESH, which
also modifies event_limit and bypasses this check?

[ ... ]

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

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

[Thomas Richter]

On 8/19/26 08:11, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Medium] `list_for_each_entry_safe` in `pai_have_samples()` is insufficient to prevent list corruption (dereferencing `LIST_POISON1`) when group throttling removes sibling events concurrently.

perf_event_throttle_group() walks a different list anchored
by perf_event::sibling_list. The list anchored by cpuhw::syswide
is a complete different list using different nodes.
Since events CRYPTO_ALL and NNPA_ALL are single sampling events
and the only sampling event for each PAI PMU, it makes no sense
to group the same event. Events of different PMUS can not be grouped
anyway.

> - [Medium] The patch fails to prohibit setting `event_limit`, leaving the unsupported `pai_stop()` invocation pathway open via `PERF_EVENT_IOC_REFRESH`.
> --
> 
This needs to be addressed in a follow-on patch, the invocation of perf_event_refresh() can not
be blocked right now. Maybe add a new PMU call back.


-- 
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