All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
	Jan Kratochvil <jan.kratochvil@redhat.com>,
	Lennart Poettering <lpoetter@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Michal Schmidt <mschmidt@redhat.com>,
	Roland McGrath <roland@hack.frob.com>, Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] wait: WSTOPPED|WCONTINUED doesn't work if a zombie leader is traced by another process
Date: Wed, 26 Feb 2014 17:56:07 +0100	[thread overview]
Message-ID: <20140226165607.GC22677@redhat.com> (raw)
In-Reply-To: <20140226165504.GA22677@redhat.com>

Even if the main thread is dead the process still can stop/continue.
However, if the leader is ptraced wait_consider_task(ptrace => false)
always skips wait_task_stopped/wait_task_continued, so WSTOPPED or
WCONTINUED can never work for the natural parent in this case.

Move the "A zombie ptracee is only visible to its ptracer" check into
the "if (!delay_group_leader(p))" block. ->notask_error is cleared by
the "fall through" code below.

This depends on the previous change, wait_task_stopped/continued must
be avoided if !delay_group_leader() and the tracer is ->real_parent.
Otherwise WSTOPPED|WEXITED could wrongly report "stopped" when the child
is already dead (single-threaded or not). If it is traced by another
task then the "stopped" state is fine until the debugger detaches and
reveals a zombie state.

Stupid test-case:

	void *tfunc(void *arg)
	{
		sleep(1);	// wait for zombie leader
		raise(SIGSTOP);
		exit(0x13);
		return NULL;
	}

	int run_child(void)
	{
		pthread_t thread;

		if (!fork()) {
			int tracee = getppid();

			assert(ptrace(PTRACE_ATTACH, tracee, 0,0) == 0);
			do
				ptrace(PTRACE_CONT, tracee, 0,0);
			while (wait(NULL) > 0);

			return 0;
		}

		sleep(1);	// wait for PTRACE_ATTACH
		assert(pthread_create(&thread, NULL, tfunc, NULL) == 0);
		pthread_exit(NULL);
	}

	int main(void)
	{
		int child, stat;

		child = fork();
		if (!child)
			return run_child();

		assert(child == waitpid(-1, &stat, WSTOPPED));
		assert(stat == 0x137f);

		kill(child, SIGCONT);

		assert(child == waitpid(-1, &stat, WCONTINUED));
		assert(stat == 0xffff);

		assert(child == waitpid(-1, &stat, 0));
		assert(stat == 0x1300);

		return 0;
	}

Without this patch it hangs in waitpid(WSTOPPED), wait_task_stopped()
is never called.

Note: this doesn't fix all problems with a zombie delay_group_leader(),
WCONTINUED | WEXITED check is not exactly right. debugger can't assume
it will be notified if another thread reaps the whole thread group.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/exit.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 56c5ac3..790b73c 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1382,20 +1382,16 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
 
 	/* slay zombie? */
 	if (p->exit_state == EXIT_ZOMBIE) {
-		/*
-		 * A zombie ptracee is only visible to its ptracer.
-		 * Notification and reaping will be cascaded to the real
-		 * parent when the ptracer detaches.
-		 */
-		if (likely(!ptrace) && unlikely(p->ptrace)) {
-			/* it will become visible, clear notask_error */
-			wo->notask_error = 0;
-			return 0;
-		}
-
 		/* we don't reap group leaders with subthreads */
-		if (!delay_group_leader(p))
-			return wait_task_zombie(wo, p);
+		if (!delay_group_leader(p)) {
+			/*
+			 * A zombie ptracee is only visible to its ptracer.
+			 * Notification and reaping will be cascaded to the
+			 * real parent when the ptracer detaches.
+			 */
+			if (unlikely(ptrace) || likely(!p->ptrace))
+				return wait_task_zombie(wo, p);
+		}
 
 		/*
 		 * Allow access to stopped/continued state via zombie by
-- 
1.5.5.1



      parent reply	other threads:[~2014-02-26 16:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-20 17:38 [PATCH 0/5] kill the racy EXIT_ZOMBIE->EXIT_DEAD->EXIT_ZOMBIE transition Oleg Nesterov
2014-02-20 17:38 ` [PATCH 1/5] wait: fix reparent_leader() vs EXIT_DEAD->EXIT_ZOMBIE race Oleg Nesterov
2014-02-20 17:39 ` [PATCH 2/5] wait: introduce EXIT_TRACE to avoid the racy EXIT_DEAD->EXIT_ZOMBIE transition Oleg Nesterov
2014-02-20 17:39 ` [PATCH 3/5] wait: use EXIT_TRACE only if thread_group_leader(zombie) Oleg Nesterov
2014-02-20 17:39 ` [PATCH 4/5] wait: completely ignore the EXIT_DEAD tasks Oleg Nesterov
2014-02-20 17:39 ` [PATCH 5/5] wait: swap EXIT_ZOMBIE and EXIT_DEAD to hide EXIT_TRACE from user-space Oleg Nesterov
2014-02-20 19:48 ` [PATCH 0/5] kill the racy EXIT_ZOMBIE->EXIT_DEAD->EXIT_ZOMBIE transition Tejun Heo
2014-02-24 15:51   ` Oleg Nesterov
2014-02-26 16:55 ` [PATCH 0/2] wait: WSTOPPED & ptrace fixes Oleg Nesterov
2014-02-26 16:55   ` [PATCH 1/2] wait: WSTOPPED|WCONTINUED hangs if a zombie child is traced by real_parent Oleg Nesterov
2014-02-26 16:56   ` Oleg Nesterov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140226165607.GC22677@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lpoetter@redhat.com \
    --cc=mschmidt@redhat.com \
    --cc=roland@hack.frob.com \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.