kernel/events/core.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 88d51e795539acd08bce028eff3aa78748b847a8
Gitweb: https://git.kernel.org/tip/88d51e795539acd08bce028eff3aa78748b847a8
Author: Frederic Weisbecker <frederic@kernel.org>
AuthorDate: Mon, 28 Apr 2025 13:11:47 +02:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 02 May 2025 12:40:40 +02:00
perf: Fix irq work dereferencing garbage
The following commit:
da916e96e2de ("perf: Make perf_pmu_unregister() useable")
has introduced two significant event's parent lifecycle changes:
1) An event that has exited now has EVENT_TOMBSTONE as a parent.
This can result in a situation where the delayed wakeup irq_work can
accidentally dereference EVENT_TOMBSTONE on:
CPU 0 CPU 1
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 882db7b..e0ca4a8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -208,7 +208,6 @@ static void perf_ctx_unlock(struct perf_cpu_context *cpuctx,
}
#define TASK_TOMBSTONE ((void *)-1L)
-#define EVENT_TOMBSTONE ((void *)-1L)
static bool is_kernel_event(struct perf_event *event)
{
@@ -2338,12 +2337,6 @@ static void perf_child_detach(struct perf_event *event)
sync_child_event(event);
list_del_init(&event->child_list);
- /*
- * Cannot set to NULL, as that would confuse the situation vs
- * not being a child event. See for example unaccount_event().
- */
- event->parent = EVENT_TOMBSTONE;
- put_event(parent_event);
}
static bool is_orphaned_event(struct perf_event *event)
@@ -5705,7 +5698,7 @@ static void put_event(struct perf_event *event)
_free_event(event);
/* Matches the refcount bump in inherit_event() */
- if (parent && parent != EVENT_TOMBSTONE)
+ if (parent)
put_event(parent);
}
@@ -9998,7 +9991,7 @@ void perf_event_text_poke(const void *addr, const void *old_bytes,
void perf_event_itrace_started(struct perf_event *event)
{
- event->attach_state |= PERF_ATTACH_ITRACE;
+ WRITE_ONCE(event->attach_state, event->attach_state | PERF_ATTACH_ITRACE);
}
static void perf_log_itrace_start(struct perf_event *event)
@@ -13922,10 +13915,7 @@ perf_event_exit_event(struct perf_event *event,
{
struct perf_event *parent_event = event->parent;
unsigned long detach_flags = DETACH_EXIT;
- bool is_child = !!parent_event;
-
- if (parent_event == EVENT_TOMBSTONE)
- parent_event = NULL;
+ unsigned int attach_state;
if (parent_event) {
/*
@@ -13942,6 +13932,8 @@ perf_event_exit_event(struct perf_event *event,
*/
detach_flags |= DETACH_GROUP | DETACH_CHILD;
mutex_lock(&parent_event->child_mutex);
+ /* PERF_ATTACH_ITRACE might be set concurrently */
+ attach_state = READ_ONCE(event->attach_state);
}
if (revoke)
@@ -13951,18 +13943,25 @@ perf_event_exit_event(struct perf_event *event,
/*
* Child events can be freed.
*/
- if (is_child) {
- if (parent_event) {
- mutex_unlock(&parent_event->child_mutex);
+ if (parent_event) {
+ mutex_unlock(&parent_event->child_mutex);
+
+ /*
+ * Match the refcount initialization. Make sure it doesn't happen
+ * twice if pmu_detach_event() calls it on an already exited task.
+ */
+ if (attach_state & PERF_ATTACH_CHILD) {
/*
* Kick perf_poll() for is_event_hup();
*/
perf_event_wakeup(parent_event);
/*
* pmu_detach_event() will have an extra refcount.
+ * perf_pending_task() might have one too.
*/
put_event(event);
}
+
return;
}Le Thu, May 08, 2025 at 10:34:29AM -0000, tip-bot2 for Frederic Weisbecker a écrit :
> The following commit has been merged into the perf/core branch of tip:
>
> Commit-ID: 88d51e795539acd08bce028eff3aa78748b847a8
> Gitweb: https://git.kernel.org/tip/88d51e795539acd08bce028eff3aa78748b847a8
> Author: Frederic Weisbecker <frederic@kernel.org>
> AuthorDate: Mon, 28 Apr 2025 13:11:47 +02:00
> Committer: Peter Zijlstra <peterz@infradead.org>
> CommitterDate: Fri, 02 May 2025 12:40:40 +02:00
>
> perf: Fix irq work dereferencing garbage
>
> The following commit:
>
> da916e96e2de ("perf: Make perf_pmu_unregister() useable")
>
> has introduced two significant event's parent lifecycle changes:
>
> 1) An event that has exited now has EVENT_TOMBSTONE as a parent.
> This can result in a situation where the delayed wakeup irq_work can
> accidentally dereference EVENT_TOMBSTONE on:
>
> CPU 0 CPU 1
This is missing the most important (and boring) part of the changelog :-)
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
And I suspect some automatic check will also yell at some point at missing Sob.
Thanks.
--
Frederic Weisbecker
SUSE Labs
On Thu, May 08, 2025 at 09:11:23PM +0200, Frederic Weisbecker wrote:
> Le Thu, May 08, 2025 at 10:34:29AM -0000, tip-bot2 for Frederic Weisbecker a écrit :
> > The following commit has been merged into the perf/core branch of tip:
> >
> > Commit-ID: 88d51e795539acd08bce028eff3aa78748b847a8
> > Gitweb: https://git.kernel.org/tip/88d51e795539acd08bce028eff3aa78748b847a8
> > Author: Frederic Weisbecker <frederic@kernel.org>
> > AuthorDate: Mon, 28 Apr 2025 13:11:47 +02:00
> > Committer: Peter Zijlstra <peterz@infradead.org>
> > CommitterDate: Fri, 02 May 2025 12:40:40 +02:00
> >
> > perf: Fix irq work dereferencing garbage
> >
> > The following commit:
> >
> > da916e96e2de ("perf: Make perf_pmu_unregister() useable")
> >
> > has introduced two significant event's parent lifecycle changes:
> >
> > 1) An event that has exited now has EVENT_TOMBSTONE as a parent.
> > This can result in a situation where the delayed wakeup irq_work can
> > accidentally dereference EVENT_TOMBSTONE on:
> >
> > CPU 0 CPU 1
>
> This is missing the most important (and boring) part of the changelog :-)
>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> And I suspect some automatic check will also yell at some point at missing Sob.
Doh, script fail :/
Let me go fix.
Le Thu, May 08, 2025 at 09:46:05PM +0200, Peter Zijlstra a écrit :
> On Thu, May 08, 2025 at 09:11:23PM +0200, Frederic Weisbecker wrote:
> > Le Thu, May 08, 2025 at 10:34:29AM -0000, tip-bot2 for Frederic Weisbecker a écrit :
> > > The following commit has been merged into the perf/core branch of tip:
> > >
> > > Commit-ID: 88d51e795539acd08bce028eff3aa78748b847a8
> > > Gitweb: https://git.kernel.org/tip/88d51e795539acd08bce028eff3aa78748b847a8
> > > Author: Frederic Weisbecker <frederic@kernel.org>
> > > AuthorDate: Mon, 28 Apr 2025 13:11:47 +02:00
> > > Committer: Peter Zijlstra <peterz@infradead.org>
> > > CommitterDate: Fri, 02 May 2025 12:40:40 +02:00
> > >
> > > perf: Fix irq work dereferencing garbage
> > >
> > > The following commit:
> > >
> > > da916e96e2de ("perf: Make perf_pmu_unregister() useable")
> > >
> > > has introduced two significant event's parent lifecycle changes:
> > >
> > > 1) An event that has exited now has EVENT_TOMBSTONE as a parent.
> > > This can result in a situation where the delayed wakeup irq_work can
> > > accidentally dereference EVENT_TOMBSTONE on:
> > >
> > > CPU 0 CPU 1
> >
> > This is missing the most important (and boring) part of the changelog :-)
> >
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >
> > And I suspect some automatic check will also yell at some point at missing Sob.
>
> Doh, script fail :/
>
> Let me go fix.
Looks good, thanks!
--
Frederic Weisbecker
SUSE Labs
© 2016 - 2025 Red Hat, Inc.