git: fe6b360a6293 - stable/13 - MFC jail: Improve locking when removing prisons

[Jamie Gritton]

The branch stable/13 has been updated by jamie:

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

commit fe6b360a62931eaea450ab9ac9b41daac5996e51
Author:     Jamie Gritton <jamie at FreeBSD.org>
AuthorDate: 2021-02-20 22:38:58 +0000
Commit:     Jamie Gritton <jamie at FreeBSD.org>
CommitDate: 2021-03-12 18:15:13 +0000

    MFC jail: Improve locking when removing prisons
    
    Change the flow of prison_deref() so it doesn't let go of allprison_lock
    until it's completely done using it (except for a possible drop as part
    of an upgrade on its first try).
    
    Differential Revision:  https://reviews.freebsd.org/D28458
    
    (cherry picked from commit 6e1d1bfcac77603541706807803a198c6d954d7c)
---
 sys/kern/kern_jail.c | 69 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index 90ab69a372d2..65201eb12951 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -2793,11 +2793,17 @@ prison_complete(void *context, int pending)
 static void
 prison_deref(struct prison *pr, int flags)
 {
-	struct prison *ppr, *tpr;
+	struct prisonlist freeprison;
+	struct prison *rpr, *tpr;
 	int lastref, lasturef;
 
+	TAILQ_INIT(&freeprison);
 	if (!(flags & PD_LOCKED))
 		mtx_lock(&pr->pr_mtx);
+	/*
+	 * Release this prison as requested, which may cause its parent
+	 * to be released, and then maybe its grandparent, etc.
+	 */
 	for (;;) {
 		if (flags & PD_DEUREF) {
 			KASSERT(refcount_load(&pr->pr_uref) > 0,
@@ -2840,56 +2846,63 @@ prison_deref(struct prison *pr, int flags)
 			mtx_unlock(&pr->pr_mtx);
 		}
 
-		/* If the prison still has references, nothing else to do. */
-		if (!lastref) {
-			if (flags & PD_LIST_SLOCKED)
-				sx_sunlock(&allprison_lock);
-			else if (flags & PD_LIST_XLOCKED)
-				sx_xunlock(&allprison_lock);
-			return;
-		}
+		if (!lastref)
+			break;
 
 		if (flags & PD_LIST_SLOCKED) {
 			if (!sx_try_upgrade(&allprison_lock)) {
 				sx_sunlock(&allprison_lock);
 				sx_xlock(&allprison_lock);
 			}
+			flags &= ~PD_LIST_SLOCKED;
 		} else if (!(flags & PD_LIST_XLOCKED))
 			sx_xlock(&allprison_lock);
+		flags |= PD_LIST_XLOCKED;
 
 		TAILQ_REMOVE(&allprison, pr, pr_list);
 		LIST_REMOVE(pr, pr_sibling);
-		ppr = pr->pr_parent;
-		for (tpr = ppr; tpr != NULL; tpr = tpr->pr_parent)
+		TAILQ_INSERT_TAIL(&freeprison, pr, pr_list);
+		for (tpr = pr->pr_parent; tpr != NULL; tpr = tpr->pr_parent)
 			tpr->pr_childcount--;
+
+		/* Removing a prison frees a reference on its parent. */
+		pr = pr->pr_parent;
+		mtx_lock(&pr->pr_mtx);
+		flags |= PD_DEREF | PD_DEUREF;
+	}
+
+	/* Release all the prison locks. */
+	if (flags & PD_LIST_SLOCKED)
+		sx_sunlock(&allprison_lock);
+	else if (flags & PD_LIST_XLOCKED)
 		sx_xunlock(&allprison_lock);
 
+	/*
+	 * Finish removing any unreferenced prisons, which couldn't happen
+	 * while allprison_lock was held (to avoid a LOR on vrele).
+	 */
+	TAILQ_FOREACH_SAFE(rpr, &freeprison, pr_list, tpr) {
 #ifdef VIMAGE
-		if (pr->pr_vnet != ppr->pr_vnet)
-			vnet_destroy(pr->pr_vnet);
+		if (rpr->pr_vnet != rpr->pr_parent->pr_vnet)
+			vnet_destroy(rpr->pr_vnet);
 #endif
-		if (pr->pr_root != NULL)
-			vrele(pr->pr_root);
-		mtx_destroy(&pr->pr_mtx);
+		if (rpr->pr_root != NULL)
+			vrele(rpr->pr_root);
+		mtx_destroy(&rpr->pr_mtx);
 #ifdef INET
-		free(pr->pr_ip4, M_PRISON);
+		free(rpr->pr_ip4, M_PRISON);
 #endif
 #ifdef INET6
-		free(pr->pr_ip6, M_PRISON);
+		free(rpr->pr_ip6, M_PRISON);
 #endif
-		if (pr->pr_cpuset != NULL)
-			cpuset_rel(pr->pr_cpuset);
-		osd_jail_exit(pr);
+		if (rpr->pr_cpuset != NULL)
+			cpuset_rel(rpr->pr_cpuset);
+		osd_jail_exit(rpr);
 #ifdef RACCT
 		if (racct_enable)
-			prison_racct_detach(pr);
+			prison_racct_detach(rpr);
 #endif
-		free(pr, M_PRISON);
-
-		/* Removing a prison frees a reference on its parent. */
-		pr = ppr;
-		mtx_lock(&pr->pr_mtx);
-		flags = PD_DEREF | PD_DEUREF;
+		free(rpr, M_PRISON);
 	}
 }