Re: [PATCH 2/2] perf: Add checks to prevent null ptr access

[Peter Zijlstra]

On Fri, Sep 04, 2026 at 11:16:25AM -0700, Vinay Belgaumkar wrote:
> Sashiko recommended some additional checks to prevent null pointer
> access. Check for revoked states inside perf_event_read_local(), as
> the pmu event may have already been freed at this point. Add a null
> check inside __perf_event_read_cpu() as well before accessing the pmu
> ptr.
> 
> Cc: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx>
> ---
>  kernel/events/core.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 7777e82aad5e..059f82f0cadd 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4788,14 +4788,19 @@ static inline const struct cpumask *perf_scope_cpu_topology_cpumask(unsigned int
>  
>  static int __perf_event_read_cpu(struct perf_event *event, int event_cpu)
>  {
> +	struct pmu *pmu = READ_ONCE(event->pmu);
>  	int local_cpu = smp_processor_id();
>  	u16 local_pkg, event_pkg;
>  
>  	if ((unsigned)event_cpu >= nr_cpu_ids)
>  		return event_cpu;
>  
> +	if (!pmu)
> +		return -ENODEV;
> +
>  	if (event->group_caps & PERF_EV_CAP_READ_SCOPE) {
> -		const struct cpumask *cpumask = perf_scope_cpu_topology_cpumask(event->pmu->scope, event_cpu);
> +		const struct cpumask *cpumask = perf_scope_cpu_topology_cpumask(pmu->scope,
> +										event_cpu);
>  
>  		if (cpumask && cpumask_test_cpu(local_cpu, cpumask))
>  			return local_cpu;
> @@ -4917,6 +4922,11 @@ int perf_event_read_local(struct perf_event *event, u64 *value,
>  		goto out;
>  	}
>  
> +	if (READ_ONCE(event->state) <= PERF_EVENT_STATE_REVOKED) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
>  	/*
>  	 * Get the event CPU numbers, and adjust them to local if the event is
>  	 * a per-package event that can be read locally

I don't think any of this is right.

When unregistered, the event is de-scheduled, this means event->oncpu
will be -1, therefore __perf_event_read_cpu() will already exit early.

And perf_event_read_local() will then already do the right thing, by
returning the old value.

So AFAICT, there is nothing to fix here.