Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1659076

[PATCH 12/26] wait: Directly test for the two cases where wait_task_zombie is called

From "Eric W. Biederman" <ebiederm@xmission.com>
Newsgroups linux.kernel
Subject [PATCH 12/26] wait: Directly test for the two cases where wait_task_zombie is called
Date 2017-06-06 21:20 +0200
Message-ID <tPsbf-4vm-3@gated-at.bofh.it> (permalink)
References <tPs1B-4ry-43@gated-at.bofh.it> <tPsbf-4vm-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


We call wait_task_zombie to reap child processes and to release
zombie ptraced threads.  Collect all of the tests for these two
cases into their own individual if statements to make it clear
what is going on.

This results in no change in behavior just a change in how the code is
written.

Seeing how these two cases that call wait_task_zombie
are equivalent to the original is not trivial so here
is my reasoning:

	/*
	 * Original test ---------------------------------------------
	 */
	if ((exit_state == EXIT_ZOMBIE) &&
	    !delay_group_leader(p) &&
	    ((ptrace ||
	      (p->ptrace && !ptrace_reparented(p))) ||
	     !p->ptrace))
		return wait_task_zombile(wo, p);

	/*
	 * Expand delay_group_leader  --------------------------------
	 */
	if ((exit_state == EXIT_ZOMBIE) &&
	    !(thread_group_leader(p) && !thread_group_empty(p)) &&
	    ((ptrace ||
	      (p->ptrace && !ptrace_reparented(p))) ||
	     !p->ptrace))
		return wait_task_zombile(wo, p);

	/*
	 * Moving ! via DeMorgan's law  ------------------------------
	 */
	if ((exit_state == EXIT_ZOMBIE) &&
	    (!thread_group_leader(p) || thread_group_empty(p)) &&
	    ((ptrace ||
	      (p->ptrace && !ptrace_reparented(p))) ||
	     !p->ptrace))
		return wait_task_zombile(wo, p);

	/*
	 * Three basic cases from the last || expression  ------------
	 */
	/* zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) && !ptrace && !p->ptrace &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/* ptraced zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) && !ptrace && p->ptrace &&
	    !ptrace_reparented(p) &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/* ptraced process or thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/*
	 * Combining the first two cases  ----------------------------
	 */
	/* zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) && !ptrace &&
	    !ptrace_reparented(p) &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/* ptraced process or thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/*
	 * Exploiting !ptrace implies thread_group_leader  -----------
	 */
	/* zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) && !ptrace &&
	    !ptrace_reparented(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/* ptraced process or thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/*
	 * Splitting the second case by ptrace_reparented  -----------
	 */
	/* zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) && !ptrace &&
	    !ptrace_reparented(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/* ptraced child process or child thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    !ptrace_reparenated(p) &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/* ptraced process or thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    ptrace_reparanted(p) &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/*
	 * Splitting the second case by leadership  ------------------
	 */
	/* zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) && !ptrace &&
	    !ptrace_reparented(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/* ptraced child process? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    !ptrace_reparenated(p) &&
	    thread_group_leader(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/* ptraced child thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    !ptrace_reparenated(p) &&
	    !thread_group_leader(p))
		return wait_task_zombie(wo, p);

	/* ptraced process or thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    ptrace_reparanted(p) &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/*
	 * Combining the first two cases  ----------------------------
	 * Using the knowledge that !ptrace implies thread_group_leader
	 */
	/* zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) &&
	    !ptrace_reparented(p) &&
	    thread_group_leader(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/* ptraced child thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    !ptrace_reparenated(p) &&
	    !thread_group_leader(p))
		return wait_task_zombie(wo, p);

	/* ptraced non-child process or thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    ptrace_reparanted(p) &&
	    (!thread_group_leader(p) || thread_group_empty(p)))
		return wait_task_zombie(wo, p);

	/*
	 * Splitting the thread case by leadership  ------------------
	 */
	/* zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) &&
	    !ptrace_reparented(p) &&
	    thread_group_leader(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/* ptraced child thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    !ptrace_reparenated(p) &&
	    !thread_group_leader(p))
		return wait_task_zombie(wo, p);

	/* ptraced non-child thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    ptrace_reparanted(p) &&
	    !thread_group_leader(p))
		return wait_task_zombie(wo, p);

	/* ptraced non-child process? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    ptrace_reparanted(p) &&
	    thread_group_leader(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/*
	 * Combining the second and third cases  ---------------------
	 */
	/* zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) &&
	    !ptrace_reparented(p) &&
	    thread_group_leader(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/* ptraced thread? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    !thread_group_leader(p))
		return wait_task_zombie(wo, p);

	/* ptraced non-child process? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    ptrace_reparanted(p) &&
	    thread_group_leader(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/*
	 * Combining the second and third cases  ---------------------
	 */
	/* zombie child process? */
	if ((exit_state == EXIT_ZOMBIE) &&
	    !ptrace_reparented(p) &&
	    thread_group_leader(p) &&
	    thread_group_empty(p))
		return wait_task_zombie(wo, p);

	/* ptraced thread or ptraced non-child process? */
	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
	    (!thread_group_leader(p) ||
	     (ptrace_reparented(p) && thread_group_empty(p))))
		return wait_task_zombie(wo, p);

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/exit.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 306e526f4c5e..9b70e21c960d 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1342,6 +1342,24 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
 	if (!ret)
 		return ret;
 
+	/* zombie child process? */
+	if ((exit_state == EXIT_ZOMBIE) &&
+	    !ptrace_reparented(p) &&
+	    thread_group_leader(p) &&
+	    thread_group_empty(p))
+		return wait_task_zombie(wo, p);
+
+	/*
+	 * A zombie ptracee that is not a child of it's ptracer's
+	 * thread group is only visible to its ptracer.  Notification
+	 * and reaping will be cascaded to the real parent when the
+	 * ptracer detaches.
+	 */
+	if ((exit_state == EXIT_ZOMBIE) && ptrace &&
+	    (!thread_group_leader(p) ||
+	     (ptrace_reparented(p) && thread_group_empty(p))))
+		return wait_task_zombie(wo, p);
+
 	if (unlikely(exit_state == EXIT_TRACE)) {
 		/*
 		 * ptrace == 0 means we are the natural parent. In this case
@@ -1354,33 +1372,17 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
 
 	if (likely(!ptrace) && unlikely(p->ptrace)) {
 		/*
-		 * If it is traced by its real parent's group, just pretend
-		 * the caller is ptrace_do_wait() and reap this child if it
-		 * is zombie.
-		 *
-		 * This also hides 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.
+		 * 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_reparented(p))
 			ptrace = 1;
 	}
 
-	/* slay zombie? */
 	if (exit_state == EXIT_ZOMBIE) {
-		/* we don't reap group leaders with subthreads */
-		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
 		 * falling through.  Clearing of notask_error is complex.
-- 
2.10.1

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/26] Fixing wait, exit, ptrace, exec, and CLONE_THREAD ebiederm@xmission.com (Eric W. Biederman) - 2017-06-06 21:10 +0200
  [PATCH 01/26] alpha: Remove unused TASK_GROUP_LEADER "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 12/26] wait: Directly test for the two cases where wait_task_zombie is called "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 15/26] wait: Don't delay !ptrace_reparented leaders "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 13/26] wait: Remove unused delay_group_leader "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 17/26] exit: Rework the exit states for ptracees "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 20/26] wait: Don't pass the list to wait_consider_task "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 26/26] pidns: Ensure zap_pid_ns_processes always terminates "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 19/26] wait: Simpler code for clearing notask_error in wait_consider_task "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 24/26] signal: In ptrace_stop improve identical signal detection "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 09/26] signal: Don't allow sending SIGKILL or SIGSTOP to init "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 03/26] signal: Do not perform permission checks when sending pdeath_signal "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
      Re: [PATCH 03/26] signal: Do not perform permission checks when  sending pdeath_signal Linus Torvalds <torvalds@linux-foundation.org> - 2017-06-06 22:10 +0200
        Re: [PATCH 03/26] signal: Do not perform permission checks when sending pdeath_signal ebiederm@xmission.com (Eric W. Biederman) - 2017-06-07 13:40 +0200
      Re: [PATCH 03/26] signal: Do not perform permission checks when  sending pdeath_signal Richard Weinberger <richard.weinberger@gmail.com> - 2017-06-06 23:50 +0200
    [PATCH 07/26] pidns: Improve the error handling in alloc_pid "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 22/26] exit: Fix auto-wait of ptraced children "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 02/26] cgroup: Don't open code tasklist_empty() "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 04/26] signal: Make group_send_sig_info static "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 23/26] signal:  Fix SIGCONT before group stop completes. "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 18/26] wait: Fix WSTOPPED on a ptraced child "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 10/26] ptrace: Simplify ptrace_detach & exit_ptrace "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 21/26] wait: Optmize waitpid "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 14/26] wait: Move changing of ptrace from wait_consider_task into wait_task_stopped "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 08/26] exit: Make the runqueue rcu safe "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
      Re: [PATCH 08/26] exit: Make the runqueue rcu safe Oleg Nesterov <oleg@redhat.com> - 2017-06-07 15:20 +0200
    [PATCH 06/26] rlimit: Remove unnecessary grab of tasklist_lock "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
      Re: [PATCH 06/26] rlimit: Remove unnecessary grab of tasklist_lock Oleg Nesterov <oleg@redhat.com> - 2017-06-07 14:40 +0200
        Re: [PATCH 06/26] rlimit: Remove unnecessary grab of tasklist_lock ebiederm@xmission.com (Eric W. Biederman) - 2017-06-07 16:20 +0200
    [PATCH 16/26] exit: Fix reporting a ptraced !reparented leader has exited "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 11/26] wait: Properly implement __WCLONE handling in the presence of exec and ptrace "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 05/26] exit:  Remove the pointless clearing of SIGPENDING in __exit_signal "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:20 +0200
    [PATCH 25/26] signal: In ptrace_stop use CLD_TRAPPED in all ptrace signals "Eric W. Biederman" <ebiederm@xmission.com> - 2017-06-06 21:30 +0200
  Re: [PATCH 00/26] Fixing wait, exit, ptrace, exec, and CLONE_THREAD Aleksa Sarai <asarai@suse.de> - 2017-06-06 21:50 +0200
    Re: [PATCH 00/26] Fixing wait, exit, ptrace, exec, and CLONE_THREAD ebiederm@xmission.com (Eric W. Biederman) - 2017-06-07 13:50 +0200
      Re: [PATCH 00/26] Fixing wait, exit, ptrace, exec, and CLONE_THREAD Aleksa Sarai <asarai@suse.de> - 2017-06-07 14:30 +0200
  Re: [PATCH 00/26] Fixing wait, exit, ptrace, exec, and CLONE_THREAD Linus Torvalds <torvalds@linux-foundation.org> - 2017-06-06 22:10 +0200
    Re: [PATCH 00/26] Fixing wait, exit, ptrace, exec, and CLONE_THREAD ebiederm@xmission.com (Eric W. Biederman) - 2017-06-07 18:10 +0200

csiph-web