From owner-dev-commits-src-branches@freebsd.org Fri Feb 19 21:53:33 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 623C9540FC0; Fri, 19 Feb 2021 21:53:33 +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 4Dj51T2GTHz3NLQ; Fri, 19 Feb 2021 21:53:33 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 402A113B1A; Fri, 19 Feb 2021 21:53:33 +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 11JLrXnk052986; Fri, 19 Feb 2021 21:53:33 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 11JLrXl9052985; Fri, 19 Feb 2021 21:53:33 GMT (envelope-from git) Date: Fri, 19 Feb 2021 21:53:33 GMT Message-Id: <202102192153.11JLrXl9052985@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Jamie Gritton Subject: git: 9f00cb5fa8a4 - releng/13.0 - MFS jail: Handle a possible race between jail_remove(2) and fork(2) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jamie X-Git-Repository: src X-Git-Refname: refs/heads/releng/13.0 X-Git-Reftype: branch X-Git-Commit: 9f00cb5fa8a438e7b9efb2158f2e2edc730badd1 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Feb 2021 21:53:33 -0000 The branch releng/13.0 has been updated by jamie: URL: https://cgit.FreeBSD.org/src/commit/?id=9f00cb5fa8a438e7b9efb2158f2e2edc730badd1 commit 9f00cb5fa8a438e7b9efb2158f2e2edc730badd1 Author: Jamie Gritton AuthorDate: 2021-02-16 19:19:13 +0000 Commit: Jamie Gritton 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 */