PERFORCE change 85419 for review

Robert Watson rwatson at FreeBSD.org
Mon Oct 17 08:27:49 GMT 2005


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

Change 85419 by rwatson at rwatson_peppercorn on 2005/10/17 08:27:38

	Lock down the event->class mapping table using a mutex.  Staticize
	the table and mutex.  Further annotate functions.  This code may
	well now be safe on SMP.
	
	Cleanups -- style, remove unused code, whitespace, etc.
	
	Assert copyright.

Affected files ...

.. //depot/projects/trustedbsd/audit3/sys/security/audit/kern_bsm_klib.c#15 edit

Differences ...

==== //depot/projects/trustedbsd/audit3/sys/security/audit/kern_bsm_klib.c#15 (text+ko) ====

@@ -1,5 +1,7 @@
 /*
- * Copyright (c) 2004 Apple Computer, Inc. All rights reserved.
+ * Copyright (c) 2004 Apple Computer, Inc.
+ * Copyright (c) 2005 Robert N. M. Watson
+ * All rights reserved.
  *
  * @APPLE_LICENSE_HEADER_START@
  * 
@@ -41,10 +43,7 @@
 /*
  * Hash table functions for the audit event number to event class mask
  * mapping.
- *
- * XXXAUDIT: Locking?
  */
-
 #define EVCLASSMAP_HASH_TABLE_SIZE 251
 struct evclass_elem {
 	au_event_t event;
@@ -55,22 +54,31 @@
 	LIST_HEAD(, evclass_elem) head;
 };
 
-struct evclass_list evclass_hash[EVCLASSMAP_HASH_TABLE_SIZE];
+static struct mtx evclass_mtx;
+static struct evclass_list evclass_hash[EVCLASSMAP_HASH_TABLE_SIZE];
 
-au_class_t au_event_class(au_event_t event)
+/*
+ * Look up the class for an audit event in the class mapping table.
+ */
+au_class_t
+au_event_class(au_event_t event)
 {
-
 	struct evclass_list *evcl;
 	struct evclass_elem *evc;
+	au_class_t class;
 
+	mtx_lock(&evclass_mtx);
 	evcl = &evclass_hash[event % EVCLASSMAP_HASH_TABLE_SIZE];
-
-	/* If an entry at our hash location matches the event, just return */
+	class = AU_NULL;
 	LIST_FOREACH(evc, &evcl->head, entry) {
-		if (evc->event == event)
-			return (evc->class);
+		if (evc->event == event) {
+			class = evc->class;
+			goto out;
+		}
 	}
-	return (AU_NULL);
+out:
+	mtx_unlock(&evclass_mtx);
+	return (class);
 }
 
 /* 
@@ -79,71 +87,83 @@
  * XXX There is currently no constraints placed on the number of mappings.
  *     May want to either limit to a number, or in terms of memory usage.
  */
-void au_evclassmap_insert(au_event_t event, au_class_t class) 
+void
+au_evclassmap_insert(au_event_t event, au_class_t class) 
 {
 	struct evclass_list *evcl;
-	struct evclass_elem *evc;
+	struct evclass_elem *evc, *evc_new;
+
+	/*
+	 * Pessimistically, always allocate storage before acquiring mutex.
+	 * Free if there is already a mapping for this event.
+	 */
+	evc_new = malloc(sizeof(*evc), M_AUDIT, M_WAITOK);
 
+	mtx_lock(&evclass_mtx);
 	evcl = &evclass_hash[event % EVCLASSMAP_HASH_TABLE_SIZE];
-
 	LIST_FOREACH(evc, &evcl->head, entry) {
 		if (evc->event == event) {
 			evc->class = class;
+			mtx_unlock(&evclass_mtx);
+			free(evc_new, M_AUDIT);
 			return;
 		}
 	}
-	evc = malloc(sizeof(*evc), M_AUDIT, M_WAITOK);
-	if (evc == NULL) {
-		return;
-	}
+	evc = evc_new;
 	evc->event = event;
 	evc->class = class;
 	LIST_INSERT_HEAD(&evcl->head, evc, entry);
-		
+	mtx_unlock(&evclass_mtx);
 }
 
-void au_evclassmap_init(void) 
+void
+au_evclassmap_init(void) 
 {
 	int i;
-	for (i = 0; i < EVCLASSMAP_HASH_TABLE_SIZE; i++) {
+
+	mtx_init(&evclass_mtx, "evclass_mtx", NULL, MTX_DEF);
+	for (i = 0; i < EVCLASSMAP_HASH_TABLE_SIZE; i++)
 		LIST_INIT(&evclass_hash[i].head);
-	}
 
-	/* Set up the initial event to class mapping for system calls.  */ 
+	/*
+	 * Set up the initial event to class mapping for system calls.
+	 *
+	 * XXXRW: Really, this should walk all possible audit events, not all
+	 * native ABI system calls, as there may be audit events reachable
+	 * only through non-native system calls.  It also seems a shame to
+	 * frob the mutex this early.
+	 */ 
 	for (i = 0; i < SYS_MAXSYSCALL; i++) {
-		if (sysent[i].sy_auevent != AUE_NULL) {
+		if (sysent[i].sy_auevent != AUE_NULL)
 			au_evclassmap_insert(sysent[i].sy_auevent, AU_NULL);
-		}
 	}
-	
 }
 
 /*
  * Check whether an event is aditable by comparing the mask of classes this
  * event is part of against the given mask.
- *
  */
-int au_preselect(au_event_t event, au_mask_t *mask_p, int sorf)
+int
+au_preselect(au_event_t event, au_mask_t *mask_p, int sorf)
 {
 	au_class_t effmask = 0;
 	au_class_t ae_class;
 
-	if(mask_p == NULL)
+	if (mask_p == NULL)
 		return (-1);
 
 	ae_class = au_event_class(event);
+
 	/* 
 	 * Perform the actual check of the masks against the event.
 	 */
-	if(sorf & AU_PRS_SUCCESS) {
+	if (sorf & AU_PRS_SUCCESS)
 		effmask |= (mask_p->am_success & ae_class);
-	}
                         
-	if(sorf & AU_PRS_FAILURE) {
+	if (sorf & AU_PRS_FAILURE)
 		effmask |= (mask_p->am_failure & ae_class);
-	}
         
-	if(effmask)
+	if (effmask)
 		return (1);
 	else 
 		return (0);
@@ -152,12 +172,14 @@
 /*
  * Convert sysctl names and present arguments to events
  */
-au_event_t ctlname_to_sysctlevent(int name[], uint64_t valid_arg) {
+au_event_t
+ctlname_to_sysctlevent(int name[], uint64_t valid_arg)
+{
 
 	/* can't parse it - so return the worst case */
 	if ((valid_arg & (ARG_CTLNAME | ARG_LEN)) != 
 	                 (ARG_CTLNAME | ARG_LEN))
-		return AUE_SYSCTL;
+		return (AUE_SYSCTL);
 
 	switch (name[0]) {
 	/* non-admin "lookups" treat them special */
@@ -174,7 +196,7 @@
 	case KERN_SAVED_IDS:
 	case KERN_OSRELDATE:
 	case KERN_DUMMY:
-		return AUE_SYSCTL_NONADMIN;
+		return (AUE_SYSCTL_NONADMIN);
 
 	/* only treat the changeable controls as admin */
 	case KERN_MAXVNODES:
@@ -185,26 +207,26 @@
 	case KERN_HOSTID:
 	case KERN_SECURELVL:
 	case KERN_HOSTNAME:
-case KERN_VNODE:
-case KERN_PROC:
-case KERN_FILE:
-case KERN_PROF:
-case KERN_NISDOMAINNAME:
-case KERN_UPDATEINTERVAL:
-case KERN_NTP_PLL:
+	case KERN_VNODE:
+	case KERN_PROC:
+	case KERN_FILE:
+	case KERN_PROF:
+	case KERN_NISDOMAINNAME:
+	case KERN_UPDATEINTERVAL:
+	case KERN_NTP_PLL:
 	case KERN_BOOTFILE:
 	case KERN_DUMPDEV:
-case KERN_IPC:
-case KERN_PS_STRINGS:
-case KERN_USRSTACK:
+	case KERN_IPC:
+	case KERN_PS_STRINGS:
+	case KERN_USRSTACK:
 	case KERN_LOGSIGEXIT:
-case KERN_IOV_MAX:
-case KERN_MAXID:
-		return (valid_arg & ARG_VALUE) ?
-			AUE_SYSCTL : AUE_SYSCTL_NONADMIN;
+	case KERN_IOV_MAX:
+	case KERN_MAXID:
+		return ((valid_arg & ARG_VALUE) ?
+			AUE_SYSCTL : AUE_SYSCTL_NONADMIN);
 
 	default:
-		return AUE_SYSCTL;
+		return (AUE_SYSCTL);
 	}
 	/* NOTREACHED */
 }
@@ -213,52 +235,66 @@
  * Convert an open flags specifier into a specific type of open event for 
  * auditing purposes.
  */
-au_event_t flags_and_error_to_openevent(int oflags, int error) {
+au_event_t
+flags_and_error_to_openevent(int oflags, int error) {
 	au_event_t aevent;
 
 	/* Need to check only those flags we care about. */
 	oflags = oflags & (O_RDONLY | O_CREAT | O_TRUNC | O_RDWR | O_WRONLY);
 
-	/* These checks determine what flags are on with the condition
-	 * that ONLY that combination is on, and no other flags are on.
+	/*
+	 * These checks determine what flags are on with the condition that
+	 * ONLY that combination is on, and no other flags are on.
 	 */
 	switch (oflags) {
 	case O_RDONLY:
 		aevent = AUE_OPEN_R;
 		break;
+
 	case (O_RDONLY | O_CREAT):
 		aevent = AUE_OPEN_RC;
 		break;
+
 	case (O_RDONLY | O_CREAT | O_TRUNC):
 		aevent = AUE_OPEN_RTC;
 		break;
+
 	case (O_RDONLY | O_TRUNC):
 		aevent = AUE_OPEN_RT;
 		break;
+
 	case O_RDWR:
 		aevent = AUE_OPEN_RW;
 		break;
+
 	case (O_RDWR | O_CREAT):
 		aevent = AUE_OPEN_RWC;
 		break;
+
 	case (O_RDWR | O_CREAT | O_TRUNC):
 		aevent = AUE_OPEN_RWTC;
 		break;
+
 	case (O_RDWR | O_TRUNC):
 		aevent = AUE_OPEN_RWT;
 		break;
+
 	case O_WRONLY:
 		aevent = AUE_OPEN_W;
 		break;
+
 	case (O_WRONLY | O_CREAT):
 		aevent = AUE_OPEN_WC;
 		break;
+
 	case (O_WRONLY | O_CREAT | O_TRUNC):
 		aevent = AUE_OPEN_WTC;
 		break;
+
 	case (O_WRONLY | O_TRUNC):
 		aevent = AUE_OPEN_WT;
 		break;
+
 	default:
 		aevent = AUE_OPEN;
 		break;
@@ -279,107 +315,132 @@
 		if (error == ENOENT)
 			aevent = AUE_OPEN;
 	}
-	return aevent;
+	return (aevent);
 }
 
-/* Convert a MSGCTL command to a specific event. */
-int msgctl_to_event(int cmd)
+/*
+ * Convert a MSGCTL command to a specific event.
+ */
+int
+msgctl_to_event(int cmd)
 {
+
 	switch (cmd) {
 	case IPC_RMID:
-		return AUE_MSGCTL_RMID;
+		return (AUE_MSGCTL_RMID);
+
 	case IPC_SET:
-		return AUE_MSGCTL_SET;
+		return (AUE_MSGCTL_SET);
+
 	case IPC_STAT:
-		return AUE_MSGCTL_STAT;
+		return (AUE_MSGCTL_STAT);
+
 	default:
-		return AUE_MSGCTL;
-			/* We will audit a bad command */
+		/* We will audit a bad command */
+		return (AUE_MSGCTL);
 	}
 }
 
-/* Convert a SEMCTL command to a specific event. */
-int semctl_to_event(int cmd)
+/*
+ * Convert a SEMCTL command to a specific event.
+ */
+int
+semctl_to_event(int cmd)
 {
+
 	switch (cmd) {
 	case GETALL:
-		return AUE_SEMCTL_GETALL;
+		return (AUE_SEMCTL_GETALL);
+
 	case GETNCNT:
-		return AUE_SEMCTL_GETNCNT;
+		return (AUE_SEMCTL_GETNCNT);
+
 	case GETPID:
-		return AUE_SEMCTL_GETPID;
+		return (AUE_SEMCTL_GETPID);
+
 	case GETVAL:
-		return AUE_SEMCTL_GETVAL;
+		return (AUE_SEMCTL_GETVAL);
+
 	case GETZCNT:
-		return AUE_SEMCTL_GETZCNT;
+		return (AUE_SEMCTL_GETZCNT);
+
 	case IPC_RMID:
-		return AUE_SEMCTL_RMID;
+		return (AUE_SEMCTL_RMID);
+
 	case IPC_SET:
-		return AUE_SEMCTL_SET;
+		return (AUE_SEMCTL_SET);
+
 	case SETALL:
-		return AUE_SEMCTL_SETALL;
+		return (AUE_SEMCTL_SETALL);
+
 	case SETVAL:
-		return AUE_SEMCTL_SETVAL;
+		return (AUE_SEMCTL_SETVAL);
+
 	case IPC_STAT:
-		return AUE_SEMCTL_STAT;
+		return (AUE_SEMCTL_STAT);
+
 	default:
-		return AUE_SEMCTL;
-				/* We will audit a bad command */
+		/* We will audit a bad command */
+		return (AUE_SEMCTL);
 	}
 }
 
-/* Convert a command for the auditon() system call to a audit event. */
-int auditon_command_event(int cmd)
+/*
+ * Convert a command for the auditon() system call to a audit event.
+ */
+int
+auditon_command_event(int cmd)
 {
+
 	switch(cmd) {
 	case A_GETPOLICY:
-		return AUE_AUDITON_GPOLICY;
-		break;
+		return (AUE_AUDITON_GPOLICY);
+
 	case A_SETPOLICY:
-		return AUE_AUDITON_SPOLICY;
-		break;
+		return (AUE_AUDITON_SPOLICY);
+
 	case A_GETKMASK:
-		return AUE_AUDITON_GETKMASK;
-		break;
+		return (AUE_AUDITON_GETKMASK);
+
 	case A_SETKMASK:
-		return AUE_AUDITON_SETKMASK;
-		break;
+		return (AUE_AUDITON_SETKMASK);
+
 	case A_GETQCTRL:
-		return AUE_AUDITON_GQCTRL;
-		break;
+		return (AUE_AUDITON_GQCTRL);
+
 	case A_SETQCTRL:
-		return AUE_AUDITON_SQCTRL;
-		break;
+		return (AUE_AUDITON_SQCTRL);
+
 	case A_GETCWD:
-		return AUE_AUDITON_GETCWD;
-		break;
+		return (AUE_AUDITON_GETCWD);
+
 	case A_GETCAR:
-		return AUE_AUDITON_GETCAR;
-		break;
+		return (AUE_AUDITON_GETCAR);
+
 	case A_GETSTAT:
-		return AUE_AUDITON_GETSTAT;
-		break;
+		return (AUE_AUDITON_GETSTAT);
+
 	case A_SETSTAT:
-		return AUE_AUDITON_SETSTAT;
-		break;
+		return (AUE_AUDITON_SETSTAT);
+
 	case A_SETUMASK:
-		return AUE_AUDITON_SETUMASK;
-		break;
+		return (AUE_AUDITON_SETUMASK);
+
 	case A_SETSMASK:
-		return AUE_AUDITON_SETSMASK;
-		break;
+		return (AUE_AUDITON_SETSMASK);
+
 	case A_GETCOND:
-		return AUE_AUDITON_GETCOND;
-		break;
+		return (AUE_AUDITON_GETCOND);
+
 	case A_SETCOND:
-		return AUE_AUDITON_SETCOND;
-		break;
+		return (AUE_AUDITON_SETCOND);
+
 	case A_GETCLASS:
-		return AUE_AUDITON_GETCLASS;
-		break;
+		return (AUE_AUDITON_GETCLASS);
+
 	case A_SETCLASS:
-		return AUE_AUDITON_SETCLASS;
-		break;
+		return (AUE_AUDITON_SETCLASS);
+
 	case A_GETPINFO:
 	case A_SETPMASK:
 	case A_SETFSIZE:
@@ -388,8 +449,7 @@
 	case A_GETKAUDIT:
 	case A_SETKAUDIT:
 	default:
-		return AUE_AUDITON;	/* No special record */
-		break;
+		return (AUE_AUDITON);	/* No special record */
 	}
 }
 
@@ -407,12 +467,8 @@
 void
 canon_path(struct thread *td, char *path, char *cpath)
 {
-
 	char *bufp;
 	char *retbuf, *freebuf;
-#if 0
-	int len;
-#endif
 	struct vnode *vnp;
 	struct filedesc *fdp;
 	int error, vfslocked;
To Unsubscribe: send mail to majordomo at trustedbsd.org
with "unsubscribe trustedbsd-cvs" in the body of the message



More information about the trustedbsd-cvs mailing list