git: 1b4701fe1e84 - main - thread_unsuspend(): do not unuspend the suspended leader thread doing SINGLE_ALLPROC

The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=1b4701fe1e840f9ce5dae557b4dd99bda8a85735

commit 1b4701fe1e840f9ce5dae557b4dd99bda8a85735
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2022-06-08 01:27:30 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2022-06-13 19:30:03 +0000

    thread_unsuspend(): do not unuspend the suspended leader thread doing SINGLE_ALLPROC
    
    markj wrote:
    tdsendsignal() may unsuspend a target thread. I think there is at least
    one bug there: suppose thread T is suspended in
    thread_single(SINGLE_ALLPROC) when trying to kill another process with
    REAP_KILL. Suppose a different thread sends SIGKILL to T->td_proc. Then,
    tdsendsignal() calls thread_unsuspend(T, T->td_proc). thread_unsuspend()
    incorrectly decrements T->td_proc->p_suspcount to -1.
    
    Later, when T->td_proc exits, it will wait forever in
    thread_single(SINGLE_EXIT) since T->td_proc->p_suspcount never reaches 1.
    
    Since the thread suspension is bounded by time needed to do
    thread_single(), skipping the thread_unsuspend_one() call there should
    not affect signal delivery if this thread is selected as target.
    
    Reported by:    markj
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 weeks
    Differential revision:  https://reviews.freebsd.org/D35310
---
 sys/kern/kern_thread.c | 14 ++++++++++++--
 sys/sys/proc.h         |  2 +-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c
index 99f76f3b014d..98e1afddc08e 100644
--- a/sys/kern/kern_thread.c
+++ b/sys/kern/kern_thread.c
@@ -1228,8 +1228,12 @@ thread_single(struct proc *p, int mode)
 		else
 			p->p_flag &= ~P_SINGLE_BOUNDARY;
 	}
-	if (mode == SINGLE_ALLPROC)
+	if (mode == SINGLE_ALLPROC) {
 		p->p_flag |= P_TOTAL_STOP;
+		thread_lock(td);
+		td->td_flags |= TDF_DOING_SA;
+		thread_unlock(td);
+	}
 	p->p_flag |= P_STOPPED_SINGLE;
 	PROC_SLOCK(p);
 	p->p_singlethread = td;
@@ -1316,6 +1320,11 @@ stopme:
 		}
 	}
 	PROC_SUNLOCK(p);
+	if (mode == SINGLE_ALLPROC) {
+		thread_lock(td);
+		td->td_flags &= ~TDF_DOING_SA;
+		thread_unlock(td);
+	}
 	return (0);
 }
 
@@ -1602,7 +1611,8 @@ thread_unsuspend(struct proc *p)
 	if (!P_SHOULDSTOP(p)) {
                 FOREACH_THREAD_IN_PROC(p, td) {
 			thread_lock(td);
-			if (TD_IS_SUSPENDED(td)) {
+			if (TD_IS_SUSPENDED(td) && (td->td_flags &
+			    TDF_DOING_SA) == 0) {
 				wakeup_swapper |= thread_unsuspend_one(td, p,
 				    true);
 			} else
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 3c210c5d8ff7..cdb9cc17945d 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -468,7 +468,7 @@ do {									\
 #define	TDF_THRWAKEUP	0x00100000 /* Libthr thread must not suspend itself. */
 #define	TDF_SEINTR	0x00200000 /* EINTR on stop attempts. */
 #define	TDF_SWAPINREQ	0x00400000 /* Swapin request due to wakeup. */
-#define	TDF_UNUSED23	0x00800000 /* --available-- */
+#define	TDF_DOING_SA	0x00800000 /* Doing SINGLE_ALLPROC, do not unsuspend me */
 #define	TDF_SCHED0	0x01000000 /* Reserved for scheduler private use */
 #define	TDF_SCHED1	0x02000000 /* Reserved for scheduler private use */
 #define	TDF_SCHED2	0x04000000 /* Reserved for scheduler private use */