DragonFlyBSD Kernel Audit
DF-2800 / fix.diff
← back to finding ↓ download raw
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -210,6 +210,31 @@
 
 	error = assign_prison_id(pr);
 	if (error) {
+		if (pr->pr_root.ncp)
+			cache_drop(&pr->pr_root);
+		varsymset_clean(&pr->pr_varsymset);
+		nlookup_done(&nd);
+		return (error);
+	}
+
+	/*
+	 * Create the per-prison sysctl tree BEFORE publishing the prison
+	 * on allprison and WITHOUT holding jail_lock.
+	 *
+	 * prison_sysctl_create() -> SYSCTL_ADD_* -> sysctl_add_oid() takes
+	 * the all-CPU sysctl xlock (_sysctl_xlock), while the
+	 * sysctl_jail_list() handler takes jail_lock while holding a
+	 * per-CPU shared gd_sysctllock.  Holding jail_lock across the
+	 * sysctl tree creation/destruction is a guaranteed AB-BA deadlock
+	 * that wedges the whole kernel ( DF-2799 ).  Creating the tree
+	 * before publication also guarantees no sysctl node for a given
+	 * prison id can be observed or shared while its prison is being
+	 * torn down ( DF-2802 ).
+	 */
+	error = prison_sysctl_create(pr);
+	if (error) {
+		if (pr->pr_root.ncp)
+			cache_drop(&pr->pr_root);
 		varsymset_clean(&pr->pr_varsymset);
 		nlookup_done(&nd);
 		return (error);
@@ -220,25 +245,21 @@
 	++prisoncount;
 	lockmgr(&jail_lock, LK_RELEASE);
 
-	error = prison_sysctl_create(pr);
-	if (error)
-		goto out;
-
 	error = kern_jail_attach(pr->pr_id);
 	if (error)
-		goto out2;
+		goto out;
 
 	nlookup_done(&nd);
 	return 0;
 
-out2:
-	prison_sysctl_done(pr);
-
 out:
 	lockmgr(&jail_lock, LK_EXCLUSIVE);
 	LIST_REMOVE(pr, pr_list);
 	--prisoncount;
 	lockmgr(&jail_lock, LK_RELEASE);
+	prison_sysctl_done(pr);
+	if (pr->pr_root.ncp)
+		cache_drop(&pr->pr_root);
 	varsymset_clean(&pr->pr_varsymset);
 	nlookup_done(&nd);
 	return (error);
@@ -325,17 +346,25 @@
 
 	error = copyinstr(j.hostname, &pr->pr_host, sizeof(pr->pr_host), 0);
 	if (error)
-		goto out;
+		goto out_locked;
 
 	/* Use default capabilities as a template */
 	pr->pr_caps = prison_default_caps;
 
+	/*
+	 * kern_jail() manages the jail_lock itself.  The lock is NOT held
+	 * across kern_jail() because prison_sysctl_create() /
+	 * prison_sysctl_done() take the all-CPU sysctl xlock, which
+	 * deadlocks against sysctl_jail_list() readers holding a per-CPU
+	 * shared gd_sysctllock and blocking on jail_lock ( DF-2799 ).
+	 */
+	lockmgr(&jail_lock, LK_RELEASE);
+
 	error = kern_jail(pr, &j);
 	if (error)
 		goto out;
 
 	sysmsg->sysmsg_result = pr->pr_id;
-	lockmgr(&jail_lock, LK_RELEASE);
 
 	return (0);
 
@@ -346,6 +375,19 @@
 		SLIST_REMOVE_HEAD(&pr->pr_ips, entries);
 		kfree(jip, M_PRISON);
 	}
+	if (pr->pr_root.ncp)
+		cache_drop(&pr->pr_root);	/* DF-2801 */
+	kfree(pr, M_PRISON);
+
+	return (error);
+
+out_locked:
+	/* Delete all ips */
+	while (!SLIST_EMPTY(&pr->pr_ips)) {
+		jip = SLIST_FIRST(&pr->pr_ips);
+		SLIST_REMOVE_HEAD(&pr->pr_ips, entries);
+		kfree(jip, M_PRISON);
+	}
 	lockmgr(&jail_lock, LK_RELEASE);
 	kfree(pr, M_PRISON);
 
@@ -889,9 +931,15 @@
 	case SYSCAP_NODEBUG_UNPRIV:
 	case SYSCAP_NOSCHED:
 	case SYSCAP_NOSCHED_CPUSET:
-	case SYSCAP_NOSETTIME:
 		return (0);
 
+	case SYSCAP_NOSETTIME:
+		/*
+		 * The system clock is host-global state; jailed root
+		 * must not be able to step or drift it ( DF-2800 ).
+		 */
+		return (EPERM);
+
 	case SYSCAP_NOEXEC_SUID:		/* group 3 allowed */
 	case SYSCAP_NOEXEC_SGID:
 		return (0);
@@ -989,9 +1037,17 @@
 int
 prison_sysctl_create(struct prison *pr)
 {
-	char id_str[7];
+	char id_str[12];
 
-	ksnprintf(id_str, 6, "%d", pr->pr_id);
+	/*
+	 * Full decimal representation of any pr_id < JAIL_MAX (999999)
+	 * plus NUL.  The old 5-character truncation made every prison
+	 * with id >= 100000 share the sysctl node of the first prison
+	 * in its 1000-id band ("10000", "11000", ...), silently
+	 * dropping all per-jail capability controls and corrupting the
+	 * shared node's reference accounting at teardown ( DF-2802 ).
+	 */
+	ksnprintf(id_str, sizeof(id_str), "%d", pr->pr_id);
 
 	pr->pr_sysctl_ctx = (struct sysctl_ctx_list *) kmalloc(
 		sizeof(struct sysctl_ctx_list), M_PRISON, M_WAITOK | M_ZERO);
--- a/sys/kern/kern_caps.c
+++ b/sys/kern/kern_caps.c
@@ -311,6 +311,7 @@
 caps_priv_check(struct ucred *cred, int cap)
 {
 	int res;
+	int ocap = cap;
 
 	if (cred == NULL) {
 		if (cap & __SYSCAP_NULLCRED)
@@ -337,7 +338,13 @@
 	}
 	if (res & __SYSCAP_SELF)
 		return EPERM;
-	return (prison_priv_check(cred, cap));
+	/*
+	 * prison_priv_check() must see the ORIGINAL capability so its
+	 * per-leaf jail policy applies; passing only the rewritten group
+	 * meta value blanket-allows whole capability groups in jails
+	 * ( DF-2800 ).
+	 */
+	return (prison_priv_check(cred, ocap));
 }
 
 int
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -373,7 +373,15 @@
 		}
 	}
 	if (oidp->oid_refcnt > 1 ) {
-		oidp->oid_refcnt--;
+		/*
+		 * A dry run (del == 0) must not consume a reference:
+		 * sysctl_ctx_free() dry-runs every context entry first,
+		 * so decrementing here frees shared dynamic nodes (e.g.
+		 * prison trees) while other contexts still reference
+		 * them ( DF-2802 ).
+		 */
+		if (del)
+			oidp->oid_refcnt--;
 	} else {
 		if (oidp->oid_refcnt == 0) {
 			kprintf("Warning: bad oid_refcnt=%u (%s)!\n",