From nobody Mon May 29 22:11:28 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4QVV9X5Chkz4XFl9; Mon, 29 May 2023 22:11:28 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QVV9X4JY2z3KjK; Mon, 29 May 2023 22:11:28 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1685398288; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=/0xTuV6eYtHyo4ueAUCOnT7gomjaMpPrVXz8SxpqSs4=; b=mmXai35VIAA169xHoYw0njgRdJarFWKrGYMILjd2zyXJzadnF7XYPBz4Kz1Dyy1LUap/Z5 hQpLBvWRmTv20Z6Y6oLvNyriQfh96T1yNJ2PYY4rWZY0tQAncURjs2FgR8fbKDLyhf0AKH 4a/6E6qeEaA6MWe5Dr05e7g8QAuagGBX99Ge+KdDfSGT6Z8J3gjqGjoa9eFyDJrNjIWiva DedGTpydMQJlLCibTE9mgOEvGtqzHE9a6Bv1/651ndWLK8p2i29ntP/DwxiuDuv0pbCxVr Ffine1I9Njs4L+PJDL+dNsMFJGBADzQZr1xsCjzAMV5TQ8txap96CCF+Mmf2qw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1685398288; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=/0xTuV6eYtHyo4ueAUCOnT7gomjaMpPrVXz8SxpqSs4=; b=K5vmrSZzU9ShfG9sE/576QWuAfX2tjiN88/SGBdMK8dMzxA9aCLnMcwLSNQZuRAEhdg1Xi jl4ddmOUf7cBtw6ZrSJ9AKaAHf8W2JDzexrNgSwaj0YeKCU6LfZWcTtYJ0vyajtGg+zbjt EBsejc8fL/pznLxQUWKniUUcBpt/JPRvoYrBrahCNhKArMgENifd/va7KeCv4aBbsk8MWU VDK6cyUewgUBdfxEdmnv76Su3gObbHB4tg/JRL5YX3mpr6PAI3OIF8RcrU0epFRmWMPgOY DkTMz4uEB+0HS4ldZ8lv93mWSllrZBb4bKCSgXyyz5lxi3YtdOBrbTdds+Fe2w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1685398288; a=rsa-sha256; cv=none; b=uztxAcD+jJ+OYcrXB5xYKB0dWqXXQBbc/FLVg1Zqc8s1fLgbc5Ih05tfocDiK6Nku3iw8a N1+YDNtAih4ikTukyt08iwf0j/owpiEDdLZtxIoIAx+xtd53JJRQTAoKw8lnTmILP2O8rA yN/051ljaGFOuklUqnv2hEA/62H5XWxiLRZjWGaVD/yobhq7bEZe9C4QTTIuYyUdegJ+B4 rDUX079vKX1NnVcSENksqSOguHsQDoGA7hnUtGH0k/foVy88Pz1qLFIMTbhHc0Hwm4wxRD 1Gm5KxCCkR8pzfT3Sj3V3t5dayrmxpx2WefHY9JNWFNT8+ZKmriHWOhIuhwtYg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4QVV9X3HbjzSkL; Mon, 29 May 2023 22:11:28 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 34TMBSn9044858; Mon, 29 May 2023 22:11:28 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 34TMBSAG044857; Mon, 29 May 2023 22:11:28 GMT (envelope-from git) Date: Mon, 29 May 2023 22:11:28 GMT Message-Id: <202305292211.34TMBSAG044857@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Konstantin Belousov Subject: git: 8164032a495b - main - reapkill: handle possible pid reuse after the pid was recorded as signalled List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kib X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 8164032a495b53b9176814f7b08e093961fabdca Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=8164032a495b53b9176814f7b08e093961fabdca commit 8164032a495b53b9176814f7b08e093961fabdca Author: Konstantin Belousov AuthorDate: 2023-05-12 22:36:52 +0000 Commit: Konstantin Belousov 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 */