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

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 <[email protected]>

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/[email protected]?part=1