Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 2 Oct 2009 17:20:42 GMT
From:      Edward Tomasz Napierala <trasz@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 169139 for review
Message-ID:  <200910021720.n92HKgnC054088@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=169139

Change 169139 by trasz@trasz_victim on 2009/10/02 17:20:24

	Rework rule storage to store pointers to subjects instead of their
	id's.

Affected files ...

.. //depot/projects/soc2009/trasz_limits/TODO#11 edit
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#61 edit
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_jail.c#14 edit
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_loginclass.c#12 edit
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_proc.c#8 integrate
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_prot.c#22 edit
.. //depot/projects/soc2009/trasz_limits/sys/kern/kern_resource.c#24 edit
.. //depot/projects/soc2009/trasz_limits/sys/sys/hrl.h#34 edit

Differences ...

==== //depot/projects/soc2009/trasz_limits/TODO#11 (text+ko) ====

@@ -1,7 +1,4 @@
 
- - In hrl_rule, instead of keeping subject ids, keep links to a process,
-   uidinfo and gidinfo, the same way we do with loginclasses.
-
  - Why did I put the loginclass pointer into the proc and not into the cred?
 
  - Make the limits lists protected by the subjects lock (e.g. process lock)

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

@@ -71,11 +71,11 @@
 };
 
 struct dict subjectnames[] = {
-	{ "process", HRL_SUBJECT_PROCESS },
-	{ "user", HRL_SUBJECT_USER },
-	{ "group", HRL_SUBJECT_GROUP },
-	{ "loginclass", HRL_SUBJECT_LOGINCLASS },
-	{ "jail", HRL_SUBJECT_JAIL },
+	{ "process", HRL_SUBJECT_TYPE_PROCESS },
+	{ "user", HRL_SUBJECT_TYPE_USER },
+	{ "group", HRL_SUBJECT_TYPE_GROUP },
+	{ "loginclass", HRL_SUBJECT_TYPE_LOGINCLASS },
+	{ "jail", HRL_SUBJECT_TYPE_JAIL },
 	{ NULL, -1 }};
 
 struct dict resourcenames[] = {
@@ -195,7 +195,7 @@
 }
 
 static const char *
-hrl_subject_name(int subject)
+hrl_subject_type_name(int subject)
 {
 	int i;
 
@@ -204,7 +204,7 @@
 			return (subjectnames[i].d_name);
 	}
 
-	panic("hrl_subject_name: unknown subject");
+	panic("hrl_subject_type_name: unknown subject type %d", subject);
 }
 
 static const char *
@@ -217,7 +217,7 @@
 			return (actionnames[i].d_name);
 	}
 
-	panic("hrl_action_name: unknown action");
+	panic("hrl_action_name: unknown action %d", action);
 }
 
 static const char *
@@ -230,7 +230,7 @@
 			return (resourcenames[i].d_name);
 	}
 
-	panic("hrl_resource_name: unknown resource");
+	panic("hrl_resource_name: unknown resource %d", resource);
 }
 
 static void
@@ -259,26 +259,26 @@
 hrl_available_resource(const struct proc *p, const struct hrl_rule *rule)
 {
 	int resource, i;
-	int64_t available, found = 0;
+	int64_t available = INT64_MAX, found = 0;
 	struct ucred *cred = p->p_ucred;
 
 	mtx_assert(&hrl_lock, MA_OWNED);
 
 	resource = rule->hr_resource;
 	switch (rule->hr_per) {
-	case HRL_SUBJECT_PROCESS:
+	case HRL_SUBJECT_TYPE_PROCESS:
 		available = rule->hr_amount -
 		    p->p_usage.hu_resources[resource];
 		break;
-	case HRL_SUBJECT_USER:
+	case HRL_SUBJECT_TYPE_USER:
 		available = rule->hr_amount -
 		    cred->cr_ruidinfo->ui_usage.hu_resources[resource];
 		break;
-	case HRL_SUBJECT_GROUP:
+	case HRL_SUBJECT_TYPE_GROUP:
 		if (hrl_group_accounting) {
 			for (i = 0; i < cred->cr_ngroups; i++) {
-				if (cred->cr_gidinfos[i]->gi_gid !=
-				    rule->hr_subject_id)
+				if (cred->cr_gidinfos[i] !=
+				    rule->hr_subject.hs_gip)
 					continue;
 
 				found = 1;
@@ -288,12 +288,12 @@
 			KASSERT(found, ("hrl_available_resource: group not found"));
 		}
 		break;
-	case HRL_SUBJECT_LOGINCLASS:
+	case HRL_SUBJECT_TYPE_LOGINCLASS:
 		available = rule->hr_amount -
 		    p->p_loginclass->lc_usage.hu_resources[resource];
-		available = INT64_MAX;
+		available = INT64_MAX; /* XXX */
 		break;
-	case HRL_SUBJECT_JAIL:
+	case HRL_SUBJECT_TYPE_JAIL:
 		available = rule->hr_amount -
 		    cred->cr_prison->pr_usage.hu_resources[resource];
 		break;
@@ -479,11 +479,11 @@
 	struct ucred *cred;
 	struct prison *pr;
 
-	KASSERT(amount > 0, ("hrl_alloc: invalid amount for %s: %lld",
+	KASSERT(amount > 0, ("hrl_alloc: invalid amount for %s: %ju",
 	    hrl_resource_name(resource), amount));
 
 #if 0
-	printf("hrl_alloc: allocating %lld of %s for %s (pid %d)\n", amount, hrl_resource_name(resource), p->p_comm, p->p_pid);
+	printf("hrl_alloc: allocating %ju of %s for %s (pid %d)\n", amount, hrl_resource_name(resource), p->p_comm, p->p_pid);
 #endif
 
 	mtx_lock(&hrl_lock);
@@ -540,7 +540,7 @@
 	struct ucred *cred;
 	struct prison *pr;
 
-	KASSERT(amount >= 0, ("hrl_allocated: invalid amount for %s: %lld",
+	KASSERT(amount >= 0, ("hrl_allocated: invalid amount for %s: %ju",
 	    hrl_resource_name(resource), amount));
 
 #if 0
@@ -599,7 +599,7 @@
 	struct ucred *cred;
 	struct prison *pr;
 
-	KASSERT(amount > 0, ("hrl_free: invalid amount for %s: %lld",
+	KASSERT(amount > 0, ("hrl_free: invalid amount for %s: %ju",
 	    hrl_resource_name(resource), amount));
 
 #if 0
@@ -608,8 +608,8 @@
 
 	mtx_lock(&hrl_lock);
 	KASSERT(amount <= p->p_usage.hu_resources[resource],
-	    ("hrl_free: freeing %lld of %s, which is more than allocated "
-	    "%lld for %s (pid %d)", amount, hrl_resource_name(resource),
+	    ("hrl_free: freeing %ju of %s, which is more than allocated "
+	    "%ld for %s (pid %d)", amount, hrl_resource_name(resource),
 	    p->p_usage.hu_resources[resource], p->p_comm, p->p_pid));
 	p->p_usage.hu_resources[resource] -= amount;
 	p->p_loginclass->lc_usage.hu_resources[resource] -= amount;
@@ -684,14 +684,40 @@
 hrl_rule_matches(const struct hrl_rule *rule, const struct hrl_rule *filter)
 {
 
-	if (filter->hr_subject != HRL_SUBJECT_UNDEFINED) {
-		if (rule->hr_subject != filter->hr_subject)
+	if (filter->hr_subject_type != HRL_SUBJECT_TYPE_UNDEFINED) {
+		if (rule->hr_subject_type != filter->hr_subject_type)
 			return (0);
-	}
 
-	if (filter->hr_subject_id != HRL_SUBJECT_ID_UNDEFINED) {
-		if (rule->hr_subject_id != filter->hr_subject_id)
-			return (0);
+		switch (filter->hr_subject_type) {
+		case HRL_SUBJECT_TYPE_PROCESS:
+			if (filter->hr_subject.hs_proc !=
+			    filter->hr_subject.hs_proc)
+				return (0);
+			break;
+		case HRL_SUBJECT_TYPE_USER:
+			if (filter->hr_subject.hs_uip !=
+			    filter->hr_subject.hs_uip)
+				return (0);
+			break;
+		case HRL_SUBJECT_TYPE_GROUP:
+			if (filter->hr_subject.hs_gip !=
+			    filter->hr_subject.hs_gip)
+				return (0);
+			break;
+		case HRL_SUBJECT_TYPE_LOGINCLASS:
+			if (filter->hr_subject.hs_loginclass !=
+			    filter->hr_subject.hs_loginclass)
+				return (0);
+			break;
+		case HRL_SUBJECT_TYPE_JAIL:
+			if (filter->hr_subject.hs_prison !=
+			    filter->hr_subject.hs_prison)
+				return (0);
+			break;
+		default:
+			panic("hrl_rule_matches: unknown subject type %d",
+			    filter->hr_subject_type);
+		}
 	}
 
 	if (filter->hr_resource != HRL_RESOURCE_UNDEFINED) {
@@ -709,7 +735,7 @@
 			return (0);
 	}
 
-	if (filter->hr_per != HRL_SUBJECT_UNDEFINED) {
+	if (filter->hr_per != HRL_SUBJECT_TYPE_UNDEFINED) {
 		if (rule->hr_per != filter->hr_per)
 			return (0);
 	}
@@ -828,6 +854,69 @@
 	return (removed);
 }
 
+static void
+hrl_rule_acquire_subject(struct hrl_rule *rule)
+{
+
+	switch (rule->hr_subject_type) {
+	case HRL_SUBJECT_TYPE_UNDEFINED:
+	case HRL_SUBJECT_TYPE_PROCESS:
+		break;
+	case HRL_SUBJECT_TYPE_USER:
+		if (rule->hr_subject.hs_uip != NULL)
+			uihold(rule->hr_subject.hs_uip);
+		break;
+	case HRL_SUBJECT_TYPE_GROUP:
+		if (rule->hr_subject.hs_gip != NULL)
+			gihold(rule->hr_subject.hs_gip);
+		break;
+	case HRL_SUBJECT_TYPE_LOGINCLASS:
+		if (rule->hr_subject.hs_loginclass != NULL)
+			loginclass_acquire(rule->hr_subject.hs_loginclass);
+		break;
+	case HRL_SUBJECT_TYPE_JAIL:
+		if (rule->hr_subject.hs_loginclass != NULL)
+			prison_hold(rule->hr_subject.hs_prison);
+		break;
+	default:
+		panic("hrl_rule_acquire_subject: unknown subject type %d",
+		    rule->hr_subject_type);
+	}
+}
+
+static void
+hrl_rule_release_subject(struct hrl_rule *rule)
+{
+
+	switch (rule->hr_subject_type) {
+	case HRL_SUBJECT_TYPE_UNDEFINED:
+	case HRL_SUBJECT_TYPE_PROCESS:
+		break;
+	case HRL_SUBJECT_TYPE_USER:
+		if (rule->hr_subject.hs_uip != NULL)
+			uifree(rule->hr_subject.hs_uip);
+		break;
+	case HRL_SUBJECT_TYPE_GROUP:
+		if (rule->hr_subject.hs_gip != NULL)
+			gifree(rule->hr_subject.hs_gip);
+		break;
+	case HRL_SUBJECT_TYPE_LOGINCLASS:
+		if (rule->hr_subject.hs_loginclass != NULL)
+			loginclass_release(rule->hr_subject.hs_loginclass);
+		break;
+	case HRL_SUBJECT_TYPE_JAIL:
+		if (rule->hr_subject.hs_prison != NULL) {
+			prison_free(rule->hr_subject.hs_prison);
+			sx_xunlock(&allprison_lock);
+		}
+		break;
+	default:
+		panic("hrl_rule_release_subject: unknown subject type %d",
+		    rule->hr_subject_type);
+	}
+}
+
+
 struct hrl_rule *
 hrl_rule_alloc(int flags)
 {
@@ -836,9 +925,13 @@
 	rule = uma_zalloc(hrl_rule_zone, flags);
 	if (rule == NULL)
 		return (NULL);
-	rule->hr_subject = HRL_SUBJECT_UNDEFINED;
-	rule->hr_subject_id = HRL_SUBJECT_ID_UNDEFINED;
-	rule->hr_per = HRL_SUBJECT_UNDEFINED;
+	rule->hr_subject_type = HRL_SUBJECT_TYPE_UNDEFINED;
+	rule->hr_subject.hs_proc = NULL;
+	rule->hr_subject.hs_uip = NULL;
+	rule->hr_subject.hs_gip = NULL;
+	rule->hr_subject.hs_loginclass = NULL;
+	rule->hr_subject.hs_prison = NULL;
+	rule->hr_per = HRL_SUBJECT_TYPE_UNDEFINED;
 	rule->hr_resource = HRL_RESOURCE_UNDEFINED;
 	rule->hr_action = HRL_ACTION_UNDEFINED;
 	rule->hr_amount = HRL_AMOUNT_UNDEFINED;
@@ -855,13 +948,18 @@
 	copy = uma_zalloc(hrl_rule_zone, flags);
 	if (copy == NULL)
 		return (NULL);
-	copy->hr_subject = rule->hr_subject;
-	copy->hr_subject_id = rule->hr_subject_id;
+	copy->hr_subject_type = rule->hr_subject_type;
+	copy->hr_subject.hs_proc = rule->hr_subject.hs_proc;
+	copy->hr_subject.hs_uip = rule->hr_subject.hs_uip;
+	copy->hr_subject.hs_gip = rule->hr_subject.hs_gip;
+	copy->hr_subject.hs_loginclass = rule->hr_subject.hs_loginclass;
+	copy->hr_subject.hs_prison = rule->hr_subject.hs_prison;
 	copy->hr_per = rule->hr_per;
 	copy->hr_resource = rule->hr_resource;
 	copy->hr_action = rule->hr_action;
 	copy->hr_amount = rule->hr_amount;
 	refcount_init(&copy->hr_refcount, 1);
+	hrl_rule_acquire_subject(copy);
 
 	return (copy);
 }
@@ -882,10 +980,7 @@
 	KASSERT(rule->hr_refcount > 0, ("rule->hr_refcount > 0"));
 
 	if (refcount_release(&rule->hr_refcount)) {
-		if (rule->hr_subject == HRL_SUBJECT_LOGINCLASS &&
-		    rule->hr_subject_id != HRL_SUBJECT_ID_UNDEFINED)
-			loginclass_release((struct loginclass *)(long)
-			    rule->hr_subject_id);
+		hrl_rule_release_subject(rule);
 		uma_zfree(hrl_rule_zone, rule);
 	}
 }
@@ -894,17 +989,40 @@
 hrl_rule_fully_specified(const struct hrl_rule *rule)
 {
 
-	if (rule->hr_subject == HRL_SUBJECT_UNDEFINED)
+	switch (rule->hr_subject_type) {
+	case HRL_SUBJECT_TYPE_UNDEFINED:
 		return (0);
-	if (rule->hr_subject_id == HRL_SUBJECT_ID_UNDEFINED)
-		return (0);
+	case HRL_SUBJECT_TYPE_PROCESS:
+		if (rule->hr_subject.hs_proc == NULL)
+			return (0);
+		break;
+	case HRL_SUBJECT_TYPE_USER:
+		if (rule->hr_subject.hs_uip == NULL)
+			return (0);
+		break;
+	case HRL_SUBJECT_TYPE_GROUP:
+		if (rule->hr_subject.hs_gip == NULL)
+			return (0);
+		break;
+	case HRL_SUBJECT_TYPE_LOGINCLASS:
+		if (rule->hr_subject.hs_loginclass == NULL)
+			return (0);
+		break;
+	case HRL_SUBJECT_TYPE_JAIL:
+		if (rule->hr_subject.hs_prison == NULL)
+			return (0);
+		break;
+	default:
+		panic("hrl_rule_fully_specified: unknown subject type %d",
+		    rule->hr_subject_type);
+	}
 	if (rule->hr_resource == HRL_RESOURCE_UNDEFINED)
 		return (0);
 	if (rule->hr_action == HRL_ACTION_UNDEFINED)
 		return (0);
 	if (rule->hr_amount == HRL_AMOUNT_UNDEFINED)
 		return (0);
-	if (rule->hr_per == HRL_SUBJECT_UNDEFINED)
+	if (rule->hr_per == HRL_SUBJECT_TYPE_UNDEFINED)
 		return (0);
 
 	return (1);
@@ -913,11 +1031,11 @@
 static struct hrl_rule *
 hrl_rule_from_string(char *rulestr)
 {
-	int error;
+	int error = 0;
 	char *subjectstr, *subject_idstr, *resourcestr, *actionstr,
 	     *amountstr, *perstr;
-	struct loginclass *lc;
 	struct hrl_rule *rule;
+	id_t id;
 
 	rule = hrl_rule_alloc(M_WAITOK);
 
@@ -929,32 +1047,64 @@
 	perstr = rulestr;
 
 	if (subjectstr == NULL || subjectstr[0] == '\0')
-		rule->hr_subject = HRL_SUBJECT_UNDEFINED;
+		rule->hr_subject_type = HRL_SUBJECT_TYPE_UNDEFINED;
 	else {
-		error = str2value(subjectstr, &rule->hr_subject, subjectnames);
+		error = str2value(subjectstr, &rule->hr_subject_type, subjectnames);
 		if (error)
 			goto out;
 	}
 
-	/*
-	 * Login classes don't have any ID.  Instead, we just put a pointer
-	 * to the 'struct loginclass' into the hr_subject_id field.
-	 */
-	if (rule->hr_subject == HRL_SUBJECT_LOGINCLASS) {
-		if (subject_idstr == NULL || subject_idstr[0] == '\0') {
-			rule->hr_subject_id = HRL_SUBJECT_ID_UNDEFINED;
-		} else {
-			lc = loginclass_find(subject_idstr);
-			rule->hr_subject_id = (long)lc;
-		}
+	if (subject_idstr == NULL || subject_idstr[0] == '\0') {
+		rule->hr_subject.hs_proc = NULL;
+		rule->hr_subject.hs_uip = NULL;
+		rule->hr_subject.hs_gip = NULL;
+		rule->hr_subject.hs_loginclass = NULL;
+		rule->hr_subject.hs_prison = NULL;
 	} else {
-		if (subject_idstr == NULL || subject_idstr[0] == '\0')
-			rule->hr_subject_id = HRL_SUBJECT_ID_UNDEFINED;
-		else {
-			error = str2id(subject_idstr, &rule->hr_subject_id);
+
+		/*
+		 * Loginclasses don't have any numerical ID's.
+		 */
+		if (rule->hr_subject_type != HRL_SUBJECT_TYPE_LOGINCLASS) {
+			error = str2id(subject_idstr, &id);
 			if (error)
 				goto out;
 		}
+		switch (rule->hr_subject_type) {
+		case HRL_SUBJECT_TYPE_UNDEFINED:
+			error = EINVAL;
+			goto out;
+		case HRL_SUBJECT_TYPE_PROCESS:
+			sx_assert(&allproc_lock, SA_LOCKED);
+			rule->hr_subject.hs_proc = pfind(id);
+			if (rule->hr_subject.hs_proc == NULL) {
+				error = ESRCH;
+				goto out;
+			}
+			PROC_UNLOCK(rule->hr_subject.hs_proc);
+			break;
+		case HRL_SUBJECT_TYPE_USER:
+			rule->hr_subject.hs_uip = uifind(id);
+			break;
+		case HRL_SUBJECT_TYPE_GROUP:
+			rule->hr_subject.hs_gip = gifind(id);
+			break;
+		case HRL_SUBJECT_TYPE_LOGINCLASS:
+			rule->hr_subject.hs_loginclass = loginclass_find(subject_idstr);
+			break;
+		case HRL_SUBJECT_TYPE_JAIL:
+			sx_xlock(&allprison_lock);
+			rule->hr_subject.hs_prison = prison_find(id);
+			if (rule->hr_subject.hs_prison == NULL) {
+				sx_xunlock(&allprison_lock);
+				error = ESRCH;
+				goto out;
+			}
+			break;
+               default:
+                       panic("hrl_rule_from_string: unknown subject type %d",
+                           rule->hr_subject_type);
+               }
 	}
 
 	if (resourcestr == NULL || resourcestr[0] == '\0')
@@ -983,17 +1133,13 @@
 	}
 
 	if (perstr == NULL || perstr[0] == '\0')
-		rule->hr_per = HRL_SUBJECT_UNDEFINED;
+		rule->hr_per = HRL_SUBJECT_TYPE_UNDEFINED;
 	else {
 		error = str2value(perstr, &rule->hr_per, subjectnames);
 		if (error)
 			goto out;
 	}
 
-	if (rule->hr_subject_id != HRL_SUBJECT_ID_UNDEFINED &&
-	    rule->hr_subject == HRL_SUBJECT_UNDEFINED)
-		goto out;
-
 out:
 	if (error) {
 		hrl_rule_release(rule);
@@ -1019,8 +1165,8 @@
 
 	KASSERT(hrl_rule_fully_specified(rule), ("rule not fully specified"));
 
-	if ((rule->hr_subject == HRL_SUBJECT_GROUP ||
-	    rule->hr_per == HRL_SUBJECT_GROUP) && !hrl_group_accounting)
+	if ((rule->hr_subject_type == HRL_SUBJECT_TYPE_GROUP ||
+	    rule->hr_per == HRL_SUBJECT_TYPE_GROUP) && !hrl_group_accounting)
 		return (EOPNOTSUPP);
 
 	if (rule->hr_action == HRL_ACTION_DELAY)
@@ -1031,101 +1177,79 @@
 	 */
 	hrl_rule_remove(rule);
 
-	switch (rule->hr_subject) {
-	case HRL_SUBJECT_PROCESS:
-		/*
-		 * The sx lock is to keep the process from going away.
-		 */
-		sx_slock(&proctree_lock);
-		p = pfind(rule->hr_subject_id);
-		if (p == NULL) {
-			sx_sunlock(&proctree_lock);
-			return (ESRCH);
-		}
-
-		PROC_UNLOCK(p);
+	switch (rule->hr_subject_type) {
+	case HRL_SUBJECT_TYPE_PROCESS:
+		p = rule->hr_subject.hs_proc;
+		KASSERT(p != NULL, ("hrl_rule_add: NULL proc"));
 		hrl_limit_add(&p->p_limits, rule);
-		sx_sunlock(&proctree_lock);
 		/*
 		 * In case of per-process rule, we don't have anything more
 		 * to do.
 		 */
 		return (0);
 
-	case HRL_SUBJECT_USER:
-		uip = uifind(rule->hr_subject_id);
-		KASSERT(uip != NULL, ("hrl_rule_add: uifind failed"));
+	case HRL_SUBJECT_TYPE_USER:
+		uip = rule->hr_subject.hs_uip;
+		KASSERT(uip != NULL, ("hrl_rule_add: NULL uip"));
 		hrl_limit_add(&uip->ui_limits, rule);
-		/*
-		 * Don't call uifree(2); we don't want the uidinfo
-		 * to go away, because the rule should stay there even
-		 * if there are no processes with that uid.  The same
-		 * applies to the cases below.
-		 */
 		break;
 
-	case HRL_SUBJECT_GROUP:
-		gip = gifind_existing(rule->hr_subject_id);
-		KASSERT(gip != NULL, ("hrl_rule_add: gifind failed"));
+	case HRL_SUBJECT_TYPE_GROUP:
+		gip = rule->hr_subject.hs_gip;
+		KASSERT(gip != NULL, ("hrl_rule_add: NULL gip"));
 		hrl_limit_add(&gip->gi_limits, rule);
 		break;
 
-	case HRL_SUBJECT_LOGINCLASS:
-		lc = (struct loginclass *)(long)rule->hr_subject_id;
+	case HRL_SUBJECT_TYPE_LOGINCLASS:
+		lc = rule->hr_subject.hs_loginclass;
+		KASSERT(lc != NULL, ("hrl_rule_add: NULL loginclass"));
 		hrl_limit_add(&lc->lc_limits, rule);
 		break;
 
-	case HRL_SUBJECT_JAIL:
-		sx_xlock(&allprison_lock);
-		pr = prison_find(rule->hr_subject_id);
-		if (pr == NULL) {
-			sx_xunlock(&allprison_lock);
-			return (ESRCH);
-		}
+	case HRL_SUBJECT_TYPE_JAIL:
+		pr = rule->hr_subject.hs_prison;
+		KASSERT(pr != NULL, ("hrl_rule_add: NULL pr"));
 		hrl_limit_add(&pr->pr_limits, rule);
-		sx_xunlock(&allprison_lock);
 		break;
 
 	default:
-		panic("hrl_rule_add_limits: unknown subject %d",
-		    rule->hr_subject);
+		panic("hrl_rule_add_limits: unknown subject type %d",
+		    rule->hr_subject_type);
 	}
 
 	/*
 	 * Now go through all the processes and add the new rule to the ones
 	 * it applies to.
 	 */
-	sx_slock(&proctree_lock);
+	sx_assert(&allproc_lock, SA_LOCKED);
 	FOREACH_PROC_IN_SYSTEM(p) {
 		cred = p->p_ucred;
-		switch (rule->hr_subject) {
-		case HRL_SUBJECT_USER:
-			if (cred->cr_uid == rule->hr_subject_id ||
-			    cred->cr_ruid == rule->hr_subject_id)
+		switch (rule->hr_subject_type) {
+		case HRL_SUBJECT_TYPE_USER:
+			if (cred->cr_uidinfo == rule->hr_subject.hs_uip ||
+			    cred->cr_ruidinfo == rule->hr_subject.hs_uip)
 				break;
 			continue;
-		case HRL_SUBJECT_GROUP:
-			if (groupmember(rule->hr_subject_id, cred))
+		case HRL_SUBJECT_TYPE_GROUP:
+			if (groupmember(rule->hr_subject.hs_gip->gi_gid, cred))
 				break;
 			continue;
-		case HRL_SUBJECT_LOGINCLASS:
-			lc = (struct loginclass *)(long)rule->hr_subject_id; /* XXX: This line is here to remove cc warning; investigate. */
-			if (p->p_loginclass == lc)
+		case HRL_SUBJECT_TYPE_LOGINCLASS:
+			if (p->p_loginclass == rule->hr_subject.hs_loginclass)
 				break;
 			continue;
-		case HRL_SUBJECT_JAIL:
+		case HRL_SUBJECT_TYPE_JAIL:
 			for (pr = cred->cr_prison; pr != NULL; pr = pr->pr_parent)
-				if (pr->pr_id == rule->hr_subject_id)
+				if (pr->pr_id == rule->hr_subject.hs_prison->pr_id)
 					break;
 			continue;
 		default:
-			panic("hrl_rule_add_limits: unknown subject %d",
-			    rule->hr_subject);
+			panic("hrl_rule_add_limits: unknown subject type %d",
+			    rule->hr_subject_type);
 		}
 
 		hrl_limit_add(&p->p_limits, rule);
 	}
-	sx_sunlock(&proctree_lock);
 
 	return (0);
 }
@@ -1148,15 +1272,10 @@
 	int error, found = 0;
 	struct proc *p;
 
-	if (filter->hr_subject == HRL_SUBJECT_PROCESS &&
-	    filter->hr_subject_id != HRL_SUBJECT_ID_UNDEFINED) {
-		sx_slock(&proctree_lock);
-		p = pfind(filter->hr_subject_id);
-		if (p == NULL)
-			return (ESRCH);
-		PROC_UNLOCK(p);
+	if (filter->hr_subject_type == HRL_SUBJECT_TYPE_PROCESS &&
+	    filter->hr_subject.hs_proc != NULL) {
+		p = filter->hr_subject.hs_proc;
 		found = hrl_limit_remove_matching(&p->p_limits, filter);
-		sx_sunlock(&proctree_lock);
 		if (found)
 			return (0);
 		return (ESRCH);
@@ -1172,13 +1291,12 @@
 	    (void *)&found);
 	KASSERT(error == 0, ("gi_limits_foreach failed"));
 
-	sx_slock(&proctree_lock);
+	sx_assert(&allproc_lock, SA_LOCKED);
 	FOREACH_PROC_IN_SYSTEM(p) {
 		found += hrl_limit_remove_matching(&p->p_limits, filter);
 		if (error == 0)
 			found = 1;
 	}
-	sx_sunlock(&proctree_lock);
 
 	if (found)
 		return (0);
@@ -1191,25 +1309,52 @@
 static void
 hrl_rule_to_sbuf(struct sbuf *sb, const struct hrl_rule *rule)
 {
-	if (rule->hr_subject == HRL_SUBJECT_LOGINCLASS) {
-		KASSERT(rule->hr_subject_id != HRL_SUBJECT_ID_UNDEFINED,
-		    ("rule->hr_subject_id != HRL_SUBJECT_ID_UNDEFINED"));
-		sbuf_printf(sb, "%s:%s:%s:%s=%jd",
-		    hrl_subject_name(rule->hr_subject),
-		    ((struct loginclass *)(long)rule->hr_subject_id)->lc_name,
-		    hrl_resource_name(rule->hr_resource),
-		    hrl_action_name(rule->hr_action),
-		    rule->hr_amount);
-	} else {
-		sbuf_printf(sb, "%s:%d:%s:%s=%jd",
-		    hrl_subject_name(rule->hr_subject),
-		    (int)rule->hr_subject_id,
-		    hrl_resource_name(rule->hr_resource),
-		    hrl_action_name(rule->hr_action),
-		    rule->hr_amount);
+
+	sbuf_printf(sb, "%s:", hrl_subject_type_name(rule->hr_subject_type));
+
+	switch (rule->hr_subject_type) {
+	case HRL_SUBJECT_TYPE_PROCESS:
+		if (rule->hr_subject.hs_proc == NULL)
+			sbuf_printf(sb, ":");
+		else
+			sbuf_printf(sb, "%d:", rule->hr_subject.hs_proc->p_pid);
+		break;
+	case HRL_SUBJECT_TYPE_USER:
+		if (rule->hr_subject.hs_uip == NULL)
+			sbuf_printf(sb, ":");
+		else
+			sbuf_printf(sb, "%d:", rule->hr_subject.hs_uip->ui_uid);
+		break;
+	case HRL_SUBJECT_TYPE_GROUP:
+		if (rule->hr_subject.hs_gip == NULL)
+			sbuf_printf(sb, ":");
+		else
+			sbuf_printf(sb, "%d:", rule->hr_subject.hs_gip->gi_gid);
+		break;
+	case HRL_SUBJECT_TYPE_LOGINCLASS:
+		if (rule->hr_subject.hs_loginclass == NULL)
+			sbuf_printf(sb, ":");
+		else
+			sbuf_printf(sb, "%s:", rule->hr_subject.hs_loginclass->lc_name);
+		break;
+	case HRL_SUBJECT_TYPE_JAIL:
+		if (rule->hr_subject.hs_prison == NULL)
+			sbuf_printf(sb, ":");
+		else
+			sbuf_printf(sb, "%d:", rule->hr_subject.hs_prison->pr_id);
+		break;
+	default:
+		panic("hrl_rule_to_sbuf: unknown subject type %d",
+		    rule->hr_subject_type);
 	}
-	if (rule->hr_per != rule->hr_subject)
-		sbuf_printf(sb, "/%s", hrl_subject_name(rule->hr_per));
+
+	sbuf_printf(sb, "%s:%s=%jd",
+	    hrl_resource_name(rule->hr_resource),
+	    hrl_action_name(rule->hr_action),
+	    rule->hr_amount);
+
+	if (rule->hr_per != rule->hr_subject_type)
+		sbuf_printf(sb, "/%s", hrl_subject_type_name(rule->hr_per));
 }
 
 /*
@@ -1273,87 +1418,6 @@
 	return (sb);
 }
 
-static int
-hrl_get_usage_pid(struct thread *td, id_t pid, struct sbuf **outputsbuf)
-{
-	struct proc *p;
-	struct hrl_usage usage;
-
-	if ((p = pfind(pid)) == NULL) {
-		if ((p = zpfind(pid)) == NULL)
-			return (ESRCH);
-	}
-	usage = p->p_usage;
-	PROC_UNLOCK(p);
-
-	*outputsbuf = hrl_usage_to_sbuf(&usage);
-
-	return (0);
-}
-
-static int
-hrl_get_usage_uid(struct thread *td, id_t uid, struct sbuf **outputsbuf)
-{
-	struct uidinfo *uip;
-
-	uip = uifind_existing(uid);
-	if (uip == NULL)
-		return (ESRCH);
-	*outputsbuf = hrl_usage_to_sbuf(&uip->ui_usage);
-	uifree(uip);
-
-	return (0);
-}
-
-static int
-hrl_get_usage_gid(struct thread *td, id_t gid, struct sbuf **outputsbuf)
-{
-	struct gidinfo *gip;
-
-	if (!hrl_group_accounting)
-		return (EOPNOTSUPP);
-
-	gip = gifind_existing(gid);
-	if (gip == NULL)
-		return (ESRCH);
-	*outputsbuf = hrl_usage_to_sbuf(&gip->gi_usage);
-	gifree(gip);
-
-	return (0);
-}
-
-static int
-hrl_get_usage_lc(struct thread *td, int lcp, struct sbuf **outputsbuf)
-{
-	struct loginclass *lc;
-
-	if (lcp == HRL_SUBJECT_ID_UNDEFINED)
-		return (EINVAL);
-
-	lc = (struct loginclass *)lcp;
-	*outputsbuf = hrl_usage_to_sbuf(&lc->lc_usage);
-
-	return (0);
-}
-
-static int
-hrl_get_usage_jid(struct thread *td, id_t jid, struct sbuf **outputsbuf)
-{
-	struct prison *pr;
-
-	sx_xlock(&allprison_lock);
-	pr = prison_find(jid);
-	if (pr == NULL) {
-		sx_xunlock(&allprison_lock);
-		return (ENOENT);
-	}
-	*outputsbuf = hrl_usage_to_sbuf(&pr->pr_usage);
-	prison_free(pr);
-	sx_xunlock(&allprison_lock);
-
-	return (0);
-}
-
 int
 hrl_get_usage(struct thread *td, struct hrl_get_usage_args *uap)
 {
@@ -1361,6 +1425,11 @@
 	char *inputstr;
 	struct hrl_rule *filter;
 	struct sbuf *outputsbuf = NULL;
+	struct proc *p;
+	struct uidinfo *uip;
+	struct gidinfo *gip;
+	struct loginclass *lc;
+	struct prison *pr;
 
 	error = hrl_read_inbuf(&inputstr, uap->inbufp, uap->inbuflen);
 	if (error)
@@ -1371,30 +1440,51 @@
 	if (filter == NULL)
 		return (EINVAL);
 
-	switch (filter->hr_subject) {
-	case HRL_SUBJECT_PROCESS:
-		error = hrl_get_usage_pid(td, filter->hr_subject_id,
-		    &outputsbuf);
+	switch (filter->hr_subject_type) {
+	case HRL_SUBJECT_TYPE_PROCESS:
+		p = filter->hr_subject.hs_proc;
+		if (p == NULL) {
+			error = EINVAL;
+			goto out;
+		}
+		outputsbuf = hrl_usage_to_sbuf(&p->p_usage);
 		break;
-	case HRL_SUBJECT_USER:
-		error = hrl_get_usage_uid(td, filter->hr_subject_id,
-		    &outputsbuf);
+	case HRL_SUBJECT_TYPE_USER:
+		uip = filter->hr_subject.hs_uip;
+		if (uip == NULL) {
+			error = EINVAL;
+			goto out;
+		}
+		outputsbuf = hrl_usage_to_sbuf(&uip->ui_usage);
 		break;
-	case HRL_SUBJECT_GROUP:
-		error = hrl_get_usage_gid(td, filter->hr_subject_id,
-		    &outputsbuf);
+	case HRL_SUBJECT_TYPE_GROUP:
+		gip = filter->hr_subject.hs_gip;
+		if (gip == NULL) {
+			error = EINVAL;
+			goto out;
+		}
+		outputsbuf = hrl_usage_to_sbuf(&gip->gi_usage);
 		break;
-	case HRL_SUBJECT_LOGINCLASS:
-		error = hrl_get_usage_lc(td, filter->hr_subject_id,
-		    &outputsbuf);
+	case HRL_SUBJECT_TYPE_LOGINCLASS:
+		lc = filter->hr_subject.hs_loginclass;
+		if (lc == NULL) {
+			error = EINVAL;
+			goto out;
+		}
+		outputsbuf = hrl_usage_to_sbuf(&lc->lc_usage);
 		break;
-	case HRL_SUBJECT_JAIL:
-		error = hrl_get_usage_jid(td, filter->hr_subject_id,
-		    &outputsbuf);
+	case HRL_SUBJECT_TYPE_JAIL:
+		pr = filter->hr_subject.hs_prison;
+		if (pr == NULL) {
+			error = EINVAL;
+			goto out;
+		}
+		outputsbuf = hrl_usage_to_sbuf(&pr->pr_usage);
 		break;
 	default:
 		error = EINVAL;
 	}
+out:
 	hrl_rule_release(filter);
 	if (error)
 		return (error);
@@ -1438,17 +1528,20 @@
 	if (error)
 		return (error);
 
+	sx_slock(&allproc_lock);
 	filter = hrl_rule_from_string(inputstr);
 	free(inputstr, M_HRL);
-	if (filter == NULL)
+	if (filter == NULL) {
+		sx_sunlock(&allproc_lock);
 		return (EINVAL);
+	}
 
 again:
 	buf = malloc(bufsize, M_HRL, M_WAITOK);
 	sb = sbuf_new(NULL, buf, bufsize, SBUF_FIXEDLEN);
 	KASSERT(sb != NULL, ("sbuf_new failed"));
 
-	sx_slock(&proctree_lock);
+	sx_assert(&allproc_lock, SA_LOCKED);
 	FOREACH_PROC_IN_SYSTEM(p) {
 		mtx_lock(&hrl_lock);
 		LIST_FOREACH(limit, &p->p_limits, hl_next) {
@@ -1456,7 +1549,7 @@
 			 * Non-process rules will be added to the buffer later.
 			 * Adding them here would result in duplicated output.
 			 */
-			if (limit->hl_rule->hr_subject != HRL_SUBJECT_PROCESS)
+			if (limit->hl_rule->hr_subject_type != HRL_SUBJECT_TYPE_PROCESS)
 				continue;
 			if (!hrl_rule_matches(limit->hl_rule, filter))
 				continue;
@@ -1465,7 +1558,6 @@
 		}
 		mtx_unlock(&hrl_lock);
 	}
-	sx_sunlock(&proctree_lock);
 
 	mtx_lock(&hrl_lock);
 	loginclass_limits_foreach(hrl_get_rules_callback, filter, sb);
@@ -1488,6 +1580,7 @@
 	error = hrl_write_outbuf(sb, uap->outbufp, uap->outbuflen);
 
 	hrl_rule_release(filter);
+	sx_sunlock(&allproc_lock);
 	free(buf, M_HRL);
 	return (error);
 }
@@ -1501,30 +1594,33 @@
 	struct sbuf *sb;
 	struct hrl_rule *filter;
 	struct hrl_limit *limit;
-	struct proc *p;
 
 	error = hrl_read_inbuf(&inputstr, uap->inbufp, uap->inbuflen);
 	if (error)
 		return (error);
 
+	sx_slock(&allproc_lock);
 	filter = hrl_rule_from_string(inputstr);
 	free(inputstr, M_HRL);
-	if (filter == NULL)
+	if (filter == NULL) {
+		sx_sunlock(&allproc_lock);
 		return (EINVAL);
+	}
 
-	if (filter->hr_subject == HRL_SUBJECT_UNDEFINED) {
+	if (filter->hr_subject_type == HRL_SUBJECT_TYPE_UNDEFINED) {
 		hrl_rule_release(filter);
+		sx_sunlock(&allproc_lock);

>>> TRUNCATED FOR MAIL (1000 lines) <<<



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