The following commit has been merged into the perf/core branch of tip:

Commit-ID:     78af4dc949daaa37b3fcd5f348f373085b4e858f
Gitweb:        
https://git.kernel.org/tip/78af4dc949daaa37b3fcd5f348f373085b4e858f
Author:        pet...@infradead.org <pet...@infradead.org>
AuthorDate:    Fri, 28 Aug 2020 14:37:20 +02:00
Committer:     Peter Zijlstra <pet...@infradead.org>
CommitterDate: Wed, 09 Dec 2020 17:08:57 +01:00

perf: Break deadlock involving exec_update_mutex

Syzbot reported a lock inversion involving perf. The sore point being
perf holding exec_update_mutex() for a very long time, specifically
across a whole bunch of filesystem ops in pmu::event_init() (uprobes)
and anon_inode_getfile().

This then inverts against procfs code trying to take
exec_update_mutex.

Move the permission checks later, such that we need to hold the mutex
over less code.

Reported-by: syzbot+db9cdf3dd1f64252c...@syzkaller.appspotmail.com
Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>
---
 kernel/events/core.c | 46 +++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index a21b0be..19ae6c9 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11832,24 +11832,6 @@ SYSCALL_DEFINE5(perf_event_open,
                goto err_task;
        }
 
-       if (task) {
-               err = 
mutex_lock_interruptible(&task->signal->exec_update_mutex);
-               if (err)
-                       goto err_task;
-
-               /*
-                * Preserve ptrace permission check for backwards compatibility.
-                *
-                * We must hold exec_update_mutex across this and any potential
-                * perf_install_in_context() call for this new event to
-                * serialize against exec() altering our credentials (and the
-                * perf_event_exit_task() that could imply).
-                */
-               err = -EACCES;
-               if (!perfmon_capable() && !ptrace_may_access(task, 
PTRACE_MODE_READ_REALCREDS))
-                       goto err_cred;
-       }
-
        if (flags & PERF_FLAG_PID_CGROUP)
                cgroup_fd = pid;
 
@@ -11857,7 +11839,7 @@ SYSCALL_DEFINE5(perf_event_open,
                                 NULL, NULL, cgroup_fd);
        if (IS_ERR(event)) {
                err = PTR_ERR(event);
-               goto err_cred;
+               goto err_task;
        }
 
        if (is_sampling_event(event)) {
@@ -11976,6 +11958,24 @@ SYSCALL_DEFINE5(perf_event_open,
                goto err_context;
        }
 
+       if (task) {
+               err = 
mutex_lock_interruptible(&task->signal->exec_update_mutex);
+               if (err)
+                       goto err_file;
+
+               /*
+                * Preserve ptrace permission check for backwards compatibility.
+                *
+                * We must hold exec_update_mutex across this and any potential
+                * perf_install_in_context() call for this new event to
+                * serialize against exec() altering our credentials (and the
+                * perf_event_exit_task() that could imply).
+                */
+               err = -EACCES;
+               if (!perfmon_capable() && !ptrace_may_access(task, 
PTRACE_MODE_READ_REALCREDS))
+                       goto err_cred;
+       }
+
        if (move_group) {
                gctx = __perf_event_ctx_lock_double(group_leader, ctx);
 
@@ -12151,7 +12151,10 @@ err_locked:
        if (move_group)
                perf_event_ctx_unlock(group_leader, gctx);
        mutex_unlock(&ctx->mutex);
-/* err_file: */
+err_cred:
+       if (task)
+               mutex_unlock(&task->signal->exec_update_mutex);
+err_file:
        fput(event_file);
 err_context:
        perf_unpin_context(ctx);
@@ -12163,9 +12166,6 @@ err_alloc:
         */
        if (!event_file)
                free_event(event);
-err_cred:
-       if (task)
-               mutex_unlock(&task->signal->exec_update_mutex);
 err_task:
        if (task)
                put_task_struct(task);

Reply via email to