exec: Transform exec_update_mutex into a rw_semaphore

[ Upstream commit f7cfd871ae0c5008d94b6f66834e7845caa93c15 ]

Recently syzbot reported[0] that there is a deadlock amongst the users
of exec_update_mutex.  The problematic lock ordering found by lockdep
was:

   perf_event_open  (exec_update_mutex -> ovl_i_mutex)
   chown            (ovl_i_mutex       -> sb_writes)
   sendfile         (sb_writes         -> p->lock)
     by reading from a proc file and writing to overlayfs
   proc_pid_syscall (p->lock           -> exec_update_mutex)

While looking at possible solutions it occured to me that all of the
users and possible users involved only wanted to state of the given
process to remain the same.  They are all readers.  The only writer is
exec.

There is no reason for readers to block on each other.  So fix
this deadlock by transforming exec_update_mutex into a rw_semaphore
named exec_update_lock that only exec takes for writing.

Cc: Jann Horn <[email protected]>
Cc: Vasiliy Kulikov <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Bernd Edlinger <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Christopher Yeoh <[email protected]>
Cc: Cyrill Gorcunov <[email protected]>
Cc: Sargun Dhillon <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Fixes: eea9673250db ("exec: Add exec_update_mutex to replace cred_guard_mutex")
[0] https://lkml.kernel.org/r/[email protected]
Reported-by: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Eric W. Biederman <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
This commit is contained in:
Eric W. Biederman 2020-12-03 14:12:00 -06:00 • committed by Greg Kroah-Hartman
commit 117433236a
8 changed files with 44 additions and 43 deletions

View file

@ -1254,7 +1254,7 @@ static void put_ctx(struct perf_event_context *ctx)
* function.
*
* Lock order:
* exec_update_mutex
* exec_update_lock
* task_struct::perf_event_mutex
* perf_event_context::mutex
* perf_event::child_mutex;
@ -11128,14 +11128,14 @@ SYSCALL_DEFINE5(perf_event_open,
}
if (task) {
err = mutex_lock_interruptible(&task->signal->exec_update_mutex);
err = down_read_interruptible(&task->signal->exec_update_lock);
if (err)
goto err_file;
/*
* Preserve ptrace permission check for backwards compatibility.
*
* We must hold exec_update_mutex across this and any potential
* We must hold exec_update_lock 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).
@ -11298,7 +11298,7 @@ SYSCALL_DEFINE5(perf_event_open,
mutex_unlock(&ctx->mutex);
if (task) {
mutex_unlock(&task->signal->exec_update_mutex);
up_read(&task->signal->exec_update_lock);
put_task_struct(task);
}
@ -11322,7 +11322,7 @@ err_locked:
mutex_unlock(&ctx->mutex);
err_cred:
if (task)
mutex_unlock(&task->signal->exec_update_mutex);
up_read(&task->signal->exec_update_lock);
err_file:
fput(event_file);
err_context:
@ -11639,7 +11639,7 @@ static void perf_event_exit_task_context(struct task_struct *child, int ctxn)
/*
* When a child task exits, feed back event values to parent events.
*
* Can be called with exec_update_mutex held when called from
* Can be called with exec_update_lock held when called from
* install_exec_creds().
*/
void perf_event_exit_task(struct task_struct *child)