Skip to content

Commit c418d8b

Browse files
Thaumyingomolnar
authored andcommitted
perf/core: Fix missing read event generation on task exit
For events with inherit_stat enabled, a "read" event will be generated to collect per task event counts on task exit. The call chain is as follows: do_exit -> perf_event_exit_task -> perf_event_exit_task_context -> perf_event_exit_event -> perf_remove_from_context -> perf_child_detach -> sync_child_event -> perf_event_read_event However, the child event context detaches the task too early in perf_event_exit_task_context, which causes sync_child_event to never generate the read event in this case, since child_event->ctx->task is always set to TASK_TOMBSTONE. Fix that by moving context lock section backward to ensure ctx->task is not set to TASK_TOMBSTONE before generating the read event. Because perf_event_free_task calls perf_event_exit_task_context with exit = false to tear down all child events from the context, and the task never lived, accessing the task PID can lead to a use-after-free. To fix that, let sync_child_event read task from argument and move the call to the only place it should be triggered to avoid the effect of setting ctx->task to TASK_TOMESTONE, and add a task parameter to perf_event_exit_event to trigger the sync_child_event properly when needed. This bug can be reproduced by running "perf record -s" and attaching to any program that generates perf events in its child tasks. If we check the result with "perf report -T", the last line of the report will leave an empty table like "# PID TID", which is expected to contain the per-task event counts by design. Fixes: ef54c1a ("perf: Rework perf_event_exit_event()") Signed-off-by: Thaumy Cheng <thaumy.love@gmail.com> Signed-off-by: Ingo Molnar <mingo@kernel.org> Acked-by: Peter Zijlstra <peterz@infradead.org> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Ian Rogers <irogers@google.com> Cc: James Clark <james.clark@linaro.org> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: linux-perf-users@vger.kernel.org Link: https://patch.msgid.link/20251209041600.963586-1-thaumy.love@gmail.com
1 parent 0143928 commit c418d8b

1 file changed

Lines changed: 12 additions & 10 deletions

File tree

kernel/events/core.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2317,8 +2317,6 @@ static void perf_group_detach(struct perf_event *event)
23172317
perf_event__header_size(leader);
23182318
}
23192319

2320-
static void sync_child_event(struct perf_event *child_event);
2321-
23222320
static void perf_child_detach(struct perf_event *event)
23232321
{
23242322
struct perf_event *parent_event = event->parent;
@@ -2337,7 +2335,6 @@ static void perf_child_detach(struct perf_event *event)
23372335
lockdep_assert_held(&parent_event->child_mutex);
23382336
*/
23392337

2340-
sync_child_event(event);
23412338
list_del_init(&event->child_list);
23422339
}
23432340

@@ -4588,6 +4585,7 @@ static void perf_event_enable_on_exec(struct perf_event_context *ctx)
45884585
static void perf_remove_from_owner(struct perf_event *event);
45894586
static void perf_event_exit_event(struct perf_event *event,
45904587
struct perf_event_context *ctx,
4588+
struct task_struct *task,
45914589
bool revoke);
45924590

45934591
/*
@@ -4615,7 +4613,7 @@ static void perf_event_remove_on_exec(struct perf_event_context *ctx)
46154613

46164614
modified = true;
46174615

4618-
perf_event_exit_event(event, ctx, false);
4616+
perf_event_exit_event(event, ctx, ctx->task, false);
46194617
}
46204618

46214619
raw_spin_lock_irqsave(&ctx->lock, flags);
@@ -12518,7 +12516,7 @@ static void __pmu_detach_event(struct pmu *pmu, struct perf_event *event,
1251812516
/*
1251912517
* De-schedule the event and mark it REVOKED.
1252012518
*/
12521-
perf_event_exit_event(event, ctx, true);
12519+
perf_event_exit_event(event, ctx, ctx->task, true);
1252212520

1252312521
/*
1252412522
* All _free_event() bits that rely on event->pmu:
@@ -14075,14 +14073,13 @@ void perf_pmu_migrate_context(struct pmu *pmu, int src_cpu, int dst_cpu)
1407514073
}
1407614074
EXPORT_SYMBOL_GPL(perf_pmu_migrate_context);
1407714075

14078-
static void sync_child_event(struct perf_event *child_event)
14076+
static void sync_child_event(struct perf_event *child_event,
14077+
struct task_struct *task)
1407914078
{
1408014079
struct perf_event *parent_event = child_event->parent;
1408114080
u64 child_val;
1408214081

1408314082
if (child_event->attr.inherit_stat) {
14084-
struct task_struct *task = child_event->ctx->task;
14085-
1408614083
if (task && task != TASK_TOMBSTONE)
1408714084
perf_event_read_event(child_event, task);
1408814085
}
@@ -14101,7 +14098,9 @@ static void sync_child_event(struct perf_event *child_event)
1410114098

1410214099
static void
1410314100
perf_event_exit_event(struct perf_event *event,
14104-
struct perf_event_context *ctx, bool revoke)
14101+
struct perf_event_context *ctx,
14102+
struct task_struct *task,
14103+
bool revoke)
1410514104
{
1410614105
struct perf_event *parent_event = event->parent;
1410714106
unsigned long detach_flags = DETACH_EXIT;
@@ -14124,6 +14123,9 @@ perf_event_exit_event(struct perf_event *event,
1412414123
mutex_lock(&parent_event->child_mutex);
1412514124
/* PERF_ATTACH_ITRACE might be set concurrently */
1412614125
attach_state = READ_ONCE(event->attach_state);
14126+
14127+
if (attach_state & PERF_ATTACH_CHILD)
14128+
sync_child_event(event, task);
1412714129
}
1412814130

1412914131
if (revoke)
@@ -14215,7 +14217,7 @@ static void perf_event_exit_task_context(struct task_struct *task, bool exit)
1421514217
perf_event_task(task, ctx, 0);
1421614218

1421714219
list_for_each_entry_safe(child_event, next, &ctx->event_list, event_entry)
14218-
perf_event_exit_event(child_event, ctx, false);
14220+
perf_event_exit_event(child_event, ctx, exit ? task : NULL, false);
1421914221

1422014222
mutex_unlock(&ctx->mutex);
1422114223

0 commit comments

Comments
 (0)