* [PATCH 1/2] perf: Fix null pointer access in is_include_guest_event()
@ 2026-09-04 18:16 Vinay Belgaumkar
2026-09-04 18:16 ` [PATCH 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Vinay Belgaumkar @ 2026-09-04 18:16 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin,
Andi Kleen, Eranian Stephane
Cc: linux-kernel, linux-perf-users, Dapeng Mi, Vinay Belgaumkar,
Alexander Kanevskiy
A typical module unload occurring event when there is an active perf
connection leads to freeing of the pmu pointer. The call log is something
like:
..
__pmu_detach_event
pmu_detach_event
pmu_detach_events
perf_pmu_unregister
..
__pmu_detach_event() sets event->pmu to null. When the perf connection
finally is closed, the following stack trace is observed:
Oops: general protection fault, kernel NULL pointer dereference
...
RIP: 0010:_free_event+0x3e/0x370
...
Call Trace:
...
perf_event_release_kernel+0x260/0x2d0
perf_release+0x12/0x20
A call to mediated_pmu_unaccount_event() inside _free_event() is the root
cause of this crash. Adding a check inside is_include_guest_event() ensures
we don't accidentally access a null pmu ptr. In addition to this, we will
now call mediated_pmu_unaccount_event() before clearing the pmu ptr so that
nr_include_guest_events counts are maintained correctly.
Fixes: eff95e170275 ("perf: Add APIs to create/release mediated guest vPMUs")
Cc: Alexander Kanevskiy <alexander.kanevskiy@intel.com>
Cc: Dapeng Mi <dapeng1.mi@linux.intel.com>
Assisted-by: Claude:Claude-Sonnet-5
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
kernel/events/core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index a6c8e38a3110..7777e82aad5e 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6350,6 +6350,9 @@ static DEFINE_MUTEX(perf_mediated_pmu_mutex);
/* !exclude_guest event of PMU with PERF_PMU_CAP_MEDIATED_VPMU */
static inline bool is_include_guest_event(struct perf_event *event)
{
+ if (!event->pmu)
+ return false;
+
if ((event->pmu->capabilities & PERF_PMU_CAP_MEDIATED_VPMU) &&
!event->attr.exclude_guest)
return true;
@@ -13002,6 +13005,7 @@ static void __pmu_detach_event(struct pmu *pmu, struct perf_event *event,
exclusive_event_destroy(event);
module_put(pmu->module);
+ mediated_pmu_unaccount_event(event);
event->pmu = NULL; /* force fault instead of UAF */
}
--
2.38.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 2/2] perf: Add checks to prevent null ptr access 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 ` Vinay Belgaumkar 2026-09-16 1:40 ` Mi, Dapeng 2026-09-17 8:39 ` Peter Zijlstra 2026-09-07 6:24 ` [PATCH 1/2] perf: Fix null pointer access in is_include_guest_event() Mi, Dapeng ` (2 subsequent siblings) 3 siblings, 2 replies; 11+ messages in thread From: Vinay Belgaumkar @ 2026-09-04 18:16 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane Cc: linux-kernel, linux-perf-users, Dapeng Mi, Vinay Belgaumkar 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 -- 2.38.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf: Add checks to prevent null ptr access 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 8:39 ` Peter Zijlstra 1 sibling, 1 reply; 11+ messages in thread From: Mi, Dapeng @ 2026-09-16 1:40 UTC (permalink / raw) To: Vinay Belgaumkar, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane Cc: linux-kernel, linux-perf-users On 9/5/2026 2:16 AM, 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 Could you please show where perf_event_read_local() could be called after the event is revoked? BTW, the prefix should be "perf/core:" instead of "perf:" by following current naming convention. Thanks. > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf: Add checks to prevent null ptr access 2026-09-16 1:40 ` Mi, Dapeng @ 2026-09-16 21:48 ` Belgaumkar, Vinay 2026-09-17 1:03 ` Mi, Dapeng 0 siblings, 1 reply; 11+ messages in thread From: Belgaumkar, Vinay @ 2026-09-16 21:48 UTC (permalink / raw) To: Mi, Dapeng, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane Cc: linux-kernel, linux-perf-users On 9/15/2026 6:40 PM, Mi, Dapeng wrote: > On 9/5/2026 2:16 AM, 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 > Could you please show where perf_event_read_local() could be called after > the event is revoked? perf_event_read_local() is only available to be called in the kernel context. So, BPF helpers can call it. This was the scenario outlined by Sashiko: If CPU A calls perf_event_read_local(), disables interrupts, and locklessly checks event->state, it can pass if the event state is PERF_EVENT_STATE_INACTIVE. Concurrently, if CPU B calls perf_pmu_unregister() which leads to __pmu_detach_event(), it will not send an IPI to synchronize with CPU A since the event is inactive. CPU B then sets event->state to PERF_EVENT_STATE_REVOKED and clears event->pmu to NULL. When CPU A continues and calls __perf_event_read_cpu(event, event->cpu), if the event has PERF_EV_CAP_READ_SCOPE, it will dereference event->pmu->scope, which was just set to NULL by CPU B, causing a panic. So, I think the scenario is- we have a regular perf session ongoing and a parallel session (through BPF) that tries to access the same counter. > > BTW, the prefix should be "perf/core:" instead of "perf:" by following > current naming convention. Sure. Thanks, Vinay. > > Thanks. > > >> 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf: Add checks to prevent null ptr access 2026-09-16 21:48 ` Belgaumkar, Vinay @ 2026-09-17 1:03 ` Mi, Dapeng 0 siblings, 0 replies; 11+ messages in thread From: Mi, Dapeng @ 2026-09-17 1:03 UTC (permalink / raw) To: Belgaumkar, Vinay, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane Cc: linux-kernel, linux-perf-users On 9/17/2026 5:48 AM, Belgaumkar, Vinay wrote: > On 9/15/2026 6:40 PM, Mi, Dapeng wrote: >> On 9/5/2026 2:16 AM, 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 >> Could you please show where perf_event_read_local() could be called after >> the event is revoked? > perf_event_read_local() is only available to be called in the kernel > context. So, BPF helpers can call it. This was the scenario outlined by > Sashiko: > > If CPU A calls perf_event_read_local(), disables interrupts, and locklessly > checks event->state, it can pass if the event state is > PERF_EVENT_STATE_INACTIVE. > > Concurrently, if CPU B calls perf_pmu_unregister() which leads to > __pmu_detach_event(), it will not send an IPI to synchronize with CPU A > since the event is inactive. CPU B then sets event->state to > PERF_EVENT_STATE_REVOKED and clears event->pmu to NULL. > > When CPU A continues and calls __perf_event_read_cpu(event, event->cpu), > if the event has PERF_EV_CAP_READ_SCOPE, it will dereference > event->pmu->scope, which was just set to NULL by CPU B, causing a panic. Ok, better add this into the change log, so the reviewers know why we need this change. Thanks. > > So, I think the scenario is- we have a regular perf session ongoing and > a parallel session (through BPF) that tries to access the same counter. > >> BTW, the prefix should be "perf/core:" instead of "perf:" by following >> current naming convention. > Sure. > > Thanks, > > Vinay. > >> Thanks. >> >> >>> 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf: Add checks to prevent null ptr access 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-17 8:39 ` Peter Zijlstra 2026-09-17 20:10 ` Belgaumkar, Vinay 1 sibling, 1 reply; 11+ messages in thread From: Peter Zijlstra @ 2026-09-17 8:39 UTC (permalink / raw) To: Vinay Belgaumkar Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane, linux-kernel, linux-perf-users, Dapeng Mi 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf: Add checks to prevent null ptr access 2026-09-17 8:39 ` Peter Zijlstra @ 2026-09-17 20:10 ` Belgaumkar, Vinay 2026-09-18 9:09 ` Peter Zijlstra 0 siblings, 1 reply; 11+ messages in thread From: Belgaumkar, Vinay @ 2026-09-17 20:10 UTC (permalink / raw) To: Peter Zijlstra Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane, linux-kernel, linux-perf-users, Dapeng Mi 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf: Add checks to prevent null ptr access 2026-09-17 20:10 ` Belgaumkar, Vinay @ 2026-09-18 9:09 ` Peter Zijlstra 0 siblings, 0 replies; 11+ messages in thread From: Peter Zijlstra @ 2026-09-18 9:09 UTC (permalink / raw) To: Belgaumkar, Vinay Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane, linux-kernel, linux-perf-users, Dapeng Mi On Thu, Sep 17, 2026 at 01:10:15PM -0700, Belgaumkar, Vinay wrote: > > 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. perf_event_read_local() has IRQs disabled, perf_pmu_unregister() will eventually have to de-schedule the event, which involves IPIs. If you have IRQs disabled, those IPIs will wait. IOW, as long as you have IRQs disabled, your event->oncpu is stable, provided of course that event->cpu is the local CPU, otherwise having called perf_event_read_local() was a bug in the first place. Hmm... I think I see a problem though. The verification of that last condition, it being a local event. That uses event->cpu as argument to __perf_event_read_cpu(), and that *can* indeed hit the pmu. I'm thinking __pmu_detach_event() should probably clear PERF_EV_CAP_READ_SCOPE or something from all the event->{event,group}_caps fields. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] perf: Fix null pointer access in is_include_guest_event() 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-07 6:24 ` Mi, Dapeng 2026-09-18 1:07 ` Mi, Dapeng 2026-09-18 10:27 ` [tip: perf/urgent] " tip-bot2 for Vinay Belgaumkar 3 siblings, 0 replies; 11+ messages in thread From: Mi, Dapeng @ 2026-09-07 6:24 UTC (permalink / raw) To: Vinay Belgaumkar, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane Cc: linux-kernel, linux-perf-users, Alexander Kanevskiy LGTM. Thanks. Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com> On 9/5/2026 2:16 AM, Vinay Belgaumkar wrote: > A typical module unload occurring event when there is an active perf > connection leads to freeing of the pmu pointer. The call log is something > like: > .. > __pmu_detach_event > pmu_detach_event > pmu_detach_events > perf_pmu_unregister > .. > > __pmu_detach_event() sets event->pmu to null. When the perf connection > finally is closed, the following stack trace is observed: > > Oops: general protection fault, kernel NULL pointer dereference > ... > RIP: 0010:_free_event+0x3e/0x370 > ... > Call Trace: > ... > perf_event_release_kernel+0x260/0x2d0 > perf_release+0x12/0x20 > > A call to mediated_pmu_unaccount_event() inside _free_event() is the root > cause of this crash. Adding a check inside is_include_guest_event() ensures > we don't accidentally access a null pmu ptr. In addition to this, we will > now call mediated_pmu_unaccount_event() before clearing the pmu ptr so that > nr_include_guest_events counts are maintained correctly. > > Fixes: eff95e170275 ("perf: Add APIs to create/release mediated guest vPMUs") > Cc: Alexander Kanevskiy <alexander.kanevskiy@intel.com> > Cc: Dapeng Mi <dapeng1.mi@linux.intel.com> > Assisted-by: Claude:Claude-Sonnet-5 > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > --- > kernel/events/core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index a6c8e38a3110..7777e82aad5e 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -6350,6 +6350,9 @@ static DEFINE_MUTEX(perf_mediated_pmu_mutex); > /* !exclude_guest event of PMU with PERF_PMU_CAP_MEDIATED_VPMU */ > static inline bool is_include_guest_event(struct perf_event *event) > { > + if (!event->pmu) > + return false; > + > if ((event->pmu->capabilities & PERF_PMU_CAP_MEDIATED_VPMU) && > !event->attr.exclude_guest) > return true; > @@ -13002,6 +13005,7 @@ static void __pmu_detach_event(struct pmu *pmu, struct perf_event *event, > exclusive_event_destroy(event); > module_put(pmu->module); > > + mediated_pmu_unaccount_event(event); > event->pmu = NULL; /* force fault instead of UAF */ > } > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] perf: Fix null pointer access in is_include_guest_event() 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-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 3 siblings, 0 replies; 11+ messages in thread From: Mi, Dapeng @ 2026-09-18 1:07 UTC (permalink / raw) To: Vinay Belgaumkar, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane Cc: linux-kernel, linux-perf-users, Alexander Kanevskiy Better update the prefix of short log to "perf/core:", other looks good to me. Thanks. Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com> On 9/5/2026 2:16 AM, Vinay Belgaumkar wrote: > A typical module unload occurring event when there is an active perf > connection leads to freeing of the pmu pointer. The call log is something > like: > .. > __pmu_detach_event > pmu_detach_event > pmu_detach_events > perf_pmu_unregister > .. > > __pmu_detach_event() sets event->pmu to null. When the perf connection > finally is closed, the following stack trace is observed: > > Oops: general protection fault, kernel NULL pointer dereference > ... > RIP: 0010:_free_event+0x3e/0x370 > ... > Call Trace: > ... > perf_event_release_kernel+0x260/0x2d0 > perf_release+0x12/0x20 > > A call to mediated_pmu_unaccount_event() inside _free_event() is the root > cause of this crash. Adding a check inside is_include_guest_event() ensures > we don't accidentally access a null pmu ptr. In addition to this, we will > now call mediated_pmu_unaccount_event() before clearing the pmu ptr so that > nr_include_guest_events counts are maintained correctly. > > Fixes: eff95e170275 ("perf: Add APIs to create/release mediated guest vPMUs") > Cc: Alexander Kanevskiy <alexander.kanevskiy@intel.com> > Cc: Dapeng Mi <dapeng1.mi@linux.intel.com> > Assisted-by: Claude:Claude-Sonnet-5 > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > --- > kernel/events/core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index a6c8e38a3110..7777e82aad5e 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -6350,6 +6350,9 @@ static DEFINE_MUTEX(perf_mediated_pmu_mutex); > /* !exclude_guest event of PMU with PERF_PMU_CAP_MEDIATED_VPMU */ > static inline bool is_include_guest_event(struct perf_event *event) > { > + if (!event->pmu) > + return false; > + > if ((event->pmu->capabilities & PERF_PMU_CAP_MEDIATED_VPMU) && > !event->attr.exclude_guest) > return true; > @@ -13002,6 +13005,7 @@ static void __pmu_detach_event(struct pmu *pmu, struct perf_event *event, > exclusive_event_destroy(event); > module_put(pmu->module); > > + mediated_pmu_unaccount_event(event); > event->pmu = NULL; /* force fault instead of UAF */ > } > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tip: perf/urgent] perf: Fix null pointer access in is_include_guest_event() 2026-09-04 18:16 [PATCH 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar ` (2 preceding siblings ...) 2026-09-18 1:07 ` Mi, Dapeng @ 2026-09-18 10:27 ` tip-bot2 for Vinay Belgaumkar 3 siblings, 0 replies; 11+ messages in thread From: tip-bot2 for Vinay Belgaumkar @ 2026-09-18 10:27 UTC (permalink / raw) To: linux-tip-commits Cc: Vinay Belgaumkar, Peter Zijlstra (Intel), Dapeng Mi, x86, linux-kernel The following commit has been merged into the perf/urgent branch of tip: Commit-ID: 88aed0422f39b22406f35f1e758cea25e7bbcfb5 Gitweb: https://git.kernel.org/tip/88aed0422f39b22406f35f1e758cea25e7bbcfb5 Author: Vinay Belgaumkar <vinay.belgaumkar@intel.com> AuthorDate: Fri, 04 Sep 2026 11:16:24 -07:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Fri, 18 Sep 2026 12:19:43 +02:00 perf: Fix null pointer access in is_include_guest_event() A typical module unload occurring event when there is an active perf connection leads to freeing of the pmu pointer. The call log is something like: .. __pmu_detach_event pmu_detach_event pmu_detach_events perf_pmu_unregister .. __pmu_detach_event() sets event->pmu to null. When the perf connection finally is closed, the following stack trace is observed: Oops: general protection fault, kernel NULL pointer dereference ... RIP: 0010:_free_event+0x3e/0x370 ... Call Trace: ... perf_event_release_kernel+0x260/0x2d0 perf_release+0x12/0x20 A call to mediated_pmu_unaccount_event() inside _free_event() is the root cause of this crash. Adding a check inside is_include_guest_event() ensures we don't accidentally access a null pmu ptr. In addition to this, we will now call mediated_pmu_unaccount_event() before clearing the pmu ptr so that nr_include_guest_events counts are maintained correctly. Fixes: eff95e170275 ("perf: Add APIs to create/release mediated guest vPMUs") Assisted-by: Claude:Claude-Sonnet-5 Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com> Link: https://patch.msgid.link/20260904181625.1394082-1-vinay.belgaumkar@intel.com --- kernel/events/core.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/kernel/events/core.c b/kernel/events/core.c index fe33fe1..db7b76d 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -6350,6 +6350,9 @@ static DEFINE_MUTEX(perf_mediated_pmu_mutex); /* !exclude_guest event of PMU with PERF_PMU_CAP_MEDIATED_VPMU */ static inline bool is_include_guest_event(struct perf_event *event) { + if (!event->pmu) + return false; + if ((event->pmu->capabilities & PERF_PMU_CAP_MEDIATED_VPMU) && !event->attr.exclude_guest) return true; @@ -13002,6 +13005,7 @@ static void __pmu_detach_event(struct pmu *pmu, struct perf_event *event, exclusive_event_destroy(event); module_put(pmu->module); + mediated_pmu_unaccount_event(event); event->pmu = NULL; /* force fault instead of UAF */ } ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-09-18 10:27 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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®