From: "Belgaumkar, Vinay" <vinay.belgaumkar@intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
"Alexander Shishkin" <alexander.shishkin@linux.intel.com>,
Andi Kleen <ak@linux.intel.com>,
Eranian Stephane <eranian@google.com>,
<linux-kernel@vger.kernel.org>,
<linux-perf-users@vger.kernel.org>,
Dapeng Mi <dapeng1.mi@linux.intel.com>
Subject: Re: [PATCH 2/2] perf: Add checks to prevent null ptr access
Date: Thu, 17 Sep 2026 13:10:15 -0700 [thread overview]
Message-ID: <e5e1d901-9cb8-44b0-a897-da48f12268bf@intel.com> (raw)
In-Reply-To: <20260917083951.GC2009045@noisy.programming.kicks-ass.net>
On 9/17/2026 1:39 AM, Peter Zijlstra wrote:
> 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@linux.intel.com>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> ---
>> 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.
yeah, I think this was more of a defensive fix which Sashiko suggested-
CPU A CPU B
perf_event_read_local ...
__perf_event_read_cpu perf_pmu_unregister
I don't think it is easy to repro this situation, but there is a
theoretical possibility of a race between these two functions. I don't
think even the changes above can guarantee to work in any case. We can
drop this second patch if that is the case.
Thanks,
Vinay.
next prev parent reply other threads:[~2026-09-17 20:10 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-04 18:16 [PATCH 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar
2026-09-04 18:16 ` [PATCH 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar
2026-09-16 1:40 ` Mi, Dapeng
2026-09-16 21:48 ` Belgaumkar, Vinay
2026-09-17 1:03 ` Mi, Dapeng
2026-09-17 8:39 ` Peter Zijlstra
2026-09-17 20:10 ` Belgaumkar, Vinay [this message]
2026-09-18 9:09 ` Peter Zijlstra
2026-09-07 6:24 ` [PATCH 1/2] perf: Fix null pointer access in is_include_guest_event() Mi, Dapeng
2026-09-18 1:07 ` Mi, Dapeng
2026-09-18 10:27 ` [tip: perf/urgent] " tip-bot2 for Vinay Belgaumkar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e5e1d901-9cb8-44b0-a897-da48f12268bf@intel.com \
--to=vinay.belgaumkar@intel.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=dapeng1.mi@linux.intel.com \
--cc=eranian@google.com \
--cc=irogers@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®