On Thu 22-09-16 13:09:25, Michal Hocko wrote: > On Thu 22-09-16 12:09:05, Mike Galbraith wrote: > > On Thu, 2016-09-22 at 11:53 +0200, Michal Hocko wrote: > > > On Thu 22-09-16 11:40:09, Mike Galbraith wrote: > > > > > > This patch doesn't help, nor does the previous patch... but with both > > > > applied, all is well. All you have to do now is figure out why :) > > > > > > Ohh, I should be more explicit, this needs the mm_access part as well. > > > Sorry for not being clear enough. So the full change is > > > > Ah. That was gonna happen after lunch, but since you already know it > > works, I can get back to un-b0rking one of my trees. > > Yeah, it should work. The testcase is running in a loop for more than > hour already and everything seems to be ok. Now the question is whether > the fix is really correct which is something for Oleg. > > Also I think there is at least one more issue lurking there. Without or > without the patch I can make tracer get stuck in do_wait waiting for > task which doesn't seem to be alive anymore. I will report it separately > after this one gets resolved to not pull more confusion in.
OK, the test in the loop has survived 3h of runtime without a single lockup so the patch seems to be working for this case. I am posting the patch with the full changelog, let's see if it is correct as well. As I've said earlier this might be a strace bug which does an unexpected syscall while it should be doing wait on the child process instead. If the patch is correct then I would mark it for stable as well. --- >From fe82d463fd2ef1585d2c37bf9fa6a1761e6ee0e5 Mon Sep 17 00:00:00 2001 From: Michal Hocko <mho...@suse.com> Date: Thu, 22 Sep 2016 10:09:34 +0200 Subject: [PATCH] signal: always signal tracer from the ptraced task Aleksa has reported the following lockup when stracing the following go program % cat exec.go package main import ( "os" "syscall" ) func main() { syscall.Exec("/bin/echo", []string{"/bin/echo", "Hello"}, os.Environ()) } $ go version go version go1.6.3 linux/amd64 $ go build -o exec exec.go $ strace -f ./exec [snip] [pid 10349] select(0, NULL, NULL, NULL, {0, 100} <unfinished ...> [pid 10346] <... select resumed> ) = 0 (Timeout) [pid 10346] select(0, NULL, NULL, NULL, {0, 20} <unfinished ...> [pid 10345] execve("/bin/echo", ["/bin/echo", "Hello"], [/* 95 vars */] <unfinished ...> [pid 10346] <... select resumed> ) = 0 (Timeout) [pid 10349] <... select resumed> ) = 0 (Timeout) execve will never finish unless the strace process get killed with SIGKILL. The reason is the following deadlock tracer thread_A thread_$N SyS_process_vm_readv SyS_execve do_exit do_execveat_common exit_notify prepare_bprm_creds do_notify_parent mutex_lock(cred_guard_mutex) __group_send_sig_info search_binary_handler send_signal load_elf_binary prepare_signal -> fail SIGHCHLD is SGL_DFL flush_old_exec # wait for sig->notify_count process_vm_rw process_vm_rw_core mm_access mutex_lock_killable(cred_guard_mutex) So there seems to be 2 issues here. The first one is that exiting threads (because of the ongoing exec) are not sending SIGCHLD signal to the tracer but they rely on the tracer to reap them and call release_task->__exit_signal which in turn would wake up the thread_A and move on with the exec. The other part of the story is that the tracer is not in do_wait but rather calls process_vm_readv (presumably to get arguments of the syscall) and it waits for a lock in killable rather than interruptible sleep. The fix is therefore twofold. We want to teach mm_access to sleep in interruptible sleep and we want to make sure that the traced child will send the signal to the parent even when it is ignored or SIG_DFL. sig_ignored already seems to be doing something along that line except it doesn't handle when a traced child sends a signal to the tracer. Fix this by checking the current ptrace status and whether the target task is the tracer. Reported-by: Aleksa Sarai <asa...@suse.com> Signed-off-by: Michal Hocko <mho...@suse.com> --- kernel/fork.c | 2 +- kernel/signal.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/kernel/fork.c b/kernel/fork.c index 5a57b9bab85c..d5b7c3aea187 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -837,7 +837,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_interruptible(&task->signal->cred_guard_mutex); if (err) return ERR_PTR(err); diff --git a/kernel/signal.c b/kernel/signal.c index 96e9bc40667f..5c8b84b76f0b 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -91,6 +91,10 @@ static int sig_ignored(struct task_struct *t, int sig, bool force) if (!sig_task_ignored(t, sig, force)) return 0; + /* Do not ignore signals sent from child to the parent */ + if (current->ptrace && current->parent == t) + return 0; + /* * Tracers may want to know about even ignored signals. */ -- 2.9.3 -- Michal Hocko SUSE Labs ------------------------------------------------------------------------------ _______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel