reapkill: handle possible pid reuse after the pid was recorded as signalled

Nothing prevents the signalled process from exiting, and then other
process among eligible for signalling to reuse the exited process pid.
In this case, presence of the pid in the 'pids' unr set prevents it from
getting the deserved signal.

Handle it by marking each process with the new flag P2_REAPKILLED when
we are about to send the signal.  If the process pid is present in the
pids unr, but the struct proc is not marked with P2_REAPKILLED, we must
send signal to the pid again.

The use of the flag relies on the global sapblk preventing parallel
reapkills.

The pids unr must be used to clear the flags to all signalled processes.

Reviewed by:	markj
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D40089

2 files changed, 36 insertions, 1 deletions

diff --git a/sys/kern/kern_procctl.c b/sys/kern/kern_procctl.c
index a4f675c2938e..16bd9ac702e8 100644
--- a/sys/kern/kern_procctl.c
+++ b/sys/kern/kern_procctl.c
@@ -416,8 +416,22 @@ reap_kill_subtree_once(struct thread *td, struct proc *p, struct proc *reaper,
 				continue;
 			if ((p2->p_treeflag & P_TREE_REAPER) != 0)
 				reap_kill_sched(&tracker, p2);
-			if (alloc_unr_specific(pids, p2->p_pid) != p2->p_pid)
+
+			/*
+			 * Handle possible pid reuse.  If we recorded
+			 * p2 as killed but its p_flag2 does not
+			 * confirm it, that means that the process
+			 * terminated and its id was reused by other
+			 * process in the reaper subtree.
+			 *
+			 * Unlocked read of p2->p_flag2 is fine, it is
+			 * our thread that set the tested flag.
+			 */
+			if (alloc_unr_specific(pids, p2->p_pid) != p2->p_pid &&
+			    (atomic_load_int(&p2->p_flag2) &
+			    (P2_REAPKILLED | P2_WEXIT)) != 0)
 				continue;
+
 			if (p2 == td->td_proc) {
 				if ((p2->p_flag & P_HADTHREADS) != 0 &&
 				    (p2->p_flag2 & P2_WEXIT) == 0) {
@@ -428,6 +442,11 @@ reap_kill_subtree_once(struct thread *td, struct proc *p, struct proc *reaper,
 					st = false;
 				}
 				PROC_LOCK(p2);
+				/*
+				 * sapblk ensures that only one thread
+				 * in the system sets this flag.
+				 */
+				p2->p_flag2 |= P2_REAPKILLED;
 				if (st)
 					r = thread_single(p2, SINGLE_NO_EXIT);
 				(void)pksignal(p2, w->rk->rk_sig, w->ksi);
@@ -445,6 +464,7 @@ reap_kill_subtree_once(struct thread *td, struct proc *p, struct proc *reaper,
 				PROC_LOCK(p2);
 				if ((p2->p_flag2 & P2_WEXIT) == 0) {
 					_PHOLD_LITE(p2);
+					p2->p_flag2 |= P2_REAPKILLED;
 					PROC_UNLOCK(p2);
 					w->target = p2;
 					taskqueue_enqueue(taskqueue_thread,
@@ -471,6 +491,9 @@ reap_kill_subtree(struct thread *td, struct proc *p, struct proc *reaper,
     struct reap_kill_proc_work *w)
 {
 	struct unrhdr pids;
+	void *ihandle;
+	struct proc *p2;
+	int pid;
 
 	/*
 	 * pids records processes which were already signalled, to
@@ -486,6 +509,17 @@ reap_kill_subtree(struct thread *td, struct proc *p, struct proc *reaper,
 	PROC_UNLOCK(td->td_proc);
 	while (reap_kill_subtree_once(td, p, reaper, &pids, w))
 	       ;
+
+	ihandle = create_iter_unr(&pids);
+	while ((pid = next_iter_unr(ihandle)) != -1) {
+		p2 = pfind(pid);
+		if (p2 != NULL) {
+			p2->p_flag2 &= ~P2_REAPKILLED;
+			PROC_UNLOCK(p2);
+		}
+	}
+	free_iter_unr(ihandle);
+
 out:
 	clean_unrhdr(&pids);
 	clear_unrhdr(&pids);
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 6af221db056f..d4c5680ba5ed 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -881,6 +881,7 @@ struct proc {
 #define	P2_WEXIT		0x00040000	/* exit just started, no
 						   external thread_single() is
 						   permitted */
+#define	P2_REAPKILLED		0x00080000
 
 /* Flags protected by proctree_lock, kept in p_treeflags. */
 #define	P_TREE_ORPHANED		0x00000001	/* Reparented, on orphan list */