ipc: fix locking issues

* ipc/ipc_port.h (struct ipc_port): Document locking exception.
* ipc/ipc_port.c (ipc_port_destroy): Avoid accessing `port's fields
without the lock.
(ipc_port_alloc_special): Lock `port'.
* ipc/mach_msg.c (mach_msg_trap): Avoid using
`ipc_port_flag_protected_payload' on unlocked port.
* ipc/ipc_kmsg.c (ipc_kmsg_copyout_header): Likewise.

4 files changed, 33 insertions, 12 deletions

diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c
index 5076809..6c4b59a 100644
--- a/ipc/ipc_kmsg.c
+++ b/ipc/ipc_kmsg.c
@@ -1713,6 +1713,7 @@ ipc_kmsg_copyout_header(
 		mach_port_t dest_name;
 		ipc_port_t nsrequest;
 		unsigned long payload;
+		int have_payload;
 
 		/* receiving an asynchronous message */
 
@@ -1732,6 +1733,7 @@ ipc_kmsg_copyout_header(
 		else
 			dest_name = MACH_PORT_NULL;
 		payload = dest->ip_protected_payload;
+		have_payload = ipc_port_flag_protected_payload(dest);
 
 		if ((--dest->ip_srights == 0) &&
 		    ((nsrequest = dest->ip_nsrequest) != IP_NULL)) {
@@ -1745,7 +1747,7 @@ ipc_kmsg_copyout_header(
 		} else
 			ip_unlock(dest);
 
-		if (! ipc_port_flag_protected_payload(dest)) {
+		if (! have_payload) {
 			msg->msgh_bits = (MACH_MSGH_BITS_OTHER(mbits) |
 				MACH_MSGH_BITS(0, MACH_MSG_TYPE_PORT_SEND));
 			msg->msgh_local_port = dest_name;
@@ -1766,6 +1768,7 @@ ipc_kmsg_copyout_header(
 		mach_port_t dest_name, reply_name;
 		ipc_port_t nsrequest;
 		unsigned long payload;
+		int have_payload;
 
 		/* receiving a request message */
 
@@ -1837,6 +1840,7 @@ ipc_kmsg_copyout_header(
 		else
 			dest_name = MACH_PORT_NULL;
 		payload = dest->ip_protected_payload;
+		have_payload = ipc_port_flag_protected_payload(dest);
 
 		if ((--dest->ip_srights == 0) &&
 		    ((nsrequest = dest->ip_nsrequest) != IP_NULL)) {
@@ -1850,7 +1854,7 @@ ipc_kmsg_copyout_header(
 		} else
 			ip_unlock(dest);
 
-		if (! ipc_port_flag_protected_payload(dest)) {
+		if (! have_payload) {
 			msg->msgh_bits = (MACH_MSGH_BITS_OTHER(mbits) |
 				MACH_MSGH_BITS(MACH_MSG_TYPE_PORT_SEND_ONCE,
 					       MACH_MSG_TYPE_PORT_SEND));
@@ -1868,6 +1872,7 @@ ipc_kmsg_copyout_header(
 	    case MACH_MSGH_BITS(MACH_MSG_TYPE_PORT_SEND_ONCE, 0): {
 		mach_port_t dest_name;
 		unsigned long payload;
+		int have_payload;
 
 		/* receiving a reply message */
 
@@ -1882,6 +1887,7 @@ ipc_kmsg_copyout_header(
 		assert(dest->ip_sorights > 0);
 
 		payload = dest->ip_protected_payload;
+		have_payload = ipc_port_flag_protected_payload(dest);
 
 		if (dest->ip_receiver == space) {
 			ip_release(dest);
@@ -1895,7 +1901,7 @@ ipc_kmsg_copyout_header(
 			dest_name = MACH_PORT_NULL;
 		}
 
-		if (! ipc_port_flag_protected_payload(dest)) {
+		if (! have_payload) {
 			msg->msgh_bits = (MACH_MSGH_BITS_OTHER(mbits) |
 				MACH_MSGH_BITS(0,
 					MACH_MSG_TYPE_PORT_SEND_ONCE));
@@ -1922,6 +1928,7 @@ ipc_kmsg_copyout_header(
 	ipc_port_t reply = (ipc_port_t) msg->msgh_local_port;
 	mach_port_t dest_name, reply_name;
 	unsigned long payload;
+	int have_payload;
 
 	if (IP_VALID(reply)) {
 		ipc_port_t notify_port;
@@ -2161,6 +2168,7 @@ ipc_kmsg_copyout_header(
 
     copyout_dest:
 	payload = dest->ip_protected_payload;
+	have_payload = ipc_port_flag_protected_payload(dest);
 
 	if (ip_active(dest)) {
 		ipc_object_copyout_dest(space, (ipc_object_t) dest,
@@ -2189,7 +2197,7 @@ ipc_kmsg_copyout_header(
 	if (IP_VALID(reply))
 		ipc_port_release(reply);
 
-	if (! ipc_port_flag_protected_payload(dest)) {
+	if (! have_payload) {
 		msg->msgh_bits = (MACH_MSGH_BITS_OTHER(mbits) |
 				  MACH_MSGH_BITS(reply_type, dest_type));
 		msg->msgh_local_port = dest_name;
diff --git a/ipc/ipc_port.c b/ipc/ipc_port.c
index 86a4ee2..4e4ad59 100644
--- a/ipc/ipc_port.c
+++ b/ipc/ipc_port.c
@@ -694,11 +694,13 @@ ipc_port_destroy(
 
 	port->ip_object.io_bits &= ~IO_BITS_ACTIVE;
 	port->ip_timestamp = ipc_port_timestamp();
+
+	nsrequest = port->ip_nsrequest;
+	dnrequests = port->ip_dnrequests;
+
 	ip_unlock(port);
 
 	/* throw away no-senders request */
-
-	nsrequest = port->ip_nsrequest;
 	if (nsrequest != IP_NULL)
 		ipc_notify_send_once(nsrequest); /* consumes ref */
 
@@ -725,8 +727,6 @@ ipc_port_destroy(
 	imq_unlock(mqueue);
 
 	/* generate dead-name notifications */
-
-	dnrequests = port->ip_dnrequests;
 	if (dnrequests != IPR_NULL) {
 		ipc_table_size_t its = dnrequests->ipr_size;
 		ipc_table_elems_t size = its->its_size;
@@ -1183,6 +1183,7 @@ ipc_port_alloc_special(ipc_space_t space)
 		return IP_NULL;
 
 	ip_lock_init(port);
+	ip_lock(port);
 	port->ip_references = 1;
 	port->ip_object.io_bits = io_makebits(TRUE, IOT_PORT, 0);
 
@@ -1198,7 +1199,7 @@ ipc_port_alloc_special(ipc_space_t space)
 	 */
 
 	ipc_port_init(port, space, (mach_port_t)port);
-
+	ip_unlock(port);
 	return port;
 }
 
diff --git a/ipc/ipc_port.h b/ipc/ipc_port.h
index ade6967..de43b42 100644
--- a/ipc/ipc_port.h
+++ b/ipc/ipc_port.h
@@ -82,6 +82,8 @@ struct ipc_port {
 		ipc_port_timestamp_t timestamp;
 	} data;
 
+	/* Note: For pagers (IKOT_PAGER), this field is protected by
+	   vm_object_cache_lock.  */
 	ipc_kobject_t ip_kobject;
 
 	mach_port_mscount_t ip_mscount;
diff --git a/ipc/mach_msg.c b/ipc/mach_msg.c
index fe0c43e..371d725 100644
--- a/ipc/mach_msg.c
+++ b/ipc/mach_msg.c
@@ -954,6 +954,7 @@ mach_msg_trap(
 				(ipc_port_t) kmsg->ikm_header.msgh_local_port;
 			mach_port_t dest_name, reply_name;
 			unsigned long payload;
+			int have_payload;
 
 			/* receiving a request message */
 
@@ -1018,6 +1019,8 @@ mach_msg_trap(
 			else
 				dest_name = MACH_PORT_NULL;
 			payload = dest_port->ip_protected_payload;
+			have_payload =
+				ipc_port_flag_protected_payload(dest_port);
 
 			if ((--dest_port->ip_srights == 0) &&
 			    (dest_port->ip_nsrequest != IP_NULL)) {
@@ -1035,7 +1038,7 @@ mach_msg_trap(
 			} else
 				ip_unlock(dest_port);
 
-			if (! ipc_port_flag_protected_payload(dest_port)) {
+			if (! have_payload) {
 				kmsg->ikm_header.msgh_bits = MACH_MSGH_BITS(
 					MACH_MSG_TYPE_PORT_SEND_ONCE,
 					MACH_MSG_TYPE_PORT_SEND);
@@ -1059,6 +1062,7 @@ mach_msg_trap(
 		    case MACH_MSGH_BITS(MACH_MSG_TYPE_PORT_SEND_ONCE, 0): {
 			mach_port_t dest_name;
 			unsigned long payload;
+			int have_payload;
 
 			/* receiving a reply message */
 
@@ -1071,6 +1075,8 @@ mach_msg_trap(
 			assert(dest_port->ip_sorights > 0);
 
 			payload = dest_port->ip_protected_payload;
+			have_payload =
+				ipc_port_flag_protected_payload(dest_port);
 
 			if (dest_port->ip_receiver == space) {
 				ip_release(dest_port);
@@ -1084,7 +1090,7 @@ mach_msg_trap(
 				dest_name = MACH_PORT_NULL;
 			}
 
-			if (! ipc_port_flag_protected_payload(dest_port)) {
+			if (! have_payload) {
 				kmsg->ikm_header.msgh_bits = MACH_MSGH_BITS(
 					0,
 					MACH_MSG_TYPE_PORT_SEND_ONCE);
@@ -1104,6 +1110,7 @@ mach_msg_trap(
 			 MACH_MSGH_BITS(MACH_MSG_TYPE_PORT_SEND_ONCE, 0): {
 			mach_port_t dest_name;
 			unsigned long payload;
+			int have_payload;
 
 			/* receiving a complex reply message */
 
@@ -1116,6 +1123,9 @@ mach_msg_trap(
 			assert(dest_port->ip_sorights > 0);
 
 			payload = dest_port->ip_protected_payload;
+			have_payload =
+				ipc_port_flag_protected_payload(dest_port);
+
 
 			if (dest_port->ip_receiver == space) {
 				ip_release(dest_port);
@@ -1129,7 +1139,7 @@ mach_msg_trap(
 				dest_name = MACH_PORT_NULL;
 			}
 
-			if (! ipc_port_flag_protected_payload(dest_port)) {
+			if (! have_payload) {
 				kmsg->ikm_header.msgh_bits =
 					MACH_MSGH_BITS_COMPLEX
 					| MACH_MSGH_BITS(