Re: [PATCH 1/3 v8] s390/pai: Use PAI PMU index as parameter replacing event

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing bounds check on the hardware-reported NNPA counter count (`ib.num_nnpa`) leads to an out-of-bounds kernel heap read.
--

commit a6f12edfe6f84375133dd3bb77d40f4e8c67385e
Author: Thomas Richter <tmricht@linux.ibm.com>
s390/pai: Use PAI PMU index as parameter replacing event

This patch updates pai_alloc_cpu() and pai_event_destroy_cpu() to use
the PAI PMU index value as a parameter instead of a struct perf_event
pointer. Only the index value is actually needed inside these functions.

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

[ ... ]

> @@ -241,12 +239,12 @@ static u64 paicrypt_getall(struct perf_event *event)
>   *
>   * Allocate the memory for the event.
>   */
> -static int pai_alloc_cpu(struct perf_event *event, int cpu)
> +static int pai_alloc_cpu(int idx, int cpu)
>  {
> -	int rc, idx = PAI_PMU_IDX(event);
>  	struct pai_map *cpump = NULL;
>  	bool need_paiext_cb = false;
>  	struct pai_mapptr *mp;
> +	int rc;

[Severity: High]
This is a pre-existing issue, but I have a question about how the buffer
allocated in this function is accessed.

In paipmu_setup(), the hardware-reported counter count (ib.num_nnpa) can be
up to 127. The code then artificially increments it for the software ALL event:

arch/s390/kernel/perf_pai.c:paipmu_setup() {
    ...
		case PAI_PMU_EXT:
			p->num_avail = ib.num_nnpa;
			break;
		}
		p->num_avail += 1;		/* Add xxx_ALL event */
    ...
}

This means p->num_avail can reach 128.

Later in pai_alloc_cpu(), cpump->area is allocated with a fixed size:

arch/s390/kernel/perf_pai.c:pai_alloc_cpu() {
    ...
			cpump->area = kzalloc(pai_pmu[idx].area_size, GFP_KERNEL);
    ...
}

For PAI_PMU_EXT, area_size is 1024 bytes, which holds exactly 128 elements.

However, pai_getdata() iterates up to p->num_avail:

arch/s390/kernel/perf_pai.c:pai_getdata() {
    ...
	for (i = 1; i <= pp->num_avail; i++) {
		u64 val = pai_getctr(cpump->area, i,
				     kernel ? pp->kernel_offset : 0);
    ...
}

If ib.num_nnpa is 127, p->num_avail becomes 128. The loop evaluates i = 128
and pai_getctr() accesses cpump->area[128] (the 129th element).

Can this cause an out-of-bounds heap read if cpump->area is strictly 1024
bytes?

>  
>  	mutex_lock(&pai_reserve_mutex);
>  	/* Allocate root node */

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