From nobody Fri Jun 24 19:36:53 2022 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 81C61864073; Fri, 24 Jun 2022 19:36:54 +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 4LV6nf1NHYz4XBk; Fri, 24 Jun 2022 19:36:54 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1656099414; 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=F7avAWHltfmxvJtLchGduVA8JSvoQXfxpxU8N9QoDeI=; b=UblS48QxTFakpr3JdvPdIWe9tyzoJA+oGOYYrukf4sIdAhgc/s0s6hXHzZ9gUFZ81+Hxxj Ghqb6ODTD3SWxhfoJ5mwp+UvxPuVqTapLllJRc6IxdVaN2SBTbQ0s+YD/lsCy663HxI/Vz v4cFG8SpSk9oQlue7vJJxBZULlbQcruvJIEq5rpdRAAHMlpAx7XFS99yDI6xbEmxENpxuA EfRe+fWORVnQ9qUU3Z8N47IHOe3yeWFaJU2VkNh0Ga/f4yZy7D2mb6p0lC8DWxKj5OCViA nNtzTD/fiSaHAdj0yoi72gSMmJIY39NWYnhdxNO9wSa80IIeXenhRighroDBMw== 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 CF17B15D78; Fri, 24 Jun 2022 19:36:53 +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 25OJarGC084133; Fri, 24 Jun 2022 19:36:53 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 25OJarpS084132; Fri, 24 Jun 2022 19:36:53 GMT (envelope-from git) Date: Fri, 24 Jun 2022 19:36:53 GMT Message-Id: <202206241936.25OJarpS084132@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Konstantin Belousov Subject: git: dbb76ce57de7 - stable/13 - Fix another race between fork(2) and PROC_REAP_KILL subtree 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/stable/13 X-Git-Reftype: branch X-Git-Commit: dbb76ce57de72b546893786d0b1f73f720816105 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1656099414; 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=F7avAWHltfmxvJtLchGduVA8JSvoQXfxpxU8N9QoDeI=; b=r3S9Iz8rJASYK0pKG+uVvAaFy5zH4Ww+o7b0Hwo4LMF25yLXLAxJzP0DY8tbIqZaAWYHyy mcW/cPdthtpdVDavFKcRu/hVRCvipLQic/XXxiuWV73+tm46blH4qId8nydNwy2kRy1TMu J1y/R93yjJmvSOSw6rIlW7OSVQAmSELf7X4rCsIFYH08tRrBZIbptueClpDw+WsnX2UL6/ tFMR0NjDY9a69vngYPeaZ3hs7R8zsx6X+M1rpZUtd18nVKLsF5E9DzTkPKEd/dx5oui+0z rQA5Jeq0XWn0+RrpMbW46SmflEymlcnqex5DHcpR2AgylFoH6lkxtH64x62Iuw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1656099414; a=rsa-sha256; cv=none; b=p2+AE/YZLREqr8KIYzGEXKNCaJALMlbn1E0WBa6Z3RlHRp4GmIYQ1oST4udHIZNjTLD7jb ri7lzW2+qX4Ck7LNDKHwE2Ag4VonxdSd5Ah09sopK9O1IxPS4/eyrTxIQXEdGLb14evaF6 wpfHXCdwyZToRY4NNNLuNMYTtfhWrf1Ci8SVGV0aMbFkktAZjtlnnVFkaZR29sP+Sm+vrf k3QEpxWT/ImfPDirkPQc862M8qM+J2kgT78fO0HJ+iNjvtkN1Zdz6ve0jd/ogUW6t+9IS7 rbwPwFc7jwW0XzyPD59ETv5J5XqtuWJjVfqZvsWcEcrYXccNg+vY7fKBcuw9Iw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=dbb76ce57de72b546893786d0b1f73f720816105 commit dbb76ce57de72b546893786d0b1f73f720816105 Author: Konstantin Belousov AuthorDate: 2022-04-21 22:39:58 +0000 Commit: Konstantin Belousov CommitDate: 2022-06-24 14:45:45 +0000 Fix another race between fork(2) and PROC_REAP_KILL subtree (cherry picked from commit 709783373e57069cc014019a14a806b580e1d62f) --- sys/kern/kern_procctl.c | 101 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 87 insertions(+), 14 deletions(-) diff --git a/sys/kern/kern_procctl.c b/sys/kern/kern_procctl.c index 83fcc57f8f78..9db9577fde3d 100644 --- a/sys/kern/kern_procctl.c +++ b/sys/kern/kern_procctl.c @@ -244,22 +244,94 @@ reap_getpids(struct thread *td, struct proc *p, void *data) } static void +reap_kill_proc_relock(struct proc *p, int xlocked) +{ + PROC_UNLOCK(p); + if (xlocked) + sx_xlock(&proctree_lock); + else + sx_slock(&proctree_lock); + PROC_LOCK(p); +} + +static bool +reap_kill_proc_locked(struct thread *td, struct proc *p2, + ksiginfo_t *ksi, struct procctl_reaper_kill *rk, int *error) +{ + int error1, r, xlocked; + bool need_stop; + + PROC_LOCK_ASSERT(p2, MA_OWNED); + PROC_ASSERT_HELD(p2); + + error1 = p_cansignal(td, p2, rk->rk_sig); + if (error1 != 0) { + if (*error == ESRCH) { + rk->rk_fpid = p2->p_pid; + *error = error1; + } + return (true); + } + + /* + * The need_stop indicates if the target process needs to be + * suspended before being signalled. This is needed when we + * guarantee that all processes in subtree are signalled, + * avoiding the race with some process not yet fully linked + * into all structures during fork, ignored by iterator, and + * then escaping signalling. + * + * If need_stop is true, then reap_kill_proc() returns true if + * the process was successfully stopped and signalled, and + * false if stopping failed and the signal was not sent. + * + * The thread cannot usefully stop itself anyway, and if other + * thread of the current process forks while the current + * thread signals the whole subtree, it is an application + * race. + */ + need_stop = p2 != td->td_proc && + (p2->p_flag & (P_KPROC | P_SYSTEM)) == 0 && + (rk->rk_flags & REAPER_KILL_CHILDREN) == 0; + + if (need_stop) { + if (P_SHOULDSTOP(p2) == P_STOPPED_SINGLE) + return (false); /* retry later */ + xlocked = sx_xlocked(&proctree_lock); + sx_unlock(&proctree_lock); + r = thread_single(p2, SINGLE_ALLPROC); + if (r != 0) { + reap_kill_proc_relock(p2, xlocked); + return (false); + } + } + + pksignal(p2, rk->rk_sig, ksi); + rk->rk_killed++; + *error = error1; + + if (need_stop) { + reap_kill_proc_relock(p2, xlocked); + thread_single_end(p2, SINGLE_ALLPROC); + } + return (true); +} + +static bool reap_kill_proc(struct thread *td, struct proc *p2, ksiginfo_t *ksi, struct procctl_reaper_kill *rk, int *error) { - int error1; + bool res; + res = true; PROC_LOCK(p2); - error1 = p_cansignal(td, p2, rk->rk_sig); - if (error1 == 0) { - pksignal(p2, rk->rk_sig, ksi); - rk->rk_killed++; - *error = error1; - } else if (*error == ESRCH) { - rk->rk_fpid = p2->p_pid; - *error = error1; + if ((p2->p_flag & P_WEXIT) == 0) { + _PHOLD_LITE(p2); + res = reap_kill_proc_locked(td, p2, ksi, rk, error); + _PRELE(p2); } PROC_UNLOCK(p2); + return (res); } struct reap_kill_tracker { @@ -286,7 +358,7 @@ reap_kill_children(struct thread *td, struct proc *reaper, struct proc *p2; LIST_FOREACH(p2, &reaper->p_children, p_sibling) { - reap_kill_proc(td, p2, ksi, rk, error); + (void)reap_kill_proc(td, p2, ksi, rk, error); /* * Do not end the loop on error, signal everything we * can. @@ -317,10 +389,11 @@ 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) { - reap_kill_proc(td, p2, ksi, rk, error); - res = true; - } + if (alloc_unr_specific(pids, p2->p_pid) != p2->p_pid) + continue; + if (!reap_kill_proc(td, p2, ksi, rk, error)) + free_unr(pids, p2->p_pid); + res = true; } free(t, M_TEMP); }