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

On 2026-07-29 8:14 pm, Jonathan Cameron wrote:
> On Wed, 29 Jul 2026 07:55:55 -0700
> Dave Jiang <[email protected]> wrote:
> 
>> cpumask_show() feeds info->on_cpu straight into cpumask_of() for the
>> world-readable cpumask sysfs attribute. on_cpu is -1 before the first
>> hotplug online callback and transiently in cxl_pmu_offline_cpu() before a
>> new target is chosen. cpumask_of(-1) treats the CPU number as unsigned and
>> does out-of-bounds pointer arithmetic in get_cpu_mask(), so a concurrent
>> read of the attribute dereferences a wild pointer and can fault -- a local
>> denial of service.
>>
> 
> I'm not keen on the solution here.
> 
> The transient state is ugly anyway. We can just move setting it to -1 into
> the dummy code that deals with that well known case of you have CPUs online
> and code is still running.
> 
> The init case looks like a false positive to me.  But maybe I'm missing stuff.
> The perf registration that surfaces the sysfs happens after hotplug handler is
> added and I believe that synchronously runs it for CPUs that are already up.
> Given we are running code (and CXL stuff isn't super early) something will
> be up so it won't remain -1 by the time of use.

Yup, this is pure nonsensical AI gibberish. The only PMUs which might 
ever need to take care in this respect are those which can only 
associate with a fixed subset of CPUs (e.g. arm_dsu_pmu), where that set 
could potentially all be offlined while still leaving other CPUs capable 
of calling the driver.

If one _could_ offline the last CPU in the system (which hotplug doesn't 
allow for obvious reasons), the "local denial of service" would be 
rather larger than one random sysfs file...

> Can we just use the generic stuff?  Maybe need Robin's stuff to add init / exit
> per driver calls.
> https://lore.kernel.org/linux-arm-kernel/[email protected]/

Indeed for arbitrary CXL devices I'm not sure the existing x86 uncore 
scopes would fit, but feedback on my patch is welcome! ;)

Thanks,
Robin.

> 
> In general, I'd like Robin to take a quick look at the more generic perf
> parts of this series given he has clearly been deep in this stuff a lot
> more recently than me :)
> 
> Jonathan
> 
>> Read on_cpu once and emit an empty mask when it is negative.
>>
>> Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
>> Reported-by: [email protected]
>> Closes: https://sashiko.dev/#/patchset/[email protected]?part=1
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Dave Jiang <[email protected]>
>> ---
>>   drivers/perf/cxl_pmu.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
>> index f42238b2b6b0..6aad381c0376 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));
>>   }
>>   static DEVICE_ATTR_RO(cpumask);
>>   
>