On Tue, Mar 10, 2020 at 06:45:32PM +0100, Bernd Edlinger wrote:
This changes lock_trace to use the new exec_update_mutex instead of cred_guard_mutex.
This fixes possible deadlocks when the trace is accessing /proc/$pid/stack for instance.
This should be safe, as the credentials are only used for reading, and task->mm is updated on execve under the new exec_update_mutex.
Signed-off-by: Bernd Edlinger bernd.edlinger@hotmail.de
I have the same question here as in 3/4. I should probably rescind my Reviewed-by until I'm convinced about the security-safety of this -- why is this not a race against cred changes?
-Kees
fs/proc/base.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c index ebea950..4fdfe4f 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -403,11 +403,11 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns, static int lock_trace(struct task_struct *task) {
- int err = mutex_lock_killable(&task->signal->cred_guard_mutex);
- int err = mutex_lock_killable(&task->signal->exec_update_mutex); if (err) return err; if (!ptrace_may_access(task, PTRACE_MODE_ATTACH_FSCREDS)) {
mutex_unlock(&task->signal->cred_guard_mutex);
return -EPERM; } return 0;mutex_unlock(&task->signal->exec_update_mutex);
@@ -415,7 +415,7 @@ static int lock_trace(struct task_struct *task) static void unlock_trace(struct task_struct *task) {
- mutex_unlock(&task->signal->cred_guard_mutex);
- mutex_unlock(&task->signal->exec_update_mutex);
}
#ifdef CONFIG_STACKTRACE
1.9.1