git: 2b0cd3b55294 - stable/13 - killpg(2): close a race with fork(2), part1
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 07 Aug 2023 00:59:49 UTC
The branch stable/13 has been updated by kib:
URL: https://cgit.FreeBSD.org/src/commit/?id=2b0cd3b552942c642a84f8e224b989c02d97125d
commit 2b0cd3b552942c642a84f8e224b989c02d97125d
Author: Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2023-06-12 07:33:43 +0000
Commit: Konstantin Belousov <kib@FreeBSD.org>
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 */