Re: [PATCH v2] 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] 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 <[email protected]>

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