PERFORCE change 108505 for review

Todd Miller millert at FreeBSD.org
Thu Oct 26 19:20:40 UTC 2006


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

Change 108505 by millert at millert_macbook on 2006/10/26 19:19:45

	Allocate audit structures with the zone allocator instead
	of kalloc().  Unlike kalloc(), zalloc() cannot fail in
	blocking mode.

Affected files ...

.. //depot/projects/trustedbsd/sedarwin8/darwin/xnu/bsd/kern/kern_audit.c#7 edit
.. //depot/projects/trustedbsd/sedarwin8/darwin/xnu/security/mac_audit.c#5 edit
.. //depot/projects/trustedbsd/sedarwin8/darwin/xnu/security/mac_base.c#19 edit

Differences ...

==== //depot/projects/trustedbsd/sedarwin8/darwin/xnu/bsd/kern/kern_audit.c#7 (text+ko) ====

@@ -133,6 +133,9 @@
 
 static wait_queue_t			audit_wait_queue;
 static zone_t				audit_zone;
+#ifdef MAC
+static zone_t				audit_mac_label_zone;
+#endif
 
 /*
  * Condition variable to signal to the worker that it has work to do:
@@ -224,6 +227,11 @@
  */
 extern task_t kernel_task;
 
+/*
+ * From mac_audit.c.
+ */
+extern zone_t mac_audit_data_zone;
+
 static void
 audit_free(struct kaudit_record *ar)
 {
@@ -252,13 +260,13 @@
 
 #ifdef MAC
 	if (ar->k_ar.ar_vnode1_mac_labels != NULL) {
-		kfree(ar->k_ar.ar_vnode1_mac_labels, MAC_AUDIT_LABEL_LEN);
+		zfree(audit_mac_label_zone, ar->k_ar.ar_vnode1_mac_labels);
 	}
 	if (ar->k_ar.ar_vnode2_mac_labels != NULL) {
-		kfree(ar->k_ar.ar_vnode2_mac_labels, MAC_AUDIT_LABEL_LEN);
+		zfree(audit_mac_label_zone, ar->k_ar.ar_vnode2_mac_labels);
 	}
 	if (ar->k_ar.ar_cred_mac_labels != NULL) {
-		kfree(ar->k_ar.ar_cred_mac_labels, MAC_AUDIT_LABEL_LEN);
+		zfree(audit_mac_label_zone, ar->k_ar.ar_cred_mac_labels);
 	}
 	if (ar->k_ar.ar_arg_mac_string != NULL) {
 		kfree(ar->k_ar.ar_arg_mac_string,
@@ -272,7 +280,7 @@
 		head = LIST_FIRST(ar->k_ar.ar_mac_records);
 		while (head != NULL) {
 			next = LIST_NEXT(head, records);
-			kfree(head->data, head->length);
+			zfree(mac_audit_data_zone, head->data);
 			kfree(head, sizeof(*head));
 			head = next;
 		}
@@ -697,6 +705,15 @@
 			   AQ_HIWATER*sizeof(struct kaudit_record),
 			   8192,
 			   "audit_zone");
+#ifdef MAC
+	/* Assume 3 MAC labels for each audit record: two for vnodes,
+	 * one for creds.
+	 */
+	audit_mac_label_zone = zinit(MAC_AUDIT_LABEL_LEN,
+				     AQ_HIWATER * 3*MAC_AUDIT_LABEL_LEN,
+				     8192,
+				     "audit_mac_label_zone");
+#endif
 
 	/* Initialize the BSM audit subsystem. */
 	kau_init();
@@ -1489,33 +1506,23 @@
 		struct mac mac;
 
 		/*
-		 * We use kalloc instead of kmem_alloc, since we are
-		 * only allocating small records, kmem_alloc would be
-		 * overly wasteful.
-		 *
-		 * XXX: requirement "audit6" states that on failure, this
-		 * code should shut down the machine or fail loudly
-		 * in some other way.
+		 * zalloc() cannot return NULL.
 		 */
 
 		/* Retrieve the MAC labels for the process. */
-		ar->k_ar.ar_cred_mac_labels = (char *)kalloc(MAC_AUDIT_LABEL_LEN);
-		if (ar->k_ar.ar_cred_mac_labels == NULL) {
-			zfree(audit_zone, ar);
-			return (NULL);
-		}
+		ar->k_ar.ar_cred_mac_labels =
+		    (char *)zalloc(audit_mac_label_zone);
 		mac.m_buflen = MAC_AUDIT_LABEL_LEN;
 		mac.m_string = ar->k_ar.ar_cred_mac_labels;
 		mac_cred_label_externalize_audit(p, &mac);
 
+		/*
+		 * kalloc() uses the Mach zone allocator. For the small size
+		 * we are allocating here, the zone allocator will never return
+		 * NULL.
+		 */
 		ar->k_ar.ar_mac_records = (struct mac_audit_record_list_t *)
 		    kalloc(sizeof(*ar->k_ar.ar_mac_records));
-		if (ar->k_ar.ar_mac_records == NULL) {
-			kfree(ar->k_ar.ar_cred_mac_labels,
-			    MAC_AUDIT_LABEL_LEN);
-			zfree(audit_zone, ar);
-			return (NULL);
-		}
 		LIST_INIT(ar->k_ar.ar_mac_records);
 
 		ar->k_ar.ar_forced_by_mac = 0;
@@ -2501,15 +2508,10 @@
 
 #ifdef MAC
 	if (*vnode_mac_labelp == NULL) {
-		*vnode_mac_labelp = (char *)kalloc(MAC_AUDIT_LABEL_LEN);
-		if (*vnode_mac_labelp != NULL) {
-			mac.m_buflen = MAC_AUDIT_LABEL_LEN;
-			mac.m_string = *vnode_mac_labelp;
-			mac_vnode_label_externalize_audit(vp, &mac);
-		} else {
-			/* XXX What to do here? This may be an "audit6" req. */
-			printf("Could not store vnode audit labels");
-		}
+		*vnode_mac_labelp = (char *)zalloc(audit_mac_label_zone);
+		mac.m_buflen = MAC_AUDIT_LABEL_LEN;
+		mac.m_string = *vnode_mac_labelp;
+		mac_vnode_label_externalize_audit(vp, &mac);
 	}
 #endif
 
@@ -2605,11 +2607,12 @@
 		goto out_fail;
 	}
 
+	/*
+	 * kalloc() uses the Mach zone allocator. For the small size
+	 * we are allocating here, the zone allocator will never return
+	 * NULL.
+	 */
 	record = (struct mac_audit_record *)kalloc(sizeof(*record));
-	if (record == NULL) {
-		ret = ENOMEM;
-		goto out_fail;
-	}
 
 	record->type = type;
 	record->length = len;
@@ -2635,8 +2638,12 @@
 	if (ar->k_ar.ar_arg_mac_string == NULL) {
 		ar->k_ar.ar_arg_mac_string =
 		    (char *)kalloc(MAC_MAX_LABEL_BUF_LEN + MAC_ARG_PREFIX_LEN);
+		/* This should be rare event. If kalloc() returns NULL, the
+		 * system is low on memory. To meet the requirement that no
+		 * auditable events be incompletely audited, we panic here.
+		 */
 		if (ar->k_ar.ar_arg_mac_string == NULL)
-			return;
+			panic("Memory allocation failure when auditing MAC system call.");
 	}
 	strncpy(ar->k_ar.ar_arg_mac_string, MAC_ARG_PREFIX, MAC_ARG_PREFIX_LEN);
 	strcpy(ar->k_ar.ar_arg_mac_string + MAC_ARG_PREFIX_LEN, string);

==== //depot/projects/trustedbsd/sedarwin8/darwin/xnu/security/mac_audit.c#5 (text+ko) ====

@@ -46,9 +46,13 @@
 #include <bsd/sys/malloc.h>
 #include <vm/vm_kern.h>
 #include <kern/kalloc.h>
+#include <kern/zalloc.h>
 
 #ifdef AUDIT
 
+/* The zone allocator is initialized in mac_base.c. */
+zone_t mac_audit_data_zone;
+
 int
 mac_system_check_audit(struct ucred *cred, void *record, int length)
 {
@@ -136,9 +140,7 @@
 	if ((len <= 0) || (len > MAC_AUDIT_DATA_LIMIT))
 		return (EINVAL);
 
-	sanitized = kalloc(len);
-	if (sanitized == NULL)
-		return (ENOMEM);
+	sanitized = (char *)zalloc(mac_audit_data_zone);
 
 	bcopy(data, sanitized, len);
 	return (audit_mac_data(MAC_AUDIT_DATA_TYPE, len, sanitized));
@@ -154,7 +156,7 @@
 {
 	char *sanitized;
 	const char *name;
-	int i, allocd, plen, len;
+	int i, size, plen, len;
 
 	name = mac_get_mpc(handle)->mpc_name;
 	len = strlen(text);
@@ -170,16 +172,14 @@
 		if (text[i] < (char) 32 || text[i] > (char) 126)
 			return (EINVAL);
 
-	allocd = len + plen + 1;
- 	sanitized = kalloc(allocd);
-	if (sanitized == NULL)
-		return (ENOMEM);
+	size = len + plen + 1;
+ 	sanitized = (char *)zalloc(mac_audit_data_zone);
 
 	strcpy(sanitized, name);
 	strcat(sanitized, ": ");
 	strcat(sanitized, text);
 
-	return (audit_mac_data(MAC_AUDIT_TEXT_TYPE, allocd, sanitized));
+	return (audit_mac_data(MAC_AUDIT_TEXT_TYPE, size, sanitized));
 }
 
 int

==== //depot/projects/trustedbsd/sedarwin8/darwin/xnu/security/mac_base.c#19 (text+ko) ====

@@ -50,6 +50,7 @@
 #include <sys/vnode_internal.h>
 #include <sys/vfs_context.h>
 #include <sys/namei.h>
+#include <bsd/bsm/audit.h>
 #include <bsd/bsm/audit_kernel.h>
 #include <sys/file.h>
 #include <sys/file_internal.h>
@@ -62,6 +63,8 @@
 #include <mach/vm_types.h>
 #include <mach/vm_prot.h>
 
+#include <kern/zalloc.h>
+
 #include <osfmk/kern/task.h>
 #include <osfmk/kern/kalloc.h>
 #include <libsa/libsa/kext.h>
@@ -152,6 +155,13 @@
     &mac_mmap_revocation_via_cow, 0, "Revoke mmap access to files via "
     "copy-on-write semantics, or by removing all write access");
 
+/*
+ * mac_audit_data_zone is the zone used for data pushed into the audit
+ * record by policies. Using a zone simplifies memory management of this
+ * data, and allows tracking of the amount of data in flight.
+ */
+extern zone_t mac_audit_data_zone;
+
 /* 
  * mac_static_base_mpc holds a pointer to the single instance of the base
  * policy MAC configuration structure. This pointer must be set at boot,
@@ -454,6 +464,10 @@
 	else 
 		mac_static_base_mpc->mpc_ops->mpo_base_notify_finalize();
 
+	mac_audit_data_zone = zinit(MAC_AUDIT_DATA_LIMIT,
+				    AQ_HIWATER * MAC_AUDIT_DATA_LIMIT,
+				    8192, "mac_audit_data_zone");
+
 	sysctl_register_oid(&sysctl__security);
 	sysctl_register_oid(&sysctl__security_mac);
 	sysctl_register_oid(&sysctl__security_mac_max_slots);


More information about the trustedbsd-cvs mailing list