Index: mac_bsdextended.c
===================================================================
RCS file: /usr/store/mlaier/fcvs/src/sys/security/mac_bsdextended/mac_bsdextended.c,v
retrieving revision 1.24
diff -u -r1.24 mac_bsdextended.c
--- mac_bsdextended.c	22 Oct 2004 11:15:47 -0000	1.24
+++ mac_bsdextended.c	26 Jan 2005 02:42:25 -0000
@@ -1,4 +1,5 @@
 /*-
+ * Copyright (c) 2005 Tom Rhodes
  * Copyright (c) 1999-2002 Robert N. M. Watson
  * Copyright (c) 2001-2004 Networks Associates Technology, Inc.
  * All rights reserved.
@@ -39,7 +40,6 @@
  * "BSD Extended" MAC policy, allowing the administrator to impose
  * mandatory rules regarding users and some system objects.
  *
- * XXX: Much locking support required here.
  */
 
 #include <sys/types.h>
@@ -47,9 +47,11 @@
 #include <sys/acl.h>
 #include <sys/conf.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/mac.h>
 #include <sys/malloc.h>
 #include <sys/mount.h>
+#include <sys/mutex.h>
 #include <sys/proc.h>
 #include <sys/systm.h>
 #include <sys/sysproto.h>
@@ -72,6 +74,8 @@
 
 #include <security/mac_bsdextended/mac_bsdextended.h>
 
+static struct mtx mac_bsdextended_mtx;
+
 SYSCTL_DECL(_security_mac);
 
 SYSCTL_NODE(_security_mac, OID_AUTO, bsdextended, CTLFLAG_RW, 0,
@@ -145,16 +149,26 @@
 		return (EINVAL);
 
 	index = name[0];
+	/*
+	 * XXX: test against rule_slots unlocked here - we check again on
+	 *	the actual add operation.  Rewrite the border check against
+	 *	index directly.
+	 */
 	if (index < 0 || index > rule_slots + 1)
 		return (ENOENT);
-	if (rule_slots >= MAC_BSDEXTENDED_MAXRULES)
+        if (index > MAC_BSDEXTENDED_MAXRULES)
 		return (ENOENT);
 
 	if (req->oldptr) {
+		/* read unlocked - it's safe (enough) here */
 		if (rules[index] == NULL)
 			return (ENOENT);
 
-		error = SYSCTL_OUT(req, rules[index], sizeof(*rules[index]));
+		mtx_lock(&mac_bsdextended_mtx);
+		temprule = *rules[index];
+		mtx_unlock(&mac_bsdextended_mtx);
+
+		error = SYSCTL_OUT(req, &temprule, sizeof(temprule));
 		if (error)
 			return (error);
 	}
@@ -162,12 +176,18 @@
 	if (req->newptr) {
 		if (req->newlen == 0) {
 			/* printf("deletion\n"); */
+			mtx_lock(&mac_bsdextended_mtx);
 			ruleptr = rules[index];
-			if (ruleptr == NULL)
+			if (ruleptr == NULL) {
+				mtx_unlock(&mac_bsdextended_mtx);
 				return (ENOENT);
+			}
 			rule_count--;
+			if (index == rule_slots - 1)
+                        	rule_slots--;
 			rules[index] = NULL;
 			FREE(ruleptr, M_MACBSDEXTENDED);
+			mtx_unlock(&mac_bsdextended_mtx);
 			return(0);
 		}
 		error = SYSCTL_IN(req, &temprule, sizeof(temprule));
@@ -178,20 +198,22 @@
 		if (error)
 			return (error);
 
+		MALLOC(ruleptr, struct mac_bsdextended_rule *,
+		    sizeof(*ruleptr), M_MACBSDEXTENDED, M_WAITOK | M_ZERO);
+		mtx_lock(&mac_bsdextended_mtx);
 		if (rules[index] == NULL) {
 			/* printf("addition\n"); */
-			MALLOC(ruleptr, struct mac_bsdextended_rule *,
-			    sizeof(*ruleptr), M_MACBSDEXTENDED, M_WAITOK |
-			    M_ZERO);
 			*ruleptr = temprule;
 			rules[index] = ruleptr;
-			if (index+1 > rule_slots)
-				rule_slots = index+1;
+			if (index + 1 > rule_slots)
+				rule_slots = index + 1;
 			rule_count++;
 		} else {
+			FREE(ruleptr, M_MACBSDEXTENDED);
 			/* printf("replacement\n"); */
 			*rules[index] = temprule;
 		}
+		mtx_unlock(&mac_bsdextended_mtx);
 	}
 
 	return (0);
@@ -203,17 +225,19 @@
 static void
 mac_bsdextended_init(struct mac_policy_conf *mpc)
 {
-
 	/* Initialize ruleset lock. */
+	mtx_init(&mac_bsdextended_mtx, "mac_bsdextended lock", NULL, MTX_DEF);
+
 	/* Register dynamic sysctl's for rules. */
 }
 
 static void
 mac_bsdextended_destroy(struct mac_policy_conf *mpc)
 {
+	/* Destroy ruleset lock. */
+	mtx_destroy(&mac_bsdextended_mtx);
 
 	/* Tear down sysctls. */
-	/* Destroy ruleset lock. */
 }
 
 static int
@@ -225,6 +249,7 @@
 	/*
 	 * Is there a subject match?
 	 */
+	mtx_assert(&mac_bsdextended_mtx, MA_OWNED);
 	if (rule->mbr_subject.mbi_flags & MBI_UID_DEFINED) {
 		match =  (rule->mbr_subject.mbi_uid == cred->cr_uid ||
 		    rule->mbr_subject.mbi_uid == cred->cr_ruid ||
@@ -301,6 +326,7 @@
 	if (suser_cred(cred, 0) == 0)
 		return (0);
 
+	mtx_lock(&mac_bsdextended_mtx);
 	for (i = 0; i < rule_slots; i++) {
 		if (rules[i] == NULL)
 			continue;
@@ -318,10 +344,12 @@
 		    object_gid, acc_mode);
 		if (error == EJUSTRETURN)
 			break;
-		if (error)
+		if (error) {
+			mtx_unlock(&mac_bsdextended_mtx);
 			return (error);
+		}
 	}
-
+	mtx_unlock(&mac_bsdextended_mtx);
 	return (0);
 }