On 3/10/20 4:13 PM, Eric W. Biederman wrote: > Bernd Edlinger <bernd.edlinger@xxxxxxxxxx> writes: > >> This fixes a deadlock in the tracer when tracing a multi-threaded >> application that calls execve while more than one thread are running. >> >> I observed that when running strace on the gcc test suite, it always >> blocks after a while, when expect calls execve, because other threads >> have to be terminated. They send ptrace events, but the strace is no >> longer able to respond, since it is blocked in vm_access. >> >> The deadlock is always happening when strace needs to access the >> tracees process mmap, while another thread in the tracee starts to >> execve a child process, but that cannot continue until the >> PTRACE_EVENT_EXIT is handled and the WIFEXITED event is received: > > Overall this looks good. Mind if I change the subject to: > "exec: Fix a deadlock in strace" ? > Sure, go ahead. Thanks Bernd. > Eric > > >> >> strace D 0 30614 30584 0x00000000 >> Call Trace: >> __schedule+0x3ce/0x6e0 >> schedule+0x5c/0xd0 >> schedule_preempt_disabled+0x15/0x20 >> __mutex_lock.isra.13+0x1ec/0x520 >> __mutex_lock_killable_slowpath+0x13/0x20 >> mutex_lock_killable+0x28/0x30 >> mm_access+0x27/0xa0 >> process_vm_rw_core.isra.3+0xff/0x550 >> process_vm_rw+0xdd/0xf0 >> __x64_sys_process_vm_readv+0x31/0x40 >> do_syscall_64+0x64/0x220 >> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> >> expect D 0 31933 30876 0x80004003 >> Call Trace: >> __schedule+0x3ce/0x6e0 >> schedule+0x5c/0xd0 >> flush_old_exec+0xc4/0x770 >> load_elf_binary+0x35a/0x16c0 >> search_binary_handler+0x97/0x1d0 >> __do_execve_file.isra.40+0x5d4/0x8a0 >> __x64_sys_execve+0x49/0x60 >> do_syscall_64+0x64/0x220 >> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> >> This changes mm_access to use the new exec_update_mutex >> instead of cred_guard_mutex. >> >> This patch is based on the following patch by Eric W. Biederman: >> "[PATCH 0/5] Infrastructure to allow fixing exec deadlocks" >> Link: https://lore.kernel.org/lkml/87v9ne5y4y.fsf_-_@xxxxxxxxxxxxxxxxxxxxx/ >> >> Signed-off-by: Bernd Edlinger <bernd.edlinger@xxxxxxxxxx> >> --- >> kernel/fork.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/fork.c b/kernel/fork.c >> index c12595a..5720ff3 100644 >> --- a/kernel/fork.c >> +++ b/kernel/fork.c >> @@ -1224,7 +1224,7 @@ struct mm_struct *mm_access(struct task_struct *task, unsigned int mode) >> struct mm_struct *mm; >> int err; >> >> - err = mutex_lock_killable(&task->signal->cred_guard_mutex); >> + err = mutex_lock_killable(&task->signal->exec_update_mutex); >> if (err) >> return ERR_PTR(err); >> >> @@ -1234,7 +1234,7 @@ struct mm_struct *mm_access(struct task_struct *task, unsigned int mode) >> mmput(mm); >> mm = ERR_PTR(-EACCES); >> } >> - mutex_unlock(&task->signal->cred_guard_mutex); >> + mutex_unlock(&task->signal->exec_update_mutex); >> >> return mm; >> }
