Re: [PATCH 2/2] s390/pai: Move locking to event init and delete

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

Pre-existing issues:
- [High] Missing CPU hotplug support causes a NULL pointer dereference in `pai_add()` when a task with an active PAI event migrates to a newly onlined CPU.
--

--- Patch [2]: [PATCH 2/2] s390/pai: Move locking to event init and delete ---
commit fa13d9fdfca46f4567335a92efab0ab04b313967
Author: Thomas Richter <[email protected]>

s390/pai: Move locking to event init and delete

This commit moves the mutex locking from the per-CPU allocation functions
to the higher-level event allocation and deletion routines. No functional
changes are introduced.

> diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
> index 7c13f5586c793..03c5bda53cccb 100644
> --- a/arch/s390/kernel/perf_pai.c
> +++ b/arch/s390/kernel/perf_pai.c

[ ... ]

> @@ -390,10 +388,12 @@ static int pai_event_init(struct perf_event *event, int idx)
>  		}
>  	}
>  
> +	mutex_lock(&pai_reserve_mutex);
>  	if (event->cpu >= 0)
>  		rc = pai_alloc_cpu(idx, event->cpu);
>  	else
>  		rc = pai_alloc(event);

[Severity: High]
This is a pre-existing issue, but does the current design lack CPU hotplug
support?

Looking at pai_alloc() called here, it only allocates memory for currently
online CPUs:

pai_alloc() {
...
        for_each_online_cpu(cpu) {
                rc = pai_alloc_cpu(idx, cpu);
...

If a task-bound PAI event is created and a new CPU is hotplugged later, the
per-CPU mapptr for the new CPU appears to remain NULL.

When the task with the active event is scheduled onto the newly onlined
CPU, the perf core will call pai_add(), which seems to unconditionally
dereference cpump:

pai_add() {
        struct pai_map *cpump = mp->mapptr;
        struct paiext_cb *pcb = cpump->paiext_cb;

        if (++cpump->active_events == 1) {
...

Can this result in a kernel panic due to a NULL pointer dereference when
a task migrates to a newly onlined CPU?

> +	mutex_unlock(&pai_reserve_mutex);
>  	if (rc) {
>  		free_page(PAI_SAVE_AREA(event));
>  		goto out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2