[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <174670046919.406.15885032121099672652.tip-bot2@tip-bot2>
Date: Thu, 08 May 2025 10:34:29 -0000
From: "tip-bot2 for Frederic Weisbecker" <tip-bot2@...utronix.de>
To: linux-tip-commits@...r.kernel.org
Cc: "Peter Zijlstra (Intel)" <peterz@...radead.org>, x86@...nel.org,
linux-kernel@...r.kernel.org
Subject: [tip: perf/core] perf: Fix irq work dereferencing garbage
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@...nel.org>
AuthorDate: Mon, 28 Apr 2025 13:11:47 +02:00
Committer: Peter Zijlstra <peterz@...radead.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@...radead.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;
}
Powered by blists - more mailing lists