git: 589e4c1df4a6 - main - jail: Add safety around prison_deref() flags.

[Jamie Gritton]

The branch main has been updated by jamie:

URL: https://cgit.FreeBSD.org/src/commit/?id=589e4c1df4a6e4b1368f26fc7fef704a2e5cb42c

commit 589e4c1df4a6e4b1368f26fc7fef704a2e5cb42c
Author:     Jamie Gritton <jamie at FreeBSD.org>
AuthorDate: 2021-02-26 04:10:42 +0000
Commit:     Jamie Gritton <jamie at FreeBSD.org>
CommitDate: 2021-02-26 04:10:42 +0000

    jail: Add safety around prison_deref() flags.
    
    do_jail_attach() now only uses the PD_XXX flags that refer to lock
    status, so make sure that something else like PD_KILL doesn't slip
    through.
    
    Add a KASSERT() in prison_deref() to catch any further PD_KILL misuse.
---
 sys/kern/kern_jail.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index c8dcf928dfaa..b5c8f6ebf9be 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -161,6 +161,8 @@ static void prison_racct_detach(struct prison *pr);
 #define	PD_LOCKED	0x10	/* pr_mtx is held */
 #define	PD_LIST_SLOCKED	0x20	/* allprison_lock is held shared */
 #define	PD_LIST_XLOCKED	0x40	/* allprison_lock is held exclusive */
+#define PD_OP_FLAGS	0x07	/* Operation flags */
+#define PD_LOCK_FLAGS	0x70	/* Lock status flags */
 
 /*
  * Parameter names corresponding to PR_* flag values.  Size values are for kvm
@@ -1836,7 +1838,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
 	/* Attach this process to the prison if requested. */
 	if (flags & JAIL_ATTACH) {
 		error = do_jail_attach(td, pr,
-		    prison_lock_xlock(pr, drflags & ~PD_KILL));
+		    prison_lock_xlock(pr, drflags & PD_LOCK_FLAGS));
 		drflags &= ~(PD_LOCKED | PD_LIST_XLOCKED);
 		if (error) {
 			vfs_opterror(opts, "attach failed");
@@ -2346,6 +2348,7 @@ do_jail_attach(struct thread *td, struct prison *pr, int drflags)
 
 	mtx_assert(&pr->pr_mtx, MA_OWNED);
 	sx_assert(&allprison_lock, SX_LOCKED);
+	drflags &= PD_LOCK_FLAGS;
 	/*
 	 * XXX: Note that there is a slight race here if two threads
 	 * in the same privileged process attempt to attach to two
@@ -2719,6 +2722,8 @@ prison_deref(struct prison *pr, int flags)
 	for (;;) {
 		if (flags & PD_KILL) {
 			/* Kill the prison and its descendents. */
+			KASSERT(pr != &prison0,
+			    ("prison_deref trying to kill prison0"));
 			if (!(flags & PD_DEREF)) {
 				prison_hold(pr);
 				flags |= PD_DEREF;
@@ -2755,7 +2760,6 @@ prison_deref(struct prison *pr, int flags)
 			}
 		}
 		if (flags & PD_KILL) {
-			flags &= ~PD_KILL;
 			/*
 			 * Any remaining user references are probably processes
 			 * that need to be killed, either in this prison or its
@@ -2763,6 +2767,8 @@ prison_deref(struct prison *pr, int flags)
 			 */
 			if (refcount_load(&pr->pr_uref) > 0)
 				killpr = pr;
+			/* Make sure the parent prison doesn't get killed. */
+			flags &= ~PD_KILL;
 		}
 		if (flags & PD_DEREF) {
 			/* Drop a reference. */