Commit 78af4dc9 authored by peterz@infradead.org's avatar peterz@infradead.org Committed by Peter Zijlstra
Browse files

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: default avatar <syzbot+db9cdf3dd1f64252c6ef@syzkaller.appspotmail.com>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
parent e6e4f42e

kernel/events/core.c

+23 −23
Original line number Diff line number Diff line
@@ -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 @@ SYSCALL_DEFINE5(perf_event_open, 
	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 @@ SYSCALL_DEFINE5(perf_event_open, 
	 */

	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);