Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 30 Dec 2010 00:09:51 GMT
From:      Edward Tomasz Napierala <trasz@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 187309 for review
Message-ID:  <201012300009.oBU09pIf078839@skunkworks.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@187309?ac=10

Change 187309 by trasz@trasz_victim on 2010/12/30 00:09:28

	Fix LORs in limit enforcement code.

Affected files ...

.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_container.c#46 edit
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#102 edit
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_jail.c#28 edit
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_loginclass.c#23 edit
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_resource.c#55 edit

Differences ...

==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_container.c#46 (text+ko) ====


==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#102 (text+ko) ====

@@ -531,7 +531,8 @@
 	int removed = 0;
 	struct hrl_rule_link *link, *linktmp;
 
-	mtx_lock(&hrl_lock);
+	mtx_assert(&hrl_lock, MA_OWNED);
+
 	LIST_FOREACH_SAFE(link, &container->c_rule_links, hrl_next, linktmp) {
 		if (!hrl_rule_matches(link->hrl_rule, filter))
 			continue;
@@ -541,7 +542,6 @@
 		uma_zfree(hrl_rule_link_zone, link);
 		removed++;
 	}
-	mtx_unlock(&hrl_lock);
 	return (removed);
 }
 
@@ -588,9 +588,8 @@
 			loginclass_release(rule->hr_subject.hs_loginclass);
 		break;
 	case HRL_SUBJECT_TYPE_JAIL:
-		if (rule->hr_subject.hs_prison != NULL) {
+		if (rule->hr_subject.hs_prison != NULL)
 			prison_free(rule->hr_subject.hs_prison);
-		}
 		break;
 	default:
 		panic("hrl_rule_release_subject: unknown subject type %d",
@@ -598,7 +597,6 @@
 	}
 }
 
-
 struct hrl_rule *
 hrl_rule_alloc(int flags)
 {
@@ -919,10 +917,15 @@
 static int
 hrl_rule_remove_callback(struct container *container, const struct hrl_rule *filter, void *arg3)
 {
-	int *found = (int *)arg3;
+	int found = 0;
+
+	mtx_lock(&hrl_lock);
+	found += hrl_container_remove_rules(container, filter);
+	mtx_unlock(&hrl_lock);
+
+	*((int *)arg3) += found;
 
-	*found += hrl_container_remove_rules(container, filter);
-	return (0);
+	return (found);
 }
 
 /*
@@ -937,7 +940,9 @@
 	if (filter->hr_subject_type == HRL_SUBJECT_TYPE_PROCESS &&
 	    filter->hr_subject.hs_proc != NULL) {
 		p = filter->hr_subject.hs_proc;
+		mtx_lock(&hrl_lock);
 		found = hrl_container_remove_rules(&p->p_container, filter);
+		mtx_unlock(&hrl_lock);
 		if (found)
 			return (0);
 		return (ESRCH);
@@ -954,11 +959,11 @@
 	KASSERT(error == 0, ("prison_container_foreach failed"));
 
 	sx_assert(&allproc_lock, SA_LOCKED);
+	mtx_lock(&hrl_lock);
 	FOREACH_PROC_IN_SYSTEM(p) {
 		found += hrl_container_remove_rules(&p->p_container, filter);
-		if (error == 0)
-			found = 1;
 	}
+	mtx_unlock(&hrl_lock);
 
 	if (found)
 		return (0);
@@ -1152,14 +1157,14 @@
 	struct hrl_rule_link *link;
 	struct sbuf *sb = (struct sbuf *)arg3;
 
-	mtx_assert(&hrl_lock, MA_OWNED);
-
+	mtx_lock(&hrl_lock);
 	LIST_FOREACH(link, &container->c_rule_links, hrl_next) {
 		if (!hrl_rule_matches(link->hrl_rule, filter))
 			continue;
 		hrl_rule_to_sbuf(sb, link->hrl_rule);
 		sbuf_printf(sb, ",");
 	}
+	mtx_unlock(&hrl_lock);
 
 	return (0);
 }
@@ -1210,11 +1215,9 @@
 		mtx_unlock(&hrl_lock);
 	}
 
-	mtx_lock(&hrl_lock);
 	loginclass_container_foreach(hrl_get_rules_callback, filter, sb);
 	ui_container_foreach(hrl_get_rules_callback, filter, sb);
 	prison_container_foreach(hrl_get_rules_callback, filter, sb);
-	mtx_unlock(&hrl_lock);
 	if (sbuf_error(sb) == ENOMEM) {
 		sbuf_delete(sb);
 		free(buf, M_HRL);

==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_jail.c#28 (text+ko) ====

@@ -4258,15 +4258,16 @@
     const struct hrl_rule *filter, void *arg3),
     const struct hrl_rule *filter, void *arg3)
 {
-	int error;
+	int again;
 	struct prison *pr;
 
+again:
 	sx_slock(&allprison_lock);
 	TAILQ_FOREACH(pr, &allprison, pr_list) {
-		error = (callback)(&pr->pr_container, filter, arg3);
-		if (error != 0) {
+		again = (callback)(&pr->pr_container, filter, arg3);
+		if (again != 0) {
 			sx_sunlock(&allprison_lock);
-			return (error);
+			goto again;
 		}
 	}
 	sx_sunlock(&allprison_lock);

==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_loginclass.c#23 (text+ko) ====

@@ -82,6 +82,11 @@
 void
 loginclass_release(struct loginclass *lc)
 {
+	int old;
+
+	old = lc->lc_refcount;
+	if (old > 1 && atomic_cmpset_int(&lc->lc_refcount, old, old - 1))
+		return;
 
 	mtx_lock(&loginclasses_lock);
 	if (refcount_release(&lc->lc_refcount)) {
@@ -212,14 +217,27 @@
     const struct hrl_rule *filter, void *arg3),
     const struct hrl_rule *filter, void *arg3)
 {
-	int error;
-	struct loginclass *lc, *lctmp;
+	int again;
+	struct loginclass *lc;
 
-	LIST_FOREACH_SAFE(lc, &loginclasses, lc_next, lctmp) {
-		error = (callback)(&lc->lc_container, filter, arg3);
-		if (error != 0)
-			return (error);
+again:
+	mtx_lock(&loginclasses_lock);
+	LIST_FOREACH(lc, &loginclasses, lc_next) {
+		/*
+		 * Callback might free the rule, which in turn could
+		 * result in freeing loginclass, which would cause
+		 * recursion on loginclasses_lock.
+		 */
+		loginclass_acquire(lc);
+		again = (callback)(&lc->lc_container, filter, arg3);
+		if (again != 0) {
+			mtx_unlock(&loginclasses_lock);
+			loginclass_release(lc);
+			goto again;
+		}
+		loginclass_release(lc);
 	}
+	mtx_unlock(&loginclasses_lock);
 
 	return (0);
 }

==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_resource.c#55 (text+ko) ====

@@ -1299,19 +1299,27 @@
     const struct hrl_rule *filter, void *arg3),
     const struct hrl_rule *filter, void *arg3)
 {
-	int error;
-	struct uidinfo *uip, *nextuip;
+	int again;
+	struct uidinfo *uip;
 	struct uihashhead *uih;
 
+again:
 	rw_rlock(&uihashtbl_lock);
 	for (uih = &uihashtbl[uihash]; uih >= uihashtbl; uih--) {
-		for (uip = LIST_FIRST(uih); uip; uip = nextuip) {
-			nextuip = LIST_NEXT(uip, ui_hash);
-			error = (callback)(&uip->ui_container, filter, arg3);
-			if (error != 0) {
+		LIST_FOREACH(uip, uih, ui_hash) {
+			/*
+			 * Callback might free the rule, which in turn could
+			 * result in freeing uidinfo, which would cause recursion
+			 * on uihashtbl_lock.
+			 */
+			uihold(uip);
+			again = (callback)(&uip->ui_container, filter, arg3);
+			if (again != 0) {
 				rw_runlock(&uihashtbl_lock);
-				return (error);
+				uifree(uip);
+				goto again;
 			}
+			uifree(uip);
 		}
 	}
 	rw_runlock(&uihashtbl_lock);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201012300009.oBU09pIf078839>