* [PATCH v5 0/2] perf: Add some null ptr checks @ 2026-09-01 21:19 Vinay Belgaumkar 2026-09-01 21:19 ` [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar 2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar 0 siblings, 2 replies; 6+ messages in thread From: Vinay Belgaumkar @ 2026-09-01 21:19 UTC (permalink / raw) To: intel-xe; +Cc: dri-devel, Vinay Belgaumkar, Dapeng Mi This series fixes a crash seen when the PMU ptr is freed while there is an open perf connection. Cc: Dapeng Mi <dapeng1.mi@linux.intel.com> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> Vinay Belgaumkar (2): perf: Fix null pointer access in is_include_guest_event() perf: Add checks to prevent null ptr access kernel/events/core.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) -- 2.38.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() 2026-09-01 21:19 [PATCH v5 0/2] perf: Add some null ptr checks Vinay Belgaumkar @ 2026-09-01 21:19 ` Vinay Belgaumkar 2026-09-02 0:54 ` Mi, Dapeng 2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar 1 sibling, 1 reply; 6+ messages in thread From: Vinay Belgaumkar @ 2026-09-01 21:19 UTC (permalink / raw) To: intel-xe; +Cc: dri-devel, Vinay Belgaumkar, Alexander Kanevskiy, Dapeng Mi 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 to is_include_guest_event() ensures we don't accidentally access a null ptr. v2: Add another check to prevent null access (sashiko) v3: Use READ_ONCE for checking pmu state (sashiko) v4: READ_ONCE for the pmu check as well (sashiko) v5: Improve commit message and split patch into 2 (Dapeng) 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 94ca277e0819..f19d27ac411d 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; @@ -12977,6 +12980,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 related [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() 2026-09-01 21:19 ` [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar @ 2026-09-02 0:54 ` Mi, Dapeng 0 siblings, 0 replies; 6+ messages in thread From: Mi, Dapeng @ 2026-09-02 0:54 UTC (permalink / raw) To: Vinay Belgaumkar, intel-xe; +Cc: dri-devel, Alexander Kanevskiy On 9/2/2026 5:19 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 to is_include_guest_event() ensures > we don't accidentally access a null ptr. Better mention adding an extra mediated_pmu_unaccount_event() call before clearing event->pmu to ensure the nr_include_guest_events count is correctly decreased. > > v2: Add another check to prevent null access (sashiko) > v3: Use READ_ONCE for checking pmu state (sashiko) > v4: READ_ONCE for the pmu check as well (sashiko) > v5: Improve commit message and split patch into 2 (Dapeng) Please remove this version history in the formal patch. Others look good to me. Thanks. > > 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 94ca277e0819..f19d27ac411d 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; > @@ -12977,6 +12980,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] 6+ messages in thread
* [PATCH v5 2/2] perf: Add checks to prevent null ptr access 2026-09-01 21:19 [PATCH v5 0/2] perf: Add some null ptr checks Vinay Belgaumkar 2026-09-01 21:19 ` [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar @ 2026-09-01 21:19 ` Vinay Belgaumkar 2026-09-01 21:41 ` sashiko-bot 2026-09-02 0:55 ` Mi, Dapeng 1 sibling, 2 replies; 6+ messages in thread From: Vinay Belgaumkar @ 2026-09-01 21:19 UTC (permalink / raw) To: intel-xe; +Cc: dri-devel, Vinay Belgaumkar, Dapeng Mi 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 f19d27ac411d..bb90034a2054 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 related [flat|nested] 6+ messages in thread
* Re: [PATCH v5 2/2] perf: Add checks to prevent null ptr access 2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar @ 2026-09-01 21:41 ` sashiko-bot 2026-09-02 0:55 ` Mi, Dapeng 1 sibling, 0 replies; 6+ messages in thread From: sashiko-bot @ 2026-09-01 21:41 UTC (permalink / raw) To: Vinay Belgaumkar; +Cc: dri-devel, intel-xe Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [Critical] The added checks for `event->pmu` and `event->state` are insufficient to prevent Time-of-Check to Time-of-Use (TOCTOU) races during concurrent PMU unregistration, leading to potential NULL pointer dereferences or Use-After-Free. -- commit af8b37d090911ccca2049c1eddb3328a00155104 Author: Vinay Belgaumkar <vinay.belgaumkar@intel.com> perf: Add checks to prevent null ptr access This commit adds checks in __perf_event_read_cpu() and perf_event_read_local() to verify that the PMU pointer is valid and the event state is not revoked before proceeding. This aims to prevent null pointer access when reading PMU events. > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 7777e82aad5e7..059f82f0cadd4 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); [Severity: Critical] This is a pre-existing issue, but does caching the PMU pointer here leave us vulnerable to a Use-After-Free? If the PMU driver is concurrently unloaded (e.g., via module unload), perf_pmu_unregister() executes synchronize_rcu() before detaching events. A BPF program starting after the RCU sync might still find the event in its map and race with __pmu_detach_event(). Even though we pass the new !pmu check, if the concurrent unregistration completes and frees the driver's memory immediately after, accessing pmu->scope could result in a Use-After-Free. [ ... ] > @@ -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; > + } > + [Severity: Critical] This is a pre-existing issue, but does this check completely prevent the NULL pointer dereference further down in the function? Later in perf_event_read_local(), there is this call: if (event_oncpu == smp_processor_id()) event->pmu->read(event); If a BPF program invokes perf_event_read_local() concurrently with PMU unregistration, the READ_ONCE(event->state) check might pass. However, __pmu_detach_event() forcefully sets event->pmu = NULL without waiting for perf_event_read_local() to finish. This could cause the subsequent event->pmu->read(event) to dereference a NULL pointer. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260901211915.127704-1-vinay.belgaumkar@intel.com?part=2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 2/2] perf: Add checks to prevent null ptr access 2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar 2026-09-01 21:41 ` sashiko-bot @ 2026-09-02 0:55 ` Mi, Dapeng 1 sibling, 0 replies; 6+ messages in thread From: Mi, Dapeng @ 2026-09-02 0:55 UTC (permalink / raw) To: Vinay Belgaumkar, intel-xe; +Cc: dri-devel LGTM. Thanks. On 9/2/2026 5:19 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 > 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 f19d27ac411d..bb90034a2054 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] 6+ messages in thread
end of thread, other threads:[~2026-09-02 0:55 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-01 21:19 [PATCH v5 0/2] perf: Add some null ptr checks Vinay Belgaumkar 2026-09-01 21:19 ` [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar 2026-09-02 0:54 ` Mi, Dapeng 2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar 2026-09-01 21:41 ` sashiko-bot 2026-09-02 0:55 ` Mi, Dapeng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox