Re: [PATCH 9/9] perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned

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

Pre-existing issues:
- [High] The patch fixes an out-of-bounds pointer dereference in `cpumask_show()` caused by `info->on_cpu == -1`, but fails to fix the exact same condition in `cxl_pmu_event_init()`. This omission allows `event->cpu` to become `-1`, leading to an out-of-bounds array read and a wild pointer dereference in the perf core.
--

--- Patch [9]: [PATCH 9/9] perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned ---
commit 94b991fd5de15c6e9d3fde196012a4cea806254d
Author: Dave Jiang <[email protected]>

perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned

This patch fixes an out-of-bounds pointer dereference in cpumask_show()
by checking if info->on_cpu is -1. When no CPU is assigned, it emits
an empty mask instead of performing invalid pointer arithmetic.

Closes: https://sashiko.dev/#/patchset/[email protected]?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index f1110c5029d6a..80145e85fe36b 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -501,8 +501,17 @@ static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
>  			    char *buf)
>  {
>  	struct cxl_pmu_info *info = dev_get_drvdata(dev);
> +	int cpu = READ_ONCE(info->on_cpu);
>  
> -	return cpumap_print_to_pagebuf(true, buf, cpumask_of(info->on_cpu));
> +	/*
> +	 * on_cpu is -1 before the first online callback and transiently during
> +	 * cxl_pmu_offline_cpu(). cpumask_of(-1) computes an out-of-bounds
> +	 * pointer, so report an empty mask instead.
> +	 */
> +	if (cpu < 0)
> +		return sysfs_emit(buf, "\n");
> +
> +	return cpumap_print_to_pagebuf(true, buf, cpumask_of(cpu));
>  }

[Severity: High]
This is a pre-existing issue, but since info->on_cpu can be -1 during
cxl_pmu_offline_cpu(), does this same out-of-bounds issue happen in
cxl_pmu_event_init()?

cxl_pmu_event_init() unconditionally assigns info->on_cpu to event->cpu:

drivers/perf/cxl_pmu.c:cxl_pmu_event_init() {
    ...
    event->cpu = info->on_cpu;
    ...
}

If a user calls perf_event_open() concurrently with cxl_pmu_offline_cpu(),
this bypasses the perf core's initial validation of the cpu argument.

The perf core will then use the -1 value:

kernel/events/core.c:find_get_context() {
    ...
    cpuctx = per_cpu_ptr(&perf_cpu_context, event->cpu);
    ...
}

Could this lead to an out-of-bounds array read and wild pointer dereference
in the perf core? Should cxl_pmu_event_init() also validate that
info->on_cpu is not -1 before assignment?

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