diff options
| author | Peter Zijlstra <peterz@infradead.org> | 2015-05-01 10:08:46 -0400 |
|---|---|---|
| committer | Ingo Molnar <mingo@kernel.org> | 2015-05-08 05:59:40 -0400 |
| commit | 8b10c5e2b59ef2a80a07ab594a3b4987a4676211 (patch) | |
| tree | 8d4f8d0b899761cc685c55ef722fb65368e8d044 /kernel | |
| parent | 3e0283a53f7d2f2dae7bc4aa7f3104cb5988018f (diff) | |
perf: Annotate inherited event ctx->mutex recursion
While fuzzing Sasha tripped over another ctx->mutex recursion lockdep
splat. Annotate this.
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Diffstat (limited to 'kernel')
| -rw-r--r-- | kernel/events/core.c | 41 |
1 files changed, 33 insertions, 8 deletions
diff --git a/kernel/events/core.c b/kernel/events/core.c index 81aa3a4ece9f..1a3bf48743ce 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c | |||
| @@ -913,10 +913,30 @@ static void put_ctx(struct perf_event_context *ctx) | |||
| 913 | * Those places that change perf_event::ctx will hold both | 913 | * Those places that change perf_event::ctx will hold both |
| 914 | * perf_event_ctx::mutex of the 'old' and 'new' ctx value. | 914 | * perf_event_ctx::mutex of the 'old' and 'new' ctx value. |
| 915 | * | 915 | * |
| 916 | * Lock ordering is by mutex address. There is one other site where | 916 | * Lock ordering is by mutex address. There are two other sites where |
| 917 | * perf_event_context::mutex nests and that is put_event(). But remember that | 917 | * perf_event_context::mutex nests and those are: |
| 918 | * that is a parent<->child context relation, and migration does not affect | 918 | * |
| 919 | * children, therefore these two orderings should not interact. | 919 | * - perf_event_exit_task_context() [ child , 0 ] |
| 920 | * __perf_event_exit_task() | ||
| 921 | * sync_child_event() | ||
| 922 | * put_event() [ parent, 1 ] | ||
| 923 | * | ||
| 924 | * - perf_event_init_context() [ parent, 0 ] | ||
| 925 | * inherit_task_group() | ||
| 926 | * inherit_group() | ||
| 927 | * inherit_event() | ||
| 928 | * perf_event_alloc() | ||
| 929 | * perf_init_event() | ||
| 930 | * perf_try_init_event() [ child , 1 ] | ||
| 931 | * | ||
| 932 | * While it appears there is an obvious deadlock here -- the parent and child | ||
| 933 | * nesting levels are inverted between the two. This is in fact safe because | ||
| 934 | * life-time rules separate them. That is an exiting task cannot fork, and a | ||
| 935 | * spawning task cannot (yet) exit. | ||
| 936 | * | ||
| 937 | * But remember that that these are parent<->child context relations, and | ||
| 938 | * migration does not affect children, therefore these two orderings should not | ||
| 939 | * interact. | ||
| 920 | * | 940 | * |
| 921 | * The change in perf_event::ctx does not affect children (as claimed above) | 941 | * The change in perf_event::ctx does not affect children (as claimed above) |
| 922 | * because the sys_perf_event_open() case will install a new event and break | 942 | * because the sys_perf_event_open() case will install a new event and break |
| @@ -3657,9 +3677,6 @@ static void perf_remove_from_owner(struct perf_event *event) | |||
| 3657 | } | 3677 | } |
| 3658 | } | 3678 | } |
| 3659 | 3679 | ||
| 3660 | /* | ||
| 3661 | * Called when the last reference to the file is gone. | ||
| 3662 | */ | ||
| 3663 | static void put_event(struct perf_event *event) | 3680 | static void put_event(struct perf_event *event) |
| 3664 | { | 3681 | { |
| 3665 | struct perf_event_context *ctx; | 3682 | struct perf_event_context *ctx; |
| @@ -3697,6 +3714,9 @@ int perf_event_release_kernel(struct perf_event *event) | |||
| 3697 | } | 3714 | } |
| 3698 | EXPORT_SYMBOL_GPL(perf_event_release_kernel); | 3715 | EXPORT_SYMBOL_GPL(perf_event_release_kernel); |
| 3699 | 3716 | ||
| 3717 | /* | ||
| 3718 | * Called when the last reference to the file is gone. | ||
| 3719 | */ | ||
| 3700 | static int perf_release(struct inode *inode, struct file *file) | 3720 | static int perf_release(struct inode *inode, struct file *file) |
| 3701 | { | 3721 | { |
| 3702 | put_event(file->private_data); | 3722 | put_event(file->private_data); |
| @@ -7364,7 +7384,12 @@ static int perf_try_init_event(struct pmu *pmu, struct perf_event *event) | |||
| 7364 | return -ENODEV; | 7384 | return -ENODEV; |
| 7365 | 7385 | ||
| 7366 | if (event->group_leader != event) { | 7386 | if (event->group_leader != event) { |
| 7367 | ctx = perf_event_ctx_lock(event->group_leader); | 7387 | /* |
| 7388 | * This ctx->mutex can nest when we're called through | ||
| 7389 | * inheritance. See the perf_event_ctx_lock_nested() comment. | ||
| 7390 | */ | ||
| 7391 | ctx = perf_event_ctx_lock_nested(event->group_leader, | ||
| 7392 | SINGLE_DEPTH_NESTING); | ||
| 7368 | BUG_ON(!ctx); | 7393 | BUG_ON(!ctx); |
| 7369 | } | 7394 | } |
| 7370 | 7395 | ||
