diff options
author | Peter Zijlstra <peterz@infradead.org> | 2015-12-10 14:57:40 -0500 |
---|---|---|
committer | Ingo Molnar <mingo@kernel.org> | 2016-01-06 04:52:38 -0500 |
commit | c127449944659543e5e2423002f08f0af98dba5c (patch) | |
tree | bee5cbca5a14a438e4669fd61c161b0cddfce878 | |
parent | 2d2e7ac14a1f98922e100054bf62715439440f1a (diff) |
perf: Fix race in perf_event_exec()
I managed to tickle this warning:
[ 2338.884942] ------------[ cut here ]------------
[ 2338.890112] WARNING: CPU: 13 PID: 35162 at ../kernel/events/core.c:2702 task_ctx_sched_out+0x6b/0x80()
[ 2338.900504] Modules linked in:
[ 2338.903933] CPU: 13 PID: 35162 Comm: bash Not tainted 4.4.0-rc4-dirty #244
[ 2338.911610] Hardware name: Intel Corporation S2600GZ/S2600GZ, BIOS SE5C600.86B.02.02.0002.122320131210 12/23/2013
[ 2338.923071] ffffffff81f1468e ffff8807c6457cb8 ffffffff815c680c 0000000000000000
[ 2338.931382] ffff8807c6457cf0 ffffffff810c8a56 ffffe8ffff8c1bd0 ffff8808132ed400
[ 2338.939678] 0000000000000286 ffff880813170380 ffff8808132ed400 ffff8807c6457d00
[ 2338.947987] Call Trace:
[ 2338.950726] [<ffffffff815c680c>] dump_stack+0x4e/0x82
[ 2338.956474] [<ffffffff810c8a56>] warn_slowpath_common+0x86/0xc0
[ 2338.963195] [<ffffffff810c8b4a>] warn_slowpath_null+0x1a/0x20
[ 2338.969720] [<ffffffff811a49cb>] task_ctx_sched_out+0x6b/0x80
[ 2338.976244] [<ffffffff811a62d2>] perf_event_exec+0xe2/0x180
[ 2338.982575] [<ffffffff8121fb6f>] setup_new_exec+0x6f/0x1b0
[ 2338.988810] [<ffffffff8126de83>] load_elf_binary+0x393/0x1660
[ 2338.995339] [<ffffffff811dc772>] ? get_user_pages+0x52/0x60
[ 2339.001669] [<ffffffff8121e297>] search_binary_handler+0x97/0x200
[ 2339.008581] [<ffffffff8121f8b3>] do_execveat_common.isra.33+0x543/0x6e0
[ 2339.016072] [<ffffffff8121fcea>] SyS_execve+0x3a/0x50
[ 2339.021819] [<ffffffff819fc165>] stub_execve+0x5/0x5
[ 2339.027469] [<ffffffff819fbeb2>] ? entry_SYSCALL_64_fastpath+0x12/0x71
[ 2339.034860] ---[ end trace ee1337c59a0ddeac ]---
Which is a WARN_ON_ONCE() indicating that cpuctx->task_ctx is not
what we expected it to be.
This is because context switches can swap the task_struct::perf_event_ctxp[]
pointer around. Therefore you have to either disable preemption when looking
at current, or hold ctx->lock.
Fix perf_event_enable_on_exec(), it loads current->perf_event_ctxp[]
before disabling interrupts, therefore a preemption in the right place
can swap contexts around and we're using the wrong one.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Potapenko <glider@google.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: syzkaller <syzkaller@googlegroups.com>
Link: http://lkml.kernel.org/r/20151210195740.GG6357@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
-rw-r--r-- | kernel/events/core.c | 15 |
1 files changed, 5 insertions, 10 deletions
diff --git a/kernel/events/core.c b/kernel/events/core.c index 39cf4a40aa4c..fd7de0418fbe 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c | |||
@@ -3154,15 +3154,16 @@ static int event_enable_on_exec(struct perf_event *event, | |||
3154 | * Enable all of a task's events that have been marked enable-on-exec. | 3154 | * Enable all of a task's events that have been marked enable-on-exec. |
3155 | * This expects task == current. | 3155 | * This expects task == current. |
3156 | */ | 3156 | */ |
3157 | static void perf_event_enable_on_exec(struct perf_event_context *ctx) | 3157 | static void perf_event_enable_on_exec(int ctxn) |
3158 | { | 3158 | { |
3159 | struct perf_event_context *clone_ctx = NULL; | 3159 | struct perf_event_context *ctx, *clone_ctx = NULL; |
3160 | struct perf_event *event; | 3160 | struct perf_event *event; |
3161 | unsigned long flags; | 3161 | unsigned long flags; |
3162 | int enabled = 0; | 3162 | int enabled = 0; |
3163 | int ret; | 3163 | int ret; |
3164 | 3164 | ||
3165 | local_irq_save(flags); | 3165 | local_irq_save(flags); |
3166 | ctx = current->perf_event_ctxp[ctxn]; | ||
3166 | if (!ctx || !ctx->nr_events) | 3167 | if (!ctx || !ctx->nr_events) |
3167 | goto out; | 3168 | goto out; |
3168 | 3169 | ||
@@ -3205,17 +3206,11 @@ out: | |||
3205 | 3206 | ||
3206 | void perf_event_exec(void) | 3207 | void perf_event_exec(void) |
3207 | { | 3208 | { |
3208 | struct perf_event_context *ctx; | ||
3209 | int ctxn; | 3209 | int ctxn; |
3210 | 3210 | ||
3211 | rcu_read_lock(); | 3211 | rcu_read_lock(); |
3212 | for_each_task_context_nr(ctxn) { | 3212 | for_each_task_context_nr(ctxn) |
3213 | ctx = current->perf_event_ctxp[ctxn]; | 3213 | perf_event_enable_on_exec(ctxn); |
3214 | if (!ctx) | ||
3215 | continue; | ||
3216 | |||
3217 | perf_event_enable_on_exec(ctx); | ||
3218 | } | ||
3219 | rcu_read_unlock(); | 3214 | rcu_read_unlock(); |
3220 | } | 3215 | } |
3221 | 3216 | ||