[lkml]   [2018]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
Subject[PATCH AUTOSEL 4.9 81/98] perf/core: Fix locking for children siblings group read
From: Jiri Olsa <>

[ Upstream commit 2aeb1883547626d82c597cce2c99f0b9c62e2425 ]

We're missing ctx lock when iterating children siblings
within the perf_read path for group reading. Following
race and crash can happen:

User space doing read syscall on event group leader:

lock event->ctx->mutex
lock leader->child_mutex
list_for_each_entry(sub, &leader->sibling_list, group_entry)

----> sub might be invalid at this point, because it could
get removed via perf_event_exit_task_context in T2

Child exiting and cleaning up its events:

lock ctx->mutex
list_for_each_entry_safe(child_event, next, &child_ctx->event_list,...
lock ctx->lock
unlock ctx->lock

----> child is removed from sibling_list without any sync
with T1 path above


Before the child is removed from the leader's child_list,
(and thus is omitted from perf_read_group processing), we
need to ensure that perf_read_group touches child's
siblings under its ctx->lock.

Peter further notes:

| One additional note; this bug got exposed by commit:
| ba5213ae6b88 ("perf/core: Correct event creation with PERF_FORMAT_GROUP")
| which made it possible to actually trigger this code-path.

Tested-by: Andi Kleen <>
Signed-off-by: Jiri Olsa <>
Acked-by: Peter Zijlstra (Intel) <>
Cc: Alexander Shishkin <>
Cc: Arnaldo Carvalho de Melo <>
Cc: Jiri Olsa <>
Cc: Linus Torvalds <>
Cc: Peter Zijlstra <>
Cc: Peter Zijlstra <>
Cc: Thomas Gleixner <>
Fixes: ba5213ae6b88 ("perf/core: Correct event creation with PERF_FORMAT_GROUP")
Signed-off-by: Ingo Molnar <>
Signed-off-by: Sasha Levin <>
kernel/events/core.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 95bd00d9f2c3..06b359af4322 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4331,7 +4331,9 @@ EXPORT_SYMBOL_GPL(perf_event_read_value);
static int __perf_read_group_add(struct perf_event *leader,
u64 read_format, u64 *values)
+ struct perf_event_context *ctx = leader->ctx;
struct perf_event *sub;
+ unsigned long flags;
int n = 1; /* skip @nr */
int ret;

@@ -4361,12 +4363,15 @@ static int __perf_read_group_add(struct perf_event *leader,
if (read_format & PERF_FORMAT_ID)
values[n++] = primary_event_id(leader);

+ raw_spin_lock_irqsave(&ctx->lock, flags);
list_for_each_entry(sub, &leader->sibling_list, group_entry) {
values[n++] += perf_event_count(sub);
if (read_format & PERF_FORMAT_ID)
values[n++] = primary_event_id(sub);

+ raw_spin_unlock_irqrestore(&ctx->lock, flags);
return 0;

 \ /
  Last update: 2018-10-25 16:38    [W:0.291 / U:4.384 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site