mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: linux-kernel@vger.kernel.org
Cc: linux-api@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Kees Cook <keescook@chromium.org>,
	Roland McGrath <roland@hack.frob.com>,
	Al Viro <viro@ZenIV.linux.org.uk>,
	David Howells <dhowells@redhat.com>,
	"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: [PATCH 18/26] wait: Fix WSTOPPED on a ptraced child
Date: Tue,  6 Jun 2017 14:03:30 -0500	[thread overview]
Message-ID: <20170606190338.28347-18-ebiederm@xmission.com> (raw)
In-Reply-To: <20170606190338.28347-1-ebiederm@xmission.com>

When ptracing waitpid(pid, WUNTRACED) has two possible meanings.
- Wait for ptrace stops from the task with tid == pid
- Wait for ordinary stops from the process with tgid == pid

The only sensible behavior and the Linux behavior in 2.2 and
2.4 has been to consume both ptrace stops and group stops
in this case.  It looks like when Oleg disentangled thread
stops and group stops in 2.6.30 fixing a lot of other issues
the case when we want to reap both was overlooked.

Consume both the group and the ptrace stop state when
waitpid(pid, WUNTRACED) could be asking for both, and
the wait status for both is idenitical.  This keeps
us from double reporting the stop and causing confusion.

This is very slight user visible change and is only visible
in the unlikely case a ptracer specifies WUNTRACED aka
WSTOPPED.

Write this code in such a way that it doesn't matter which
list we are traversing when we find a child whose stop states
we care about.

Fixes: 90bc8d8b1a38 ("do_wait: fix waiting for the group stop with the dead leader")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/exit.c | 82 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 40 insertions(+), 42 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index ff2ed1d60a8c..4e2d2b6f5581 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1151,22 +1151,23 @@ static int wait_task_zombie(struct wait_opts *wo, int old_state, struct task_str
 	return retval;
 }
 
-static int *task_stopped_code(struct task_struct *p, bool ptrace)
+static int *task_trace_stopped_code(struct task_struct *p)
 {
-	if (ptrace) {
-		if (task_is_traced(p) && !(p->jobctl & JOBCTL_LISTENING))
-			return &p->exit_code;
-	} else {
-		if (p->signal->flags & SIGNAL_STOP_STOPPED)
-			return &p->signal->group_exit_code;
-	}
+	if (task_is_traced(p) && !(p->jobctl & JOBCTL_LISTENING))
+		return &p->exit_code;
+	return NULL;
+}
+
+static int *task_group_stopped_code(struct task_struct *p)
+{
+	if (p->signal->flags & SIGNAL_STOP_STOPPED)
+		return &p->signal->group_exit_code;
 	return NULL;
 }
 
 /**
  * wait_task_stopped - Wait for %TASK_STOPPED or %TASK_TRACED
  * @wo: wait options
- * @ptrace: is the wait for ptrace
  * @p: task to wait for
  *
  * Handle sys_wait4() work for %p in state %TASK_STOPPED or %TASK_TRACED.
@@ -1181,49 +1182,47 @@ static int *task_stopped_code(struct task_struct *p, bool ptrace)
  * success, implies that tasklist_lock is released and wait condition
  * search should terminate.
  */
-static int wait_task_stopped(struct wait_opts *wo,
-				int ptrace, struct task_struct *p)
+static int wait_task_stopped(struct wait_opts *wo, struct task_struct *p)
 {
 	struct siginfo __user *infop;
-	int retval, exit_code, *p_code, why;
-	uid_t uid = 0; /* unneeded, required by compiler */
+	int retval, exit_code, *p_code, *g_code, why;
+	bool group, gstop, pstop;
+	uid_t uid;
 	pid_t pid;
 
 	/*
-	 * Hide group stop state from real parent; otherwise a single
-	 * stop can be reported twice as group and ptrace stop.  If a
-	 * ptracer wants to distinguish these two events for its own
-	 * children it should create a separate process which takes the
-	 * role of real parent.
-	 */
-	if (!ptrace && p->ptrace && !ptrace_reparented(p))
-		ptrace = 1;
-
-	/*
 	 * Traditionally we see ptrace'd stopped tasks regardless of options.
 	 */
-	if (!ptrace && !(wo->wo_flags & WUNTRACED))
+	group = thread_group_leader(p) && !ptrace_reparented(p);
+	pstop = same_thread_group(current, p->parent);
+	gstop = group && (wo->wo_flags & WUNTRACED);
+	if (!pstop && !gstop)
 		return 0;
 
-	if (!task_stopped_code(p, ptrace))
+	if ((!pstop || !task_trace_stopped_code(p)) &&
+	    (!gstop || !task_group_stopped_code(p)))
 		return 0;
 
 	exit_code = 0;
 	spin_lock_irq(&p->sighand->siglock);
 
-	p_code = task_stopped_code(p, ptrace);
-	if (unlikely(!p_code))
-		goto unlock_sig;
-
-	exit_code = *p_code;
-	if (!exit_code)
-		goto unlock_sig;
-
-	if (!unlikely(wo->wo_flags & WNOWAIT))
-		*p_code = 0;
-
-	uid = from_kuid_munged(current_user_ns(), task_uid(p));
-unlock_sig:
+	p_code = g_code = NULL;
+	if (pstop)
+		p_code = task_trace_stopped_code(p);
+	if (gstop)
+		g_code = task_group_stopped_code(p);
+	if (p_code) {
+		exit_code = *p_code;
+		why = CLD_TRAPPED;
+		if (!(wo->wo_flags & WNOWAIT))
+			*p_code = 0;
+	}
+	if (g_code && (!exit_code || (*g_code == exit_code))) {
+		exit_code = *g_code;
+		why = CLD_STOPPED;
+		if (!(wo->wo_flags & WNOWAIT))
+			*g_code = 0;
+	}
 	spin_unlock_irq(&p->sighand->siglock);
 	if (!exit_code)
 		return 0;
@@ -1236,8 +1235,8 @@ static int wait_task_stopped(struct wait_opts *wo,
 	 * possibly take page faults for user memory.
 	 */
 	get_task_struct(p);
+	uid = from_kuid_munged(current_user_ns(), task_uid(p));
 	pid = task_pid_vnr(p);
-	why = ptrace ? CLD_TRAPPED : CLD_STOPPED;
 	read_unlock(&tasklist_lock);
 	sched_annotate_sleep();
 
@@ -1403,10 +1402,9 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
 	}
 
 	/*
-	 * Wait for stopped.  Depending on @ptrace, different stopped state
-	 * is used and the two don't interact with each other.
+	 * Wait for stopped.
 	 */
-	ret = wait_task_stopped(wo, ptrace, p);
+	ret = wait_task_stopped(wo, p);
 	if (ret)
 		return ret;
 
-- 
2.10.1

  parent reply	other threads:[~2017-06-06 19:16 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-06 19:01 [PATCH 00/26] Fixing wait, exit, ptrace, exec, and CLONE_THREAD Eric W. Biederman
2017-06-06 19:03 ` [PATCH 01/26] alpha: Remove unused TASK_GROUP_LEADER Eric W. Biederman
2017-06-06 19:03   ` [PATCH 02/26] cgroup: Don't open code tasklist_empty() Eric W. Biederman
2017-06-06 19:03   ` [PATCH 03/26] signal: Do not perform permission checks when sending pdeath_signal Eric W. Biederman
2017-06-06 20:01     ` Linus Torvalds
2017-06-07 11:23       ` Eric W. Biederman
2017-06-06 21:42     ` Richard Weinberger
2017-06-06 19:03   ` [PATCH 04/26] signal: Make group_send_sig_info static Eric W. Biederman
2017-06-06 19:03   ` [PATCH 05/26] exit: Remove the pointless clearing of SIGPENDING in __exit_signal Eric W. Biederman
2017-06-06 19:03   ` [PATCH 06/26] rlimit: Remove unnecessary grab of tasklist_lock Eric W. Biederman
2017-06-07 12:36     ` Oleg Nesterov
2017-06-07 14:08       ` Eric W. Biederman
2017-06-06 19:03   ` [PATCH 07/26] pidns: Improve the error handling in alloc_pid Eric W. Biederman
2017-06-06 19:03   ` [PATCH 08/26] exit: Make the runqueue rcu safe Eric W. Biederman
2017-06-07 13:16     ` Oleg Nesterov
2017-06-06 19:03   ` [PATCH 09/26] signal: Don't allow sending SIGKILL or SIGSTOP to init Eric W. Biederman
2017-06-06 19:03   ` [PATCH 10/26] ptrace: Simplify ptrace_detach & exit_ptrace Eric W. Biederman
2017-06-06 19:03   ` [PATCH 11/26] wait: Properly implement __WCLONE handling in the presence of exec and ptrace Eric W. Biederman
2017-06-06 19:03   ` [PATCH 12/26] wait: Directly test for the two cases where wait_task_zombie is called Eric W. Biederman
2017-06-06 19:03   ` [PATCH 13/26] wait: Remove unused delay_group_leader Eric W. Biederman
2017-06-06 19:03   ` [PATCH 14/26] wait: Move changing of ptrace from wait_consider_task into wait_task_stopped Eric W. Biederman
2017-06-06 19:03   ` [PATCH 15/26] wait: Don't delay !ptrace_reparented leaders Eric W. Biederman
2017-06-06 19:03   ` [PATCH 16/26] exit: Fix reporting a ptraced !reparented leader has exited Eric W. Biederman
2017-06-06 19:03   ` [PATCH 17/26] exit: Rework the exit states for ptracees Eric W. Biederman
2017-06-06 19:03   ` Eric W. Biederman [this message]
2017-06-06 19:03   ` [PATCH 19/26] wait: Simpler code for clearing notask_error in wait_consider_task Eric W. Biederman
2017-06-06 19:03   ` [PATCH 20/26] wait: Don't pass the list to wait_consider_task Eric W. Biederman
2017-06-06 19:03   ` [PATCH 21/26] wait: Optmize waitpid Eric W. Biederman
2017-06-06 19:03   ` [PATCH 22/26] exit: Fix auto-wait of ptraced children Eric W. Biederman
2017-06-06 19:03   ` [PATCH 23/26] signal: Fix SIGCONT before group stop completes Eric W. Biederman
2017-06-06 19:03   ` [PATCH 24/26] signal: In ptrace_stop improve identical signal detection Eric W. Biederman
2017-06-06 19:03   ` [PATCH 25/26] signal: In ptrace_stop use CLD_TRAPPED in all ptrace signals Eric W. Biederman
2017-06-06 19:03   ` [PATCH 26/26] pidns: Ensure zap_pid_ns_processes always terminates Eric W. Biederman
2017-06-06 19:40 ` [PATCH 00/26] Fixing wait, exit, ptrace, exec, and CLONE_THREAD Aleksa Sarai
2017-06-07 11:36   ` Eric W. Biederman
2017-06-07 12:21     ` Aleksa Sarai
2017-06-06 20:07 ` Linus Torvalds
2017-06-07 15:59   ` Eric W. Biederman

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=20170606190338.28347-18-ebiederm@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=dhowells@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=oleg@redhat.com \
    --cc=roland@hack.frob.com \
    --cc=tglx@linutronix.de \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®