git: 8164032a495b - main - reapkill: handle possible pid reuse after the pid was recorded as signalled

The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=8164032a495b53b9176814f7b08e093961fabdca

commit 8164032a495b53b9176814f7b08e093961fabdca
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2023-05-12 22:36:52 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-05-29 22:10:36 +0000

    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
---
 sys/kern/kern_procctl.c | 36 +++++++++++++++++++++++++++++++++++-
 sys/sys/proc.h          |  1 +
 2 files changed, 36 insertions(+), 1 deletion(-)

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 */