Date: Sun, 27 Dec 2020 04:25:18 GMT From: Jamie Gritton <jamie@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 0fe74ae624fc - main - jail: Consistently handle the pr_allow bitmask Message-ID: <202012270425.0BR4PIbp092658@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by jamie: URL: https://cgit.FreeBSD.org/src/commit/?id=0fe74ae624fcbd9378eeee30f257b08f4eae5abc commit 0fe74ae624fcbd9378eeee30f257b08f4eae5abc Author: Jamie Gritton <jamie@FreeBSD.org> AuthorDate: 2020-12-27 04:25:02 +0000 Commit: Jamie Gritton <jamie@FreeBSD.org> CommitDate: 2020-12-27 04:25:02 +0000 jail: Consistently handle the pr_allow bitmask Return a boolean (i.e. 0 or 1) from prison_allow, instead of the flag value itself, which is what sysctl expects. Add prison_set_allow(), which can set or clear a permission bit, and propagates cleared bits down to child jails. Use prison_allow() and prison_set_allow() in the various jail.allow.* sysctls, and others that depend on thoe permissions. Add locking around checking both pr_allow and pr_enforce_statfs in prison_priv_check(). --- sys/kern/kern_jail.c | 77 +++++++++++++++++++++++++++++++++++++++------------- sys/kern/kern_mib.c | 37 +++++++++++++++---------- sys/kern/kern_priv.c | 31 ++------------------- sys/kern/kern_prot.c | 22 ++++----------- sys/sys/jail.h | 1 + 5 files changed, 89 insertions(+), 79 deletions(-) diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c index bdcebcdfaf6c..a140b6f537d1 100644 --- a/sys/kern/kern_jail.c +++ b/sys/kern/kern_jail.c @@ -140,6 +140,8 @@ static int get_next_prid(struct prison **insprp); static int do_jail_attach(struct thread *td, struct prison *pr); static void prison_complete(void *context, int pending); static void prison_deref(struct prison *pr, int flags); +static void prison_set_allow_locked(struct prison *pr, unsigned flag, + int enable); static char *prison_path(struct prison *pr1, struct prison *pr2); static void prison_remove_one(struct prison *pr); #ifdef RACCT @@ -1726,12 +1728,9 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) } } } - if ((tallow = ch_allow & ~pr_allow)) { - /* Clear allow bits in all children. */ - FOREACH_PRISON_DESCENDANT_LOCKED(pr, tpr, descend) - tpr->pr_allow &= ~tallow; - } pr->pr_allow = (pr->pr_allow & ~ch_allow) | pr_allow; + if ((tallow = ch_allow & ~pr_allow)) + prison_set_allow_locked(pr, tallow, 0); /* * Persistent prisons get an extra reference, and prisons losing their * persist flag lose that reference. Only do this for existing prisons @@ -2589,13 +2588,15 @@ prison_find_name(struct prison *mypr, const char *name) } /* - * See if a prison has the specific flag set. + * See if a prison has the specific flag set. The prison should be locked, + * unless checking for flags that are only set at jail creation (such as + * PR_IP4 and PR_IP6), or only the single bit is examined, without regard + * to any other prison data. */ int prison_flag(struct ucred *cred, unsigned flag) { - /* This is an atomic read, so no locking is necessary. */ return (cred->cr_prison->pr_flags & flag); } @@ -2603,8 +2604,7 @@ int prison_allow(struct ucred *cred, unsigned flag) { - /* This is an atomic read, so no locking is necessary. */ - return (cred->cr_prison->pr_allow & flag); + return ((cred->cr_prison->pr_allow & flag) != 0); } /* @@ -2802,6 +2802,38 @@ prison_proc_free(struct prison *pr) mtx_unlock(&pr->pr_mtx); } +/* + * Set or clear a permission bit in the pr_allow field, passing restrictions + * (cleared permission) down to child jails. + */ +void +prison_set_allow(struct ucred *cred, unsigned flag, int enable) +{ + struct prison *pr; + + pr = cred->cr_prison; + sx_slock(&allprison_lock); + mtx_lock(&pr->pr_mtx); + prison_set_allow_locked(pr, flag, enable); + mtx_unlock(&pr->pr_mtx); + sx_sunlock(&allprison_lock); +} + +static void +prison_set_allow_locked(struct prison *pr, unsigned flag, int enable) +{ + struct prison *cpr; + int descend; + + if (enable != 0) + pr->pr_allow |= flag; + else { + pr->pr_allow &= ~flag; + FOREACH_PRISON_DESCENDANT_LOCKED(pr, cpr, descend) + cpr->pr_allow &= ~flag; + } +} + /* * Check if a jail supports the given address family. * @@ -3117,6 +3149,8 @@ prison_enforce_statfs(struct ucred *cred, struct mount *mp, struct statfs *sp) int prison_priv_check(struct ucred *cred, int priv) { + struct prison *pr; + int error; /* * Some policies have custom handlers. This routine should not be @@ -3388,11 +3422,14 @@ prison_priv_check(struct ucred *cred, int priv) case PRIV_VFS_UNMOUNT: case PRIV_VFS_MOUNT_NONUSER: case PRIV_VFS_MOUNT_OWNER: - if (cred->cr_prison->pr_allow & PR_ALLOW_MOUNT && - cred->cr_prison->pr_enforce_statfs < 2) - return (0); + pr = cred->cr_prison; + prison_lock(pr); + if (pr->pr_allow & PR_ALLOW_MOUNT && pr->pr_enforce_statfs < 2) + error = 0; else - return (EPERM); + error = EPERM; + prison_unlock(pr); + return (error); /* * Jails should hold no disposition on the PRIV_VFS_READ_DIR @@ -3685,14 +3722,16 @@ SYSCTL_UINT(_security_jail, OID_AUTO, jail_max_af_ips, CTLFLAG_RW, static int sysctl_jail_default_allow(SYSCTL_HANDLER_ARGS) { - struct prison *pr; - int allow, error, i; - - pr = req->td->td_ucred->cr_prison; - allow = (pr == &prison0) ? jail_default_allow : pr->pr_allow; + int error, i; /* Get the current flag value, and convert it to a boolean. */ - i = (allow & arg2) ? 1 : 0; + if (req->td->td_ucred->cr_prison == &prison0) { + mtx_lock(&prison0.pr_mtx); + i = (jail_default_allow & arg2) != 0; + mtx_unlock(&prison0.pr_mtx); + } else + i = prison_allow(req->td->td_ucred, arg2); + if (arg1 != NULL) i = !i; error = sysctl_handle_int(oidp, &i, 0, req); diff --git a/sys/kern/kern_mib.c b/sys/kern/kern_mib.c index abd04b47023b..483bbe453b0c 100644 --- a/sys/kern/kern_mib.c +++ b/sys/kern/kern_mib.c @@ -346,25 +346,27 @@ sysctl_hostname(SYSCTL_HANDLER_ARGS) KASSERT(len <= sizeof(tmpname), ("length %d too long for %s", len, __func__)); - pr = req->td->td_ucred->cr_prison; - if (!(pr->pr_allow & PR_ALLOW_SET_HOSTNAME) && req->newptr) - return (EPERM); /* * Make a local copy of hostname to get/set so we don't have to hold * the jail mutex during the sysctl copyin/copyout activities. */ + pr = req->td->td_ucred->cr_prison; mtx_lock(&pr->pr_mtx); bcopy((char *)pr + pr_offset, tmpname, len); mtx_unlock(&pr->pr_mtx); error = sysctl_handle_string(oidp, tmpname, len, req); + if (error != 0 || req->newptr == NULL) + return (error); - if (req->newptr != NULL && error == 0) { - /* - * Copy the locally set hostname to all jails that share - * this host info. - */ - sx_slock(&allprison_lock); + /* + * Copy the locally set hostname to all jails that share + * this host info. + */ + sx_slock(&allprison_lock); + if (!(pr->pr_allow & PR_ALLOW_SET_HOSTNAME)) + error = EPERM; + else { while (!(pr->pr_flags & PR_HOST)) pr = pr->pr_parent; mtx_lock(&pr->pr_mtx); @@ -375,8 +377,8 @@ sysctl_hostname(SYSCTL_HANDLER_ARGS) else bcopy(tmpname, (char *)cpr + pr_offset, len); mtx_unlock(&pr->pr_mtx); - sx_sunlock(&allprison_lock); } + sx_sunlock(&allprison_lock); return (error); } @@ -465,13 +467,18 @@ sysctl_hostid(SYSCTL_HANDLER_ARGS) * instead of a string, and is used only for hostid. */ pr = req->td->td_ucred->cr_prison; - if (!(pr->pr_allow & PR_ALLOW_SET_HOSTNAME) && req->newptr) - return (EPERM); + mtx_lock(&pr->pr_mtx); tmpid = pr->pr_hostid; + mtx_unlock(&pr->pr_mtx); + error = sysctl_handle_long(oidp, &tmpid, 0, req); + if (error != 0 || req->newptr == NULL) + return (error); - if (req->newptr != NULL && error == 0) { - sx_slock(&allprison_lock); + sx_slock(&allprison_lock); + if (!(pr->pr_allow & PR_ALLOW_SET_HOSTNAME)) + error = EPERM; + else { while (!(pr->pr_flags & PR_HOST)) pr = pr->pr_parent; mtx_lock(&pr->pr_mtx); @@ -482,8 +489,8 @@ sysctl_hostid(SYSCTL_HANDLER_ARGS) else cpr->pr_hostid = tmpid; mtx_unlock(&pr->pr_mtx); - sx_sunlock(&allprison_lock); } + sx_sunlock(&allprison_lock); return (error); } diff --git a/sys/kern/kern_priv.c b/sys/kern/kern_priv.c index b621de58f685..c1bd373bcb9e 100644 --- a/sys/kern/kern_priv.c +++ b/sys/kern/kern_priv.c @@ -63,46 +63,21 @@ static bool suser_enabled(struct ucred *cred) { - return (prison_allow(cred, PR_ALLOW_SUSER) ? true : false); -} - -static void inline -prison_suser_set(struct prison *pr, int enabled) -{ - - if (enabled) { - pr->pr_allow |= PR_ALLOW_SUSER; - } else { - pr->pr_allow &= ~PR_ALLOW_SUSER; - } + return (prison_allow(cred, PR_ALLOW_SUSER)); } static int sysctl_kern_suser_enabled(SYSCTL_HANDLER_ARGS) { - struct prison *pr, *cpr; struct ucred *cred; - int descend, error, enabled; + int error, enabled; cred = req->td->td_ucred; enabled = suser_enabled(cred); - error = sysctl_handle_int(oidp, &enabled, 0, req); if (error || !req->newptr) return (error); - - pr = cred->cr_prison; - sx_slock(&allprison_lock); - mtx_lock(&pr->pr_mtx); - - prison_suser_set(pr, enabled); - if (!enabled) { - FOREACH_PRISON_DESCENDANT_LOCKED(pr, cpr, descend) { - prison_suser_set(cpr, 0); - } - } - mtx_unlock(&pr->pr_mtx); - sx_sunlock(&allprison_lock); + prison_set_allow(cred, PR_ALLOW_SUSER, enabled); return (0); } diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c index 73b89582230d..529a6de4b2c8 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -1646,28 +1646,16 @@ p_cansched(struct thread *td, struct proc *p) static int sysctl_unprivileged_proc_debug(SYSCTL_HANDLER_ARGS) { - struct prison *pr; int error, val; - val = prison_allow(req->td->td_ucred, PR_ALLOW_UNPRIV_DEBUG) != 0; + val = prison_allow(req->td->td_ucred, PR_ALLOW_UNPRIV_DEBUG); error = sysctl_handle_int(oidp, &val, 0, req); if (error != 0 || req->newptr == NULL) return (error); - pr = req->td->td_ucred->cr_prison; - mtx_lock(&pr->pr_mtx); - switch (val) { - case 0: - pr->pr_allow &= ~(PR_ALLOW_UNPRIV_DEBUG); - break; - case 1: - pr->pr_allow |= PR_ALLOW_UNPRIV_DEBUG; - break; - default: - error = EINVAL; - } - mtx_unlock(&pr->pr_mtx); - - return (error); + if (val != 0 && val != 1) + return (EINVAL); + prison_set_allow(req->td->td_ucred, PR_ALLOW_UNPRIV_DEBUG, val); + return (0); } /* diff --git a/sys/sys/jail.h b/sys/sys/jail.h index b95406079ea1..96201b0638b3 100644 --- a/sys/sys/jail.h +++ b/sys/sys/jail.h @@ -405,6 +405,7 @@ void prison_hold(struct prison *pr); void prison_hold_locked(struct prison *pr); 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 *); int prison_equal_ip4(struct prison *, struct prison *); int prison_get_ip4(struct ucred *cred, struct in_addr *ia);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202012270425.0BR4PIbp092658>