git: 0fe74ae624fc - main - jail: Consistently handle the pr_allow bitmask
Jamie Gritton
jamie at FreeBSD.org
Sun Dec 27 04:25:19 UTC 2020
The branch main has been updated by jamie:
URL: https://cgit.FreeBSD.org/src/commit/?id=0fe74ae624fcbd9378eeee30f257b08f4eae5abc
commit 0fe74ae624fcbd9378eeee30f257b08f4eae5abc
Author: Jamie Gritton <jamie at FreeBSD.org>
AuthorDate: 2020-12-27 04:25:02 +0000
Commit: Jamie Gritton <jamie at 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);
More information about the dev-commits-src-all
mailing list