PERFORCE change 108414 for review

[Todd Miller]

http://perforce.freebsd.org/chv.cgi?CH=108414

Change 108414 by millert at millert_macbook on 2006/10/25 19:18:23

	Replace the migscs load lock with an rwlock around access
	to msgid2class.  Now that we actually free up the old
	contents on reload we need to make sure there are no
	use-after-free issues.
	
	Fix some signedness mismatches with the mig class data.

Affected files ...

.. //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/sedarwin/ss/init.c#2 edit
.. //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/sedarwin/ss/mach_av.c#5 edit
.. //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/sedarwin/ss/services.h#2 edit

Differences ...

==== //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/sedarwin/ss/init.c#2 (text+ko) ====

@@ -49,7 +49,7 @@
 
 	policy_rwlock = lck_rw_alloc_init(ss_lck_grp, ss_lck_attr);
 	load_sem = lck_mtx_alloc_init(ss_lck_grp, ss_lck_attr);
-	migscs_load_lock = lck_mtx_alloc_init(ss_lck_grp, ss_lck_attr);
+	migscs_rwlock = lck_rw_alloc_init(ss_lck_grp, ss_lck_attr);
 
 	lck_attr_free(ss_lck_attr);
 	lck_grp_attr_free(ss_lck_grp_attr);

==== //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/sedarwin/ss/mach_av.c#5 (text+ko) ====

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2005 SPARTA, Inc.
+ * Copyright (c) 2005-2006 SPARTA, Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -46,15 +46,15 @@
 
 struct msgid_classinfo
 {
-	int nmsgids;
-	int *msgids;
-	int nclasses;
-	int classes[0];		/* actually larger */
+	u32 nmsgids;
+	u32 *msgids;
+	u32 nclasses;
+	u32 classes[0];		/* actually larger */
 };
 
 static struct hashtab *msgid2class;
 
-lck_mtx_t *migscs_load_lock;
+lck_rw_t *migscs_rwlock;
 
 static int
 msgid_destroy(void *key, void *datum, void *arg)
@@ -76,16 +76,16 @@
 {
 	struct msgid_classinfo *cinfo;
 	struct hashtab *ht, *oht;
-	int error, entries, i, msgid, nclasses, size;
-	int *p, *ep;
+	u32 entries, i, msgid, nclasses, size, *p, *ep;
+	int error;
 
 	ht = hashtab_create(msgid_hash, msgid_cmp, 31337);
 	if (ht == NULL)
 		return (-1);
 
 	error = 0;
-	p = (int *)tdata;
-	ep = (int *)((char *)tdata + tsize);
+	p = (u32 *)tdata;
+	ep = (u32 *)((char *)tdata + tsize);
 	for (entries = 0; p < ep; entries++) {
 		if (p + 3 > ep)
 			goto bad;
@@ -101,7 +101,7 @@
 		 * so we can free things up with a map operation later.
 		 */
 		cinfo = sebsd_malloc(sizeof(*cinfo) +
-		    (nclasses + size) * sizeof(int), M_SEBSD, M_WAITOK);
+		    (nclasses + size) * sizeof(u32), M_SEBSD, M_WAITOK);
 		cinfo->nclasses = nclasses;
 		for (i = 0; i < nclasses; i++)
 			cinfo->classes[i] = *p++;
@@ -116,17 +116,16 @@
 		}
 	}
 
-	printf("security class to subsystem table: %d classes\n", entries);
+	printf("security: class to subsystem table: %d classes\n", entries);
 
 	/*
 	 * Swap the old message id to class mapping with the new one
 	 * and free the old.
 	 */
-	/* XXX - need rwlock for msgid2class  */
-	lck_mtx_lock(migscs_load_lock);
+	lck_rw_lock_exclusive(migscs_rwlock);
 	oht = msgid2class;
 	msgid2class = ht;
-	lck_mtx_unlock(migscs_load_lock);
+	lck_rw_unlock_exclusive(migscs_rwlock);
 	if (oht != NULL) {
 		hashtab_map(oht, msgid_destroy, NULL);
 		hashtab_destroy(oht);
@@ -158,28 +157,38 @@
 sebsd_ipc_check_method1(int subj, int obj, int msgid)
 {
 	struct msgid_classinfo *mcl;
-	u32 perms;
-	int cl;
+	u16 tclass;
+	u32 perms, cl;
+	int msgid_norm;
 
 	/*
 	 * Return allowed for messages in an unknown subsystem.
 	 * Instead, we probably should make a check against a
 	 * new permission to be added to mach_port for this purpose.
 	 */
-	/* XXX - need rwlock for msgid2class  */
+	lck_rw_lock_shared(migscs_rwlock);
 	mcl = hashtab_search(msgid2class, &msgid); 
-	if (mcl == NULL)
-		return 0;
+	if (mcl == NULL) {
+		lck_rw_unlock_shared(migscs_rwlock);
+		return (0);
+	}
 
-	cl = (msgid - mcl->msgids[0]) / (8 * sizeof(u32));
+	/*
+	 * Normalize msgid relative to base id and find class index and value.
+	 */
+	msgid_norm = msgid - mcl->msgids[0];
+	cl = msgid_norm / (8 * sizeof(u32));
 	if (cl >= mcl->nclasses) {
 		/* bad message */
+		lck_rw_unlock_shared(migscs_rwlock);
 		if (selinux_enforcing)
 			return (EACCES);
 		else
 			return (0);
 	}
+	tclass = (u16)mcl->classes[cl];
+	lck_rw_unlock_shared(migscs_rwlock);
 
-	perms = (u32)1 << (msgid - mcl->msgids[0] - (cl * 8 * sizeof(u32)));
-	return avc_has_perm(subj, obj, mcl->classes[cl], perms, NULL);
+	perms = (u32)1 << (msgid_norm - (cl * 8 * sizeof(u32)));
+	return avc_has_perm(subj, obj, tclass, perms, NULL);
 }

==== //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/sedarwin/ss/services.h#2 (text+ko) ====

@@ -21,8 +21,8 @@
  * Security server locks, as allocated by security_init().
  */
 extern lck_rw_t *policy_rwlock;
+extern lck_rw_t *migscs_rwlock;
 extern lck_mtx_t *load_sem;
-extern lck_mtx_t *migscs_load_lock;
 
 extern const char *security_class_to_string(u16 tclass);