From owner-dev-commits-src-all@freebsd.org Mon Jan 18 18:58:09 2021 Return-Path: Delivered-To: dev-commits-src-all@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 AD5464F3441; Mon, 18 Jan 2021 18:58:09 +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 4DKLds4Ypfz4qy7; Mon, 18 Jan 2021 18:58:09 +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 8F5BA19093; Mon, 18 Jan 2021 18:58:09 +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 10IIw95u090965; Mon, 18 Jan 2021 18:58:09 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 10IIw95j090964; Mon, 18 Jan 2021 18:58:09 GMT (envelope-from git) Date: Mon, 18 Jan 2021 18:58:09 GMT Message-Id: <202101181858.10IIw95j090964@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Jamie Gritton Subject: git: 76ad42abf9d4 - main - jail: Add prison_isvalid() and prison_isalive() 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/main X-Git-Reftype: branch X-Git-Commit: 76ad42abf9d46c7a86c9e727603fe62e8b62a37b Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jan 2021 18:58:09 -0000 The branch main has been updated by jamie: URL: https://cgit.FreeBSD.org/src/commit/?id=76ad42abf9d46c7a86c9e727603fe62e8b62a37b commit 76ad42abf9d46c7a86c9e727603fe62e8b62a37b Author: Jamie Gritton AuthorDate: 2021-01-18 18:56:20 +0000 Commit: Jamie Gritton CommitDate: 2021-01-18 18:56:20 +0000 jail: Add prison_isvalid() and prison_isalive() prison_isvalid() checks if a prison record can be used at all, i.e. pr_ref > 0. This filters out prisons that aren't fully created, and those that are either in the process of being dismantled, or will be at the next opportunity. While the check for pr_ref > 0 is simple enough to make without a convenience function, this prepares the way for other measures of prison validity. prison_isalive() checks not only validity as far as the useablity of the prison structure, but also whether the prison is visible to user space. It replaces a test for pr_uref > 0, which is currently only used within kern_jail.c, and not often there. Both of these functions also assert that either the prison mutex or allprison_lock is held, since it's generally the case that unlocked prisons aren't guaranteed to remain useable for any length of time. This isn't entirely true, for example a thread can assume its own prison is good, but most exceptions will exist inside of kern_jail.c. --- sys/kern/kern_jail.c | 104 +++++++++++++++++++++++++++++++------------------ sys/kern/sysv_msg.c | 2 +- sys/kern/sysv_sem.c | 2 +- sys/kern/sysv_shm.c | 2 +- sys/kern/uipc_mqueue.c | 3 +- sys/sys/jail.h | 2 + 6 files changed, 73 insertions(+), 42 deletions(-) diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c index ff540875c90e..ba4c1aab0d85 100644 --- a/sys/kern/kern_jail.c +++ b/sys/kern/kern_jail.c @@ -1008,7 +1008,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) TAILQ_FOREACH(inspr, &allprison, pr_list) { if (inspr->pr_id == jid) { mtx_lock(&inspr->pr_mtx); - if (inspr->pr_ref > 0) { + if (prison_isvalid(inspr)) { pr = inspr; drflags |= PD_LOCKED; inspr = NULL; @@ -1041,7 +1041,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) error = ENOENT; vfs_opterror(opts, "jail %d not found", jid); goto done_deref; - } else if (pr->pr_uref == 0) { + } else if (!prison_isalive(pr)) { if (!(flags & JAIL_DYING)) { error = ENOENT; vfs_opterror(opts, "jail %d is dying", @@ -1118,24 +1118,18 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) FOREACH_PRISON_CHILD(ppr, tpr) { if (tpr != pr && tpr->pr_ref > 0 && !strcmp(tpr->pr_name + pnamelen, namelc)) { - if (pr == NULL && - cuflags != JAIL_CREATE) { - mtx_lock(&tpr->pr_mtx); - if (tpr->pr_ref > 0) { + mtx_lock(&tpr->pr_mtx); + drflags |= PD_LOCKED; + if (prison_isalive(tpr)) { + if (pr == NULL && + cuflags != JAIL_CREATE) { /* * Use this jail * for updates. */ - if (tpr->pr_uref > 0) { - pr = tpr; - drflags |= - PD_LOCKED; - break; - } - deadpr = tpr; + pr = tpr; + break; } - mtx_unlock(&tpr->pr_mtx); - } else if (tpr->pr_uref > 0) { /* * Create, or update(jid): * name must not exist in an @@ -1147,13 +1141,19 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) name); goto done_deref; } + if (pr == NULL && + cuflags != JAIL_CREATE && + prison_isvalid(tpr)) + deadpr = tpr; + mtx_unlock(&tpr->pr_mtx); + drflags &= ~PD_LOCKED; } } /* If no active jail is found, use a dying one. */ if (deadpr != NULL && pr == NULL) { if (flags & JAIL_DYING) { mtx_lock(&deadpr->pr_mtx); - if (deadpr->pr_ref == 0) { + if (!prison_isvalid(deadpr)) { mtx_unlock(&deadpr->pr_mtx); goto name_again; } @@ -1192,7 +1192,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) goto done_deref; } mtx_lock(&ppr->pr_mtx); - if (ppr->pr_ref == 0) { + if (!prison_isvalid(ppr)) { mtx_unlock(&ppr->pr_mtx); error = ENOENT; vfs_opterror(opts, "jail \"%s\" not found", @@ -1735,7 +1735,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) * for now, so new ones will remain unseen until after the module * handlers have completed. */ - born = pr->pr_uref == 0; + born = !prison_isalive(pr); if (!created && (ch_flags & PR_PERSIST & (pr_flags ^ pr->pr_flags))) { if (pr_flags & PR_PERSIST) { pr->pr_ref++; @@ -2029,8 +2029,8 @@ kern_jail_get(struct thread *td, struct uio *optuio, int flags) TAILQ_FOREACH(pr, &allprison, pr_list) { if (pr->pr_id > jid && prison_ischild(mypr, pr)) { mtx_lock(&pr->pr_mtx); - if (pr->pr_ref > 0 && - (pr->pr_uref > 0 || (flags & JAIL_DYING))) + if ((flags & JAIL_DYING) + ? prison_isvalid(pr) : prison_isalive(pr)) break; mtx_unlock(&pr->pr_mtx); } @@ -2051,7 +2051,8 @@ kern_jail_get(struct thread *td, struct uio *optuio, int flags) pr = prison_find_child(mypr, jid); if (pr != NULL) { drflags |= PD_LOCKED; - if (pr->pr_uref == 0 && !(flags & JAIL_DYING)) { + if (!(prison_isalive(pr) || + (flags & JAIL_DYING))) { error = ENOENT; vfs_opterror(opts, "jail %d is dying", jid); @@ -2075,7 +2076,7 @@ kern_jail_get(struct thread *td, struct uio *optuio, int flags) pr = prison_find_name(mypr, name); if (pr != NULL) { drflags |= PD_LOCKED; - if (pr->pr_uref == 0 && !(flags & JAIL_DYING)) { + if (!(prison_isalive(pr) || (flags & JAIL_DYING))) { error = ENOENT; vfs_opterror(opts, "jail \"%s\" is dying", name); @@ -2203,7 +2204,7 @@ kern_jail_get(struct thread *td, struct uio *optuio, int flags) if (error != 0 && error != ENOENT) goto done; } - i = (pr->pr_uref == 0); + i = !prison_isalive(pr); error = vfs_setopt(opts, "dying", &i, sizeof(i)); if (error != 0 && error != ENOENT) goto done; @@ -2314,7 +2315,7 @@ sys_jail_remove(struct thread *td, struct jail_remove_args *uap) lpr = NULL; FOREACH_PRISON_DESCENDANT(pr, cpr, descend) { mtx_lock(&cpr->pr_mtx); - if (cpr->pr_ref > 0) { + if (prison_isvalid(cpr)) { tpr = cpr; cpr->pr_ref++; } else { @@ -2414,11 +2415,8 @@ sys_jail_attach(struct thread *td, struct jail_attach_args *uap) return (EINVAL); } - /* - * Do not allow a process to attach to a prison that is not - * considered to be "alive". - */ - if (pr->pr_uref == 0) { + /* Do not allow a process to attach to a prison that is not alive. */ + if (!prison_isalive(pr)) { mtx_unlock(&pr->pr_mtx); sx_sunlock(&allprison_lock); return (EINVAL); @@ -2514,7 +2512,7 @@ prison_find(int prid) TAILQ_FOREACH(pr, &allprison, pr_list) { if (pr->pr_id == prid) { mtx_lock(&pr->pr_mtx); - if (pr->pr_ref > 0) + if (prison_isvalid(pr)) return (pr); /* * Any active prison with the same ID would have @@ -2542,7 +2540,7 @@ prison_find_child(struct prison *mypr, int prid) FOREACH_PRISON_DESCENDANT(mypr, pr, descend) { if (pr->pr_id == prid) { mtx_lock(&pr->pr_mtx); - if (pr->pr_ref > 0) + if (prison_isvalid(pr)) return (pr); mtx_unlock(&pr->pr_mtx); } @@ -2567,18 +2565,17 @@ prison_find_name(struct prison *mypr, const char *name) FOREACH_PRISON_DESCENDANT(mypr, pr, descend) { if (!strcmp(pr->pr_name + mylen, name)) { mtx_lock(&pr->pr_mtx); - if (pr->pr_ref > 0) { - if (pr->pr_uref > 0) - return (pr); + if (prison_isalive(pr)) + return (pr); + if (prison_isvalid(pr)) deadpr = pr; - } mtx_unlock(&pr->pr_mtx); } } /* There was no valid prison - perhaps there was a dying one. */ if (deadpr != NULL) { mtx_lock(&deadpr->pr_mtx); - if (deadpr->pr_ref == 0) { + if (!prison_isvalid(deadpr)) { mtx_unlock(&deadpr->pr_mtx); goto again; } @@ -2967,6 +2964,37 @@ prison_ischild(struct prison *pr1, struct prison *pr2) return (0); } +/* + * Return true if the prison is currently alive. A prison is alive if it is + * valid and it holds user references. + */ +bool +prison_isalive(struct prison *pr) +{ + + mtx_assert(&pr->pr_mtx, MA_OWNED); + if (__predict_false(pr->pr_ref == 0)) + return (false); + if (__predict_false(pr->pr_uref == 0)) + return (false); + return (true); +} + +/* + * Return true if the prison is currently valid. A prison is valid if it has + * been fully created, and is not being destroyed. Note that dying prisons + * are still considered valid. + */ +bool +prison_isvalid(struct prison *pr) +{ + + mtx_assert(&pr->pr_mtx, MA_OWNED); + if (__predict_false(pr->pr_ref == 0)) + return (false); + return (true); +} + /* * Return 1 if the passed credential is in a jail and that jail does not * have its own virtual network stack, otherwise 0. @@ -3623,14 +3651,14 @@ sysctl_jail_list(SYSCTL_HANDLER_ARGS) cpr->pr_ip6s * sizeof(struct in6_addr)); } #endif - if (cpr->pr_ref == 0) { + if (!prison_isvalid(cpr)) { mtx_unlock(&cpr->pr_mtx); continue; } bzero(xp, sizeof(*xp)); xp->pr_version = XPRISON_VERSION; xp->pr_id = cpr->pr_id; - xp->pr_state = cpr->pr_uref > 0 + xp->pr_state = prison_isalive(cpr) ? PRISON_STATE_ALIVE : PRISON_STATE_DYING; strlcpy(xp->pr_path, prison_path(pr, cpr), sizeof(xp->pr_path)); strlcpy(xp->pr_host, cpr->pr_hostname, sizeof(xp->pr_host)); diff --git a/sys/kern/sysv_msg.c b/sys/kern/sysv_msg.c index 098bba612636..f048234f3a33 100644 --- a/sys/kern/sysv_msg.c +++ b/sys/kern/sysv_msg.c @@ -290,7 +290,7 @@ msginit() if (rsv == NULL) rsv = osd_reserve(msg_prison_slot); prison_lock(pr); - if ((pr->pr_allow & PR_ALLOW_SYSVIPC) && pr->pr_ref > 0) { + if (prison_isvalid(pr) && (pr->pr_allow & PR_ALLOW_SYSVIPC)) { (void)osd_jail_set_reserved(pr, msg_prison_slot, rsv, &prison0); rsv = NULL; diff --git a/sys/kern/sysv_sem.c b/sys/kern/sysv_sem.c index 612f45a3144a..deee60d87a5a 100644 --- a/sys/kern/sysv_sem.c +++ b/sys/kern/sysv_sem.c @@ -321,7 +321,7 @@ seminit(void) if (rsv == NULL) rsv = osd_reserve(sem_prison_slot); prison_lock(pr); - if ((pr->pr_allow & PR_ALLOW_SYSVIPC) && pr->pr_ref > 0) { + if (prison_isvalid(pr) && (pr->pr_allow & PR_ALLOW_SYSVIPC)) { (void)osd_jail_set_reserved(pr, sem_prison_slot, rsv, &prison0); rsv = NULL; diff --git a/sys/kern/sysv_shm.c b/sys/kern/sysv_shm.c index eb6c57a77a08..ad5f0030b965 100644 --- a/sys/kern/sysv_shm.c +++ b/sys/kern/sysv_shm.c @@ -979,7 +979,7 @@ shminit(void) if (rsv == NULL) rsv = osd_reserve(shm_prison_slot); prison_lock(pr); - if ((pr->pr_allow & PR_ALLOW_SYSVIPC) && pr->pr_ref > 0) { + if (prison_isvalid(pr) && (pr->pr_allow & PR_ALLOW_SYSVIPC)) { (void)osd_jail_set_reserved(pr, shm_prison_slot, rsv, &prison0); rsv = NULL; diff --git a/sys/kern/uipc_mqueue.c b/sys/kern/uipc_mqueue.c index 309fa3423fbd..dc94ce213d08 100644 --- a/sys/kern/uipc_mqueue.c +++ b/sys/kern/uipc_mqueue.c @@ -1569,7 +1569,8 @@ mqfs_prison_remove(void *obj, void *data __unused) found = 0; TAILQ_FOREACH(tpr, &allprison, pr_list) { prison_lock(tpr); - if (tpr->pr_root == pr->pr_root && tpr != pr && tpr->pr_ref > 0) + if (tpr != pr && prison_isvalid(tpr) && + tpr->pr_root == pr->pr_root) found = 1; prison_unlock(tpr); } diff --git a/sys/sys/jail.h b/sys/sys/jail.h index 96201b0638b3..67ef9347d093 100644 --- a/sys/sys/jail.h +++ b/sys/sys/jail.h @@ -407,6 +407,8 @@ void prison_proc_hold(struct prison *); void prison_proc_free(struct prison *); void prison_set_allow(struct ucred *cred, unsigned flag, int enable); int prison_ischild(struct prison *, struct prison *); +bool prison_isalive(struct prison *); +bool prison_isvalid(struct prison *); int prison_equal_ip4(struct prison *, struct prison *); int prison_get_ip4(struct ucred *cred, struct in_addr *ia); int prison_local_ip4(struct ucred *cred, struct in_addr *ia);