prison_free() can be called with a mutex held. This wasn't a problem until

I converted allprison_mtx mutex to allprison_lock sx lock. To fix this LOR,
move prison removal to prison_complete() entirely. To ensure that noone
will reference this prison before it's beeing removed from the list skip
prisons with 'pr_ref == 0' in prison_find() and assert that pr_ref has to
greater than 0 in prison_hold().

Reported by:	kris
OK'ed by:	rwatson

1 files changed, 16 insertions, 11 deletions

diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index a1d03c0..780b89a 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -296,6 +296,10 @@ prison_find(int prid)
 	LIST_FOREACH(pr, &allprison, pr_list) {
 		if (pr->pr_id == prid) {
 			mtx_lock(&pr->pr_mtx);
+			if (pr->pr_ref == 0) {
+				mtx_unlock(&pr->pr_mtx);
+				break;
+			}
 			return (pr);
 		}
 	}
@@ -305,37 +309,36 @@ prison_find(int prid)
 void
 prison_free(struct prison *pr)
 {
-	struct prison_service *psrv;
 
-	sx_xlock(&allprison_lock);
 	mtx_lock(&pr->pr_mtx);
 	pr->pr_ref--;
 	if (pr->pr_ref == 0) {
 		mtx_unlock(&pr->pr_mtx);
-		LIST_REMOVE(pr, pr_list);
-		prisoncount--;
-		sx_downgrade(&allprison_lock);
-		TAILQ_FOREACH(psrv, &prison_services, ps_next) {
-			psrv->ps_destroy(psrv, pr);
-		}
-		sx_sunlock(&allprison_lock);
-
 		TASK_INIT(&pr->pr_task, 0, prison_complete, pr);
 		taskqueue_enqueue(taskqueue_thread, &pr->pr_task);
 		return;
 	}
 	mtx_unlock(&pr->pr_mtx);
-	sx_xunlock(&allprison_lock);
 }
 
 static void
 prison_complete(void *context, int pending)
 {
+	struct prison_service *psrv;
 	struct prison *pr;
 	int vfslocked;
 
 	pr = (struct prison *)context;
 
+	sx_xlock(&allprison_lock);
+	LIST_REMOVE(pr, pr_list);
+	prisoncount--;
+	sx_downgrade(&allprison_lock);
+	TAILQ_FOREACH(psrv, &prison_services, ps_next) {
+		psrv->ps_destroy(psrv, pr);
+	}
+	sx_sunlock(&allprison_lock);
+
 	vfslocked = VFS_LOCK_GIANT(pr->pr_root->v_mount);
 	vrele(pr->pr_root);
 	VFS_UNLOCK_GIANT(vfslocked);
@@ -351,6 +354,8 @@ prison_hold(struct prison *pr)
 {
 
 	mtx_lock(&pr->pr_mtx);
+	KASSERT(pr->pr_ref > 0,
+	    ("Trying to hold dead prison (id=%d).", pr->pr_id));
 	pr->pr_ref++;
 	mtx_unlock(&pr->pr_mtx);
 }