ipc: fix the locking of the IPC entry allocation functions

* ipc/ipc_entry.c (ipc_entry_alloc): Assume the space is write-locked.
(ipc_entry_alloc_name): Likewise.
* ipc/ipc_object.c: Fix the locking around all call sites to the two
functions where the space was not locked before.

2 files changed, 22 insertions, 31 deletions

diff --git a/ipc/ipc_entry.c b/ipc/ipc_entry.c
index a5fe319..0414ba5 100644
--- a/ipc/ipc_entry.c
+++ b/ipc/ipc_entry.c
@@ -56,8 +56,7 @@ struct kmem_cache ipc_entry_cache;
  *	Purpose:
  *		Allocate an entry out of the space.
  *	Conditions:
- *		The space is not locked before, but it is write-locked after
- *		if the call is successful.  May allocate memory.
+ *		The space must be write-locked.  May allocate memory.
  *	Returns:
  *		KERN_SUCCESS		An entry was allocated.
  *		KERN_INVALID_TASK	The space is dead.
@@ -75,27 +74,21 @@ ipc_entry_alloc(
 	ipc_entry_t entry;
 	rdxtree_key_t key;
 
-	is_write_lock(space);
-
 	if (!space->is_active) {
-		is_write_unlock(space);
 		return KERN_INVALID_TASK;
 	}
 
 	kr = ipc_entry_get(space, namep, entryp);
 	if (kr == KERN_SUCCESS)
-		/* Success.  Space is write-locked.  */
 		return kr;
 
 	entry = ie_alloc();
 	if (entry == IE_NULL) {
-		is_write_unlock(space);
 		return KERN_RESOURCE_SHORTAGE;
 	}
 
 	kr = rdxtree_insert_alloc(&space->is_map, entry, &key);
 	if (kr) {
-		is_write_unlock(space);
 		ie_free(entry);
 		return kr;
 	}
@@ -108,7 +101,6 @@ ipc_entry_alloc(
 
 	*entryp = entry;
 	*namep = (mach_port_t) key;
-	/* Success.  Space is write-locked.  */
 	return KERN_SUCCESS;
 }
 
@@ -118,8 +110,7 @@ ipc_entry_alloc(
  *		Allocates/finds an entry with a specific name.
  *		If an existing entry is returned, its type will be nonzero.
  *	Conditions:
- *		The space is not locked before, but it is write-locked after
- *		if the call is successful.  May allocate memory.
+ *		The space must be write-locked.  May allocate memory.
  *	Returns:
  *		KERN_SUCCESS		Found existing entry with same name.
  *		KERN_SUCCESS		Allocated a new entry.
@@ -138,10 +129,7 @@ ipc_entry_alloc_name(
 	void **slot;
 	assert(MACH_PORT_VALID(name));
 
-	is_write_lock(space);
-
 	if (!space->is_active) {
-		is_write_unlock(space);
 		return KERN_INVALID_TASK;
 	}
 
@@ -152,7 +140,6 @@ ipc_entry_alloc_name(
 	if (slot == NULL || entry == IE_NULL) {
 		entry = ie_alloc();
 		if (entry == IE_NULL) {
-			is_write_unlock(space);
 			return KERN_RESOURCE_SHORTAGE;
 		}
 
@@ -167,7 +154,6 @@ ipc_entry_alloc_name(
 			kr = rdxtree_insert(&space->is_map,
 					    (rdxtree_key_t) name, entry);
 			if (kr != KERN_SUCCESS) {
-				is_write_unlock(space);
 				ie_free(entry);
 				return kr;
 			}
@@ -175,14 +161,12 @@ ipc_entry_alloc_name(
 		space->is_size += 1;
 
 		*entryp = entry;
-		/* Success.  Space is write-locked.  */
 		return KERN_SUCCESS;
 	}
 
 	if (IE_BITS_TYPE(entry->ie_bits)) {
 		/* Used entry.  */
 		*entryp = entry;
-		/* Success.  Space is write-locked.  */
 		return KERN_SUCCESS;
 	}
 
@@ -202,7 +186,6 @@ ipc_entry_alloc_name(
 
 	space->is_size += 1;
 	*entryp = entry;
-	/* Success.  Space is write-locked.  */
 	return KERN_SUCCESS;
 }
 
diff --git a/ipc/ipc_object.c b/ipc/ipc_object.c
index 2d84cf5..320fbcb 100644
--- a/ipc/ipc_object.c
+++ b/ipc/ipc_object.c
@@ -155,11 +155,12 @@ ipc_object_alloc_dead(
 	ipc_entry_t entry;
 	kern_return_t kr;
 
-
+	is_write_lock(space);
 	kr = ipc_entry_alloc(space, namep, &entry);
-	if (kr != KERN_SUCCESS)
+	if (kr != KERN_SUCCESS) {
+		is_write_unlock(space);
 		return kr;
-	/* space is write-locked */
+	}
 
 	/* null object, MACH_PORT_TYPE_DEAD_NAME, 1 uref */
 
@@ -191,11 +192,12 @@ ipc_object_alloc_dead_name(
 	ipc_entry_t entry;
 	kern_return_t kr;
 
-
+	is_write_lock(space);
 	kr = ipc_entry_alloc_name(space, name, &entry);
-	if (kr != KERN_SUCCESS)
+	if (kr != KERN_SUCCESS) {
+		is_write_unlock(space);
 		return kr;
-	/* space is write-locked */
+	}
 
 	if (ipc_right_inuse(space, name, entry))
 		return KERN_NAME_EXISTS;
@@ -254,12 +256,13 @@ ipc_object_alloc(
 
 		memset(pset, 0, sizeof(*pset));
 	}
+	is_write_lock(space);
 	kr = ipc_entry_alloc(space, namep, &entry);
 	if (kr != KERN_SUCCESS) {
+		is_write_unlock(space);
 		io_free(otype, object);
 		return kr;
 	}
-	/* space is write-locked */
 
 	entry->ie_bits |= type | urefs;
 	entry->ie_object = object;
@@ -321,12 +324,13 @@ ipc_object_alloc_name(
 		memset(pset, 0, sizeof(*pset));
 	}
 
+	is_write_lock(space);
 	kr = ipc_entry_alloc_name(space, name, &entry);
 	if (kr != KERN_SUCCESS) {
+		is_write_unlock(space);
 		io_free(otype, object);
 		return kr;
 	}
-	/* space is write-locked */
 
 	if (ipc_right_inuse(space, name, entry)) {
 		io_free(otype, object);
@@ -753,10 +757,12 @@ ipc_object_copyout_name(
 	assert(IO_VALID(object));
 	assert(io_otype(object) == IOT_PORT);
 
+	is_write_lock(space);
 	kr = ipc_entry_alloc_name(space, name, &entry);
-	if (kr != KERN_SUCCESS)
+	if (kr != KERN_SUCCESS) {
+		is_write_unlock(space);
 		return kr;
-	/* space is write-locked and active */
+	}
 
 	if ((msgt_name != MACH_MSG_TYPE_PORT_SEND_ONCE) &&
 	    ipc_right_reverse(space, object, &oname, &oentry)) {
@@ -930,10 +936,12 @@ ipc_object_rename(
 	ipc_entry_t oentry, nentry;
 	kern_return_t kr;
 
+	is_write_lock(space);
 	kr = ipc_entry_alloc_name(space, nname, &nentry);
-	if (kr != KERN_SUCCESS)
+	if (kr != KERN_SUCCESS) {
+		is_write_unlock(space);
 		return kr;
-	/* space is write-locked and active */
+	}
 
 	if (ipc_right_inuse(space, nname, nentry)) {
 		/* space is unlocked */