git: 894360bacd42 - stable/13 - MFC jail: Handle a possible race between jail_remove(2) and fork(2)
Jamie Gritton
jamie at FreeBSD.org
Fri Feb 19 01:23:39 UTC 2021
The branch stable/13 has been updated by jamie:
URL: https://cgit.FreeBSD.org/src/commit/?id=894360bacd42f021551f76518edd445f6d299f2e
commit 894360bacd42f021551f76518edd445f6d299f2e
Author: Jamie Gritton <jamie at FreeBSD.org>
AuthorDate: 2021-02-16 19:19:13 +0000
Commit: Jamie Gritton <jamie at FreeBSD.org>
CommitDate: 2021-02-19 01:22:08 +0000
MFC jail: Handle a possible race between jail_remove(2) and fork(2)
jail_remove(2) includes a loop that sends SIGKILL to all processes
in a jail, but skips processes in PRS_NEW state. Thus it is possible
the a process in mid-fork(2) during jail removal can survive the jail
being removed.
Add a prison flag PR_REMOVE, which is checked before the new process
returns. If the jail is being removed, the process will then exit.
Also check this flag in jail_attach(2) which has a similar issue.
Reported by: trasz
Approved by: kib
(cherry picked from commit cc7b73065302005ebc4a19503188c8d6d5eb923d)
---
sys/kern/kern_fork.c | 6 ++++++
sys/kern/kern_jail.c | 24 +++++++++++++++++++++---
sys/sys/jail.h | 1 +
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index 5bdf5054863d..870ae494de5d 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -1126,6 +1126,12 @@ fork_return(struct thread *td, struct trapframe *frame)
PROC_UNLOCK(p);
}
+ /*
+ * If the prison was killed mid-fork, die along with it.
+ */
+ if (!prison_isalive(td->td_ucred->cr_prison))
+ exit1(td, 0, SIGKILL);
+
userret(td, frame);
#ifdef KTRACE
diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index ece0aa33e642..b56c889eeb7e 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -1764,6 +1764,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
}
}
pr->pr_flags = (pr->pr_flags & ~ch_flags) | pr_flags;
+ pr->pr_flags &= ~PR_REMOVE;
mtx_unlock(&pr->pr_mtx);
drflags &= ~PD_LOCKED;
@@ -2368,6 +2369,12 @@ prison_remove_one(struct prison *pr)
drflags = PD_DEREF | PD_LOCKED | PD_LIST_XLOCKED;
+ /*
+ * Mark the prison as doomed, so it doesn't accidentally come back
+ * to life. It may still be explicitly brought back by jail_set(2).
+ */
+ pr->pr_flags |= PR_REMOVE;
+
/* If the prison was persistent, it is not anymore. */
if (pr->pr_flags & PR_PERSIST) {
refcount_release(&pr->pr_ref);
@@ -2508,6 +2515,17 @@ do_jail_attach(struct thread *td, struct prison *pr)
#endif
prison_deref(oldcred->cr_prison, PD_DEREF | PD_DEUREF);
crfree(oldcred);
+
+ /*
+ * If the prison was killed while changing credentials, die along
+ * with it.
+ */
+ if (!prison_isalive(pr)) {
+ PROC_LOCK(p);
+ kern_psignal(p, SIGKILL);
+ PROC_UNLOCK(p);
+ }
+
return (0);
e_unlock:
@@ -3038,17 +3056,18 @@ prison_ischild(struct prison *pr1, struct prison *pr2)
/*
* Return true if the prison is currently alive. A prison is alive if it is
- * valid and it holds user references.
+ * valid and holds user references, and it isn't being removed.
*/
bool
prison_isalive(struct prison *pr)
{
- mtx_assert(&pr->pr_mtx, MA_OWNED);
if (__predict_false(refcount_load(&pr->pr_ref) == 0))
return (false);
if (__predict_false(refcount_load(&pr->pr_uref) == 0))
return (false);
+ if (__predict_false(pr->pr_flags & PR_REMOVE))
+ return (false);
return (true);
}
@@ -3061,7 +3080,6 @@ bool
prison_isvalid(struct prison *pr)
{
- mtx_assert(&pr->pr_mtx, MA_OWNED);
if (__predict_false(refcount_load(&pr->pr_ref) == 0))
return (false);
return (true);
diff --git a/sys/sys/jail.h b/sys/sys/jail.h
index 2d1a26787b99..2ac6aabdbd43 100644
--- a/sys/sys/jail.h
+++ b/sys/sys/jail.h
@@ -216,6 +216,7 @@ struct prison_racct {
/* primary jail address. */
/* Internal flag bits */
+#define PR_REMOVE 0x01000000 /* In process of being removed */
#define PR_IP4 0x02000000 /* IPv4 restricted or disabled */
/* by this jail or an ancestor */
#define PR_IP6 0x04000000 /* IPv6 restricted or disabled */
More information about the dev-commits-src-all
mailing list