Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 19 Feb 2021 21:53:33 GMT
From:      Jamie Gritton <jamie@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 9f00cb5fa8a4 - releng/13.0 - MFS jail: Handle a possible race between jail_remove(2) and fork(2)
Message-ID:  <202102192153.11JLrXl9052985@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch releng/13.0 has been updated by jamie:

URL: https://cgit.FreeBSD.org/src/commit/?id=9f00cb5fa8a438e7b9efb2158f2e2edc730badd1

commit 9f00cb5fa8a438e7b9efb2158f2e2edc730badd1
Author:     Jamie Gritton <jamie@FreeBSD.org>
AuthorDate: 2021-02-16 19:19:13 +0000
Commit:     Jamie Gritton <jamie@FreeBSD.org>
CommitDate: 2021-02-19 21:53:07 +0000

    MFS 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.
    
    Approved by:    re (gjb)
    Reported by:    trasz
    Approved by:    kib
    
    (cherry picked from commit cc7b73065302005ebc4a19503188c8d6d5eb923d)
    (cherry picked from commit 894360bacd42f021551f76518edd445f6d299f2e)
---
 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 */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202102192153.11JLrXl9052985>