From nobody Mon Aug 7 00:59:49 2023 X-Original-To: dev-commits-src-branches@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 4RJydy28Mgz4pk0B; Mon, 7 Aug 2023 00:59:50 +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 4RJydy1cF8z3WfW; Mon, 7 Aug 2023 00:59:50 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1691369990; 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=f9R2qvhsSyMRXfkjWVjUq+Tf8isDP4EK/xpGo639MMk=; b=dUm4Iw1SnUxJHg40DbP3zreD/+aPxT1BpmIhs/wiUd+Z9BeTUTiD84sOTdQNor2nu+awRK 2DtDxrzWGek8FIqhHHtVXDgVPPmEy5BapJSzXWcYYtfzYf+6CQ9MR4ZmicbtjLa88RVnIa W6kjuRd1bDCkTXj2Ll4v4ggifpFb72x9/1B0YKwg5iLv3NLPRbLakhsFx8xluXXRpRQPdU onmJLdxR/9i8KlNnmQFD7mxLiiPP33iE/6dmZS7o/JQ/soC3m+EhynuBPXxwEtHnAFmYPK B3RolXBpA098VpySppWxIrwHR9N/STJiin8xn/kfTbHKOKQRvgX0Uda1QZAccQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1691369990; 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=f9R2qvhsSyMRXfkjWVjUq+Tf8isDP4EK/xpGo639MMk=; b=G2hohYZmRziJgWwLgy1gAISUPL48wixb+2u/KAd4i1bSHFezOlYp2u3j6Z8idV0jxHTXoF 8VDdP/q1k7fP90a5EK1h5V7ZG+68NdW6hsPiXeayDlPxNUJPWHSqZ6tMyzLw3BmQIy10eG wE9VUEyzlFwhtP6qoRZMBMx2Q9sUjgwj/gbHhcxGR6qny+do0hduxVD93roUyHwgpLRKy8 RW1vt/9tSetZIoKlNLu55nmuQQkmqsmYEVBnhDqh5KmP9wuEZCtE5c/Or9u8U5uJScsuQ/ 52J44rELfEwyLGWL0RryDFpOB1sqYY2RjqYjvjGEQQyO852NDC3K4HXBsSkiEw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1691369990; a=rsa-sha256; cv=none; b=j+1U9i7c/rqC0auoBUhaEJvpCs+zujCrla4uWpSzn7Xg1Oik9nLuY/xH8piMY8Lzet99B7 NHoLy/V3dg2HfuKojAnBQWRzmMBOvIlei9GpP87z4PysGYdqKikdKdq3aDgvI7qlf5R1tE 7ucXN+Wq28EDtPTWW7EXu+UVmJDJWeC1MRtHHjAvSl0sQwEZWrHJ0A6Ife4ntEonQDL+Y9 fbqkzPWXa9p40alBnAgswjwX/cU6Pqce3Cb3LztRGqElPRzJXRoOIJqL20a2+To56rMmah tynAnPcr5FCyFPJNCNxVlB9NU5lWYzoOCGXQxf6wBWpuRPb6wOS845lUeNq0Vg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none 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 4RJydy0X3qzXbY; Mon, 7 Aug 2023 00:59:50 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 3770xnBv077188; Mon, 7 Aug 2023 00:59:49 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3770xnGF077187; Mon, 7 Aug 2023 00:59:49 GMT (envelope-from git) Date: Mon, 7 Aug 2023 00:59:49 GMT Message-Id: <202308070059.3770xnGF077187@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: 2b0cd3b55294 - stable/13 - killpg(2): close a race with fork(2), part1 List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@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: 2b0cd3b552942c642a84f8e224b989c02d97125d Auto-Submitted: auto-generated The branch stable/13 has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=2b0cd3b552942c642a84f8e224b989c02d97125d commit 2b0cd3b552942c642a84f8e224b989c02d97125d Author: Konstantin Belousov AuthorDate: 2023-06-12 07:33:43 +0000 Commit: Konstantin Belousov CommitDate: 2023-08-07 00:48:45 +0000 killpg(2): close a race with fork(2), part1 (cherry picked from commit 3360b48525fc966894e77b8cd9c124669664472d) --- sys/kern/init_main.c | 1 + sys/kern/kern_fork.c | 28 ++++++++++++++++++++++++++++ sys/kern/kern_proc.c | 20 ++++++++++++++++++++ sys/kern/kern_prot.c | 17 +++++++++++++---- sys/kern/kern_sig.c | 8 ++++++++ sys/sys/proc.h | 2 ++ 6 files changed, 72 insertions(+), 4 deletions(-) diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c index c8cb0f5613ee..2f98d8a577d9 100644 --- a/sys/kern/init_main.c +++ b/sys/kern/init_main.c @@ -489,6 +489,7 @@ proc0_init(void *dummy __unused) LIST_INSERT_HEAD(&allproc, p, p_list); LIST_INSERT_HEAD(PIDHASH(0), p, p_hash); mtx_init(&pgrp0.pg_mtx, "process group", NULL, MTX_DEF | MTX_DUPOK); + sx_init(&pgrp0.pg_killsx, "killpg racer"); p->p_pgrp = &pgrp0; LIST_INSERT_HEAD(PGRPHASH(0), &pgrp0, pg_hash); LIST_INIT(&pgrp0.pg_members); diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 2d7e7bc0de4a..6fb9d2a83c8f 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -851,11 +851,13 @@ fork1(struct thread *td, struct fork_req *fr) struct vmspace *vm2; struct ucred *cred; struct file *fp_procdesc; + struct pgrp *pg; vm_ooffset_t mem_charged; int error, nprocs_new; static int curfail; static struct timeval lastfail; int flags, pages; + bool killsx_locked; flags = fr->fr_flags; pages = fr->fr_pages; @@ -912,6 +914,7 @@ fork1(struct thread *td, struct fork_req *fr) fp_procdesc = NULL; newproc = NULL; vm2 = NULL; + killsx_locked = false; /* * Increment the nprocs resource before allocations occur. @@ -941,6 +944,28 @@ fork1(struct thread *td, struct fork_req *fr) } } + /* + * Atomically check for signals and block threads from sending + * a signal to our process group until the child is visible. + */ + pg = p1->p_pgrp; + if (sx_slock_sig(&pg->pg_killsx) != 0) { + error = ERESTART; + goto fail2; + } else if (__predict_false(p1->p_pgrp != pg || sig_intr() != 0)) { + /* + * Either the process was moved to other process + * group, or there is pending signal. sx_slock_sig() + * does not check for signals if not sleeping for the + * lock. + */ + sx_sunlock(&pg->pg_killsx); + error = ERESTART; + goto fail2; + } else { + killsx_locked = true; + } + /* * If required, create a process descriptor in the parent first; we * will abandon it if something goes wrong. We don't finit() until @@ -1031,6 +1056,7 @@ fork1(struct thread *td, struct fork_req *fr) } do_fork(td, fr, newproc, td2, vm2, fp_procdesc); + sx_sunlock(&pg->pg_killsx); return (0); fail0: error = EAGAIN; @@ -1049,6 +1075,8 @@ fail2: fdrop(fp_procdesc, td); } atomic_add_int(&nprocs, -1); + if (killsx_locked) + sx_sunlock(&pg->pg_killsx); pause("fork", hz / 2); return (error); } diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c index 2879af8f9335..09796d542b4d 100644 --- a/sys/kern/kern_proc.c +++ b/sys/kern/kern_proc.c @@ -311,6 +311,7 @@ pgrp_init(void *mem, int size, int flags) pg = mem; mtx_init(&pg->pg_mtx, "process group", NULL, MTX_DEF | MTX_DUPOK); + sx_init(&pg->pg_killsx, "killpg racer"); return (0); } @@ -574,6 +575,7 @@ errout: int enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgrp, struct session *sess) { + struct pgrp *old_pgrp; sx_assert(&proctree_lock, SX_XLOCKED); @@ -585,6 +587,11 @@ enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgrp, struct session *sess) KASSERT(!SESS_LEADER(p), ("enterpgrp: session leader attempted setpgrp")); + old_pgrp = p->p_pgrp; + if (!sx_try_xlock(&old_pgrp->pg_killsx)) + return (ERESTART); + MPASS(old_pgrp == p->p_pgrp); + if (sess != NULL) { /* * new session @@ -626,6 +633,7 @@ enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgrp, struct session *sess) doenterpgrp(p, pgrp); + sx_xunlock(&old_pgrp->pg_killsx); return (0); } @@ -635,6 +643,7 @@ enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgrp, struct session *sess) int enterthispgrp(struct proc *p, struct pgrp *pgrp) { + struct pgrp *old_pgrp; sx_assert(&proctree_lock, SX_XLOCKED); PROC_LOCK_ASSERT(p, MA_NOTOWNED); @@ -647,8 +656,19 @@ enterthispgrp(struct proc *p, struct pgrp *pgrp) KASSERT(pgrp != p->p_pgrp, ("%s: p %p belongs to pgrp %p", __func__, p, pgrp)); + old_pgrp = p->p_pgrp; + if (!sx_try_xlock(&old_pgrp->pg_killsx)) + return (ERESTART); + MPASS(old_pgrp == p->p_pgrp); + if (!sx_try_xlock(&pgrp->pg_killsx)) { + sx_xunlock(&old_pgrp->pg_killsx); + return (ERESTART); + } + doenterpgrp(p, pgrp); + sx_xunlock(&pgrp->pg_killsx); + sx_xunlock(&old_pgrp->pg_killsx); return (0); } diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c index cf88cae3a72d..3677c6db368e 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -332,12 +332,13 @@ sys_setsid(struct thread *td, struct setsid_args *uap) struct pgrp *newpgrp; struct session *newsess; - error = 0; pgrp = NULL; newpgrp = uma_zalloc(pgrp_zone, M_WAITOK); newsess = malloc(sizeof(struct session), M_SESSION, M_WAITOK | M_ZERO); +again: + error = 0; sx_xlock(&proctree_lock); if (p->p_pgid == p->p_pid || (pgrp = pgfind(p->p_pid)) != NULL) { @@ -345,7 +346,12 @@ sys_setsid(struct thread *td, struct setsid_args *uap) PGRP_UNLOCK(pgrp); error = EPERM; } else { - (void)enterpgrp(p, p->p_pid, newpgrp, newsess); + error = enterpgrp(p, p->p_pid, newpgrp, newsess); + if (error == ERESTART) { + sx_xunlock(&proctree_lock); + goto again; + } + MPASS(error == 0); td->td_retval[0] = p->p_pid; newpgrp = NULL; newsess = NULL; @@ -391,10 +397,11 @@ sys_setpgid(struct thread *td, struct setpgid_args *uap) if (uap->pgid < 0) return (EINVAL); - error = 0; - newpgrp = uma_zalloc(pgrp_zone, M_WAITOK); +again: + error = 0; + sx_xlock(&proctree_lock); if (uap->pid != 0 && uap->pid != curp->p_pid) { if ((targp = pfind(uap->pid)) == NULL) { @@ -456,6 +463,8 @@ done: sx_xunlock(&proctree_lock); KASSERT(error == 0 || newpgrp != NULL, ("setpgid failed and newpgrp is NULL")); + if (error == ERESTART) + goto again; uma_zfree(pgrp_zone, newpgrp); return (error); } diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 79440c2866df..77f63252454a 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -1769,6 +1769,7 @@ killpg1(struct thread *td, int sig, int pgid, int all, ksiginfo_t *ksi) } sx_sunlock(&allproc_lock); } else { +again: sx_slock(&proctree_lock); if (pgid == 0) { /* @@ -1784,10 +1785,17 @@ killpg1(struct thread *td, int sig, int pgid, int all, ksiginfo_t *ksi) } } sx_sunlock(&proctree_lock); + if (!sx_try_xlock(&pgrp->pg_killsx)) { + PGRP_UNLOCK(pgrp); + sx_xlock(&pgrp->pg_killsx); + sx_xunlock(&pgrp->pg_killsx); + goto again; + } LIST_FOREACH(p, &pgrp->pg_members, p_pglist) { killpg1_sendsig(p, false, &arg); } PGRP_UNLOCK(pgrp); + sx_xunlock(&pgrp->pg_killsx); } MPASS(arg.ret != 0 || arg.found || !arg.sent); if (arg.ret == 0 && !arg.sent) diff --git a/sys/sys/proc.h b/sys/sys/proc.h index 997ff110bcdc..d3daa0c6879a 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -113,6 +113,8 @@ struct pgrp { pid_t pg_id; /* (c) Process group id. */ struct mtx pg_mtx; /* Mutex to protect members */ int pg_flags; /* (m) PGRP_ flags */ + struct sx pg_killsx; /* Mutual exclusion between group member + * fork() and killpg() */ }; #define PGRP_ORPHANED 0x00000001 /* Group is orphaned */