perf: Break deadlock involving exec_update_mutex
authorpeterz@infradead.org <peterz@infradead.org>
Fri, 28 Aug 2020 12:37:20 +0000 (14:37 +0200)
committerPeter Zijlstra <peterz@infradead.org>
Wed, 9 Dec 2020 16:08:57 +0000 (17:08 +0100)
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+db9cdf3dd1f64252c6ef@syzkaller.appspotmail.com
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
kernel/events/core.c

index a21b0be2f22cacc4cbfd502e0aa4dac841cc9c66..19ae6c931c5249b761763dfc53a763a413c0b2a3 100644 (file)
@@ -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);