PERFORCE change 92921 for review

[Todd Miller]

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

Change 92921 by millert at millert_ibook on 2006/03/07 16:46:29

	Change how label handles are freed when their reference
	count reaches zero.  We no longer free label handle storage
	when the ref count == 0.  For kernel-allocated label handles,
	just deallocate the associated port.  The actual label
	handle storage is deallocated by labelh_destroy() which is
	called by ipc_kobject_destroy() after all port references
	are gone.  This fixes a race between a user program requesting
	(and accessing) the label of a labeled kernel object and
	the destruction of that object (task, port, etc).

Affected files ...

.. //depot/projects/trustedbsd/sedarwin7/src/darwin/xnu/osfmk/ipc/ipc_labelh.c#8 edit
.. //depot/projects/trustedbsd/sedarwin7/src/darwin/xnu/osfmk/ipc/ipc_labelh.h#9 edit
.. //depot/projects/trustedbsd/sedarwin7/src/darwin/xnu/osfmk/kern/ipc_kobject.c#4 edit

Differences ...

==== //depot/projects/trustedbsd/sedarwin7/src/darwin/xnu/osfmk/ipc/ipc_labelh.c#8 (text+ko) ====

@@ -47,7 +47,7 @@
 	if (space == IS_NULL || space->is_task == NULL)
 		return (KERN_INVALID_TASK);
 
-	/* XXX - perform entrypoint check here */
+	/* XXX - perform entrypoint check here? */
 
 	/*
 	 * Note: the calling task will have a receive right for the port.
@@ -75,7 +75,7 @@
 	lh->lh_port = port;
 	lh->lh_label = *inl;
 	lh->lh_type = LABELH_TYPE_USER;
-	lh->lh_references = 1;
+	lh->lh_references = 1;		/* unused for LABELH_TYPE_USER */
 
 	/* Must call ipc_kobject_set() with port unlocked. */
 	ip_unlock(lh->lh_port);
@@ -154,6 +154,11 @@
 {
 	ipc_labelh_t lh;
 
+	/*
+	 * A label handle may only have a single reference. 
+	 * If there are no other references this is a no-op.
+	 * Otherwise, make a copy we can write to and return it.
+	 */
 	if (old->lh_references == 1)
 		return (old);
 	lh = labelh_duplicate(old);
@@ -175,6 +180,9 @@
 	return (lh);
 }
 
+/*
+ * Release a reference on an (unlocked) label handle.
+ */
 void
 labelh_release(ipc_labelh_t lh)
 {
@@ -183,10 +191,18 @@
 	lh_check_unlock(lh);
 }
 
+/*
+ * Deallocate space associated with the label handle backed by the
+ * specified port.  For kernel-allocated label handles the
+ * label handle reference count should be 0.  For user-allocated
+ * handles the ref count is not used (it was initialized to 1).
+ */
 void
-lh_free(ipc_labelh_t lh)
+labelh_destroy(ipc_port_t port)
 {
-	ipc_object_release(&lh->lh_port->ip_object);
+	ipc_labelh_t lh = (ipc_labelh_t) port->ip_kobject;
+
+	ip_release(lh->lh_port);
 	mac_destroy_port_label(&lh->lh_label);
 	zfree(ipc_labelh_zone, (vm_offset_t)lh);
 }

==== //depot/projects/trustedbsd/sedarwin7/src/darwin/xnu/osfmk/ipc/ipc_labelh.h#9 (text+ko) ====

@@ -47,19 +47,19 @@
 #define	LABELH_TYPE_KERN	0
 #define	LABELH_TYPE_USER	1
 
+void labelh_destroy(ipc_port_t port);
 ipc_labelh_t labelh_duplicate(ipc_labelh_t old);
 ipc_labelh_t labelh_modify(ipc_labelh_t old);
 ipc_labelh_t labelh_new(void);
 kern_return_t labelh_new_user(ipc_space_t, struct label *, mach_port_name_t *);
 void labelh_release(ipc_labelh_t lh);
 ipc_labelh_t labelh_reference(ipc_labelh_t lh);
-void lh_free(ipc_labelh_t lh);
 
 #define lh_reference(lh)	((lh)->lh_references++)
-#define lh_release(lh)					\
-MACRO_BEGIN						\
-  assert((lh)->lh_references > 0);			\
-	(lh)->lh_references--;				\
+#define lh_release(lh)						\
+MACRO_BEGIN							\
+	assert((lh)->lh_references > 0);			\
+	(lh)->lh_references--;					\
 MACRO_END
 
 extern zone_t ipc_labelh_zone;
@@ -67,13 +67,21 @@
 #define lh_lock io_lock
 #define lh_unlock io_unlock
 
-#define lh_check_unlock(lh) 		       			\
-MACRO_BEGIN				       			\
+/*
+ * Check the number of references the label handle a left.
+ * If there are 0 references and this is a kernel-allocated
+ * label handle, deallocate the associated port.  The
+ * storage space for the label handle will be deallocated
+ * as part of the port destruction.  User-allocated label
+ * handles are destroyed along with their ports.
+ */
+#define lh_check_unlock(lh)					\
+MACRO_BEGIN							\
 	_VOLATILE_ natural_t _refs = (lh)->lh_references;	\
-					       			\
-	lh_unlock(lh);			       			\
-	if (_refs == 0)			       			\
-		lh_free(lh);		      			\
+								\
+	lh_unlock(lh);						\
+	if (_refs == 0 && (lh)->lh_type == LABELH_TYPE_KERN)	\
+		ipc_port_dealloc_kernel((lh)->lh_port);		\
 MACRO_END
 
 #endif

==== //depot/projects/trustedbsd/sedarwin7/src/darwin/xnu/osfmk/kern/ipc_kobject.c#4 (text+ko) ====

@@ -514,7 +514,7 @@
 		break;
 
 	case IKOT_LABELH:
-		labelh_release(port->ip_kobject);
+		labelh_destroy(port);
 		break;
 
 	default: