PERFORCE change 91450 for review
Rob Deker
deker at FreeBSD.org
Thu Feb 9 19:22:38 GMT 2006
http://perforce.freebsd.org/chv.cgi?CH=91450
Change 91450 by deker at deker_build1.columbia.sparta.com on 2006/02/09 19:21:49
This combines 2 2 changes. Both comments from millert included:
"Fix the order of the message checks; we need to do the port check
before the rights check (it was the other way around). To do this
we must Move mac_check_port_send() out of ipc_kmsg_send() and into
ipc_kmsg_copyin(). It is too late to deny in ipc_kmsg_send() since
the rights have already have been copied at this point."
"Move the mac_check_port_send() call down into ipc_kmsg_copyin_header().
This reduces the amount of duplicated setup we have to do.
Change an assert into an if() since apparently we can
end up with a NULL ipc_port_t when the system is rebooting."
Submitted by: millert
Affected files ...
.. //depot/projects/trustedbsd/sedarwin7/src/darwin/xnu/osfmk/ipc/ipc_kmsg.c#7 edit
Differences ...
==== //depot/projects/trustedbsd/sedarwin7/src/darwin/xnu/osfmk/ipc/ipc_kmsg.c#7 (text+ko) ====
@@ -1024,6 +1024,9 @@
ipc_object_t dest_port, reply_port;
ipc_port_t dest_soright, reply_soright;
ipc_port_t notify_port;
+#ifdef MAC
+ ipc_entry_t entry;
+#endif
if ((mbits != msg->msgh_bits) ||
(!MACH_MSG_TYPE_PORT_ANY_SEND(dest_type)) ||
@@ -1041,6 +1044,33 @@
if (!MACH_PORT_VALID(dest_name))
goto invalid_dest;
+#ifdef MAC
+ /*
+ * We do the port send check here instead of in ipc_kmsg_send()
+ * because copying the header involves copying the port rights too
+ * and we need to do the send check before anything is actually copied.
+ */
+ entry = ipc_entry_lookup(space, dest_name);
+ if (entry != IE_NULL) {
+ int error = 0;
+ ipc_port_t port = (ipc_port_t) entry->ie_object;
+ // assert(port != IP_NULL);
+ if (port == IP_NULL)
+ goto invalid_dest;
+ ip_lock(port);
+ if (ip_active(port)) {
+ task_t self = current_task();
+ tasklabel_lock(self);
+ error = mac_check_port_send(&self->maclabel,
+ &port->ip_label);
+ tasklabel_unlock(self);
+ }
+ ip_unlock(port);
+ if (error != 0)
+ goto invalid_dest;
+ }
+#endif
+
if (notify != MACH_PORT_NULL) {
ipc_entry_t entry;
@@ -1718,45 +1748,6 @@
mach_port_name_t notify)
{
mach_msg_return_t mr;
-#ifdef MAC
- mach_port_name_t dest_name;
- ipc_entry_t entry;
- ipc_port_t port;
- task_t self;
- int error = 0;
-
- /*
- * We do the port send check here instead of in ipc_kmsg_send()
- * because copying the header involves copying the port rights too
- * and we need to do the send check before anything is actually copied.
- * We don't currently try to mediate kernel-resident servers.
- */
- self = current_task();
- if (self != kernel_task) {
- is_read_lock(space);
- if (space->is_active) {
- dest_name = (mach_port_name_t) kmsg->ikm_header.msgh_remote_port;
- if (MACH_PORT_VALID(dest_name)) {
- entry = ipc_entry_lookup(space, dest_name);
- if (entry != IE_NULL) {
- port = (ipc_port_t) entry->ie_object;
- assert(port != IP_NULL);
- ip_lock(port);
- if (ip_active(port)) {
- tasklabel_lock(self);
- error = mac_check_port_send(&self->maclabel,
- &port->ip_label);
- tasklabel_unlock(self);
- }
- ip_unlock(port);
- }
- }
- }
- is_read_unlock(space);
- if (error != 0)
- return MACH_SEND_INVALID_DEST;
- }
-#endif
mr = ipc_kmsg_copyin_header(&kmsg->ikm_header, space, notify);
if (mr != MACH_MSG_SUCCESS)
@@ -1993,6 +1984,7 @@
ipc_port_request_index_t request;
if (!space->is_active) {
+ printf("ipc_kmsg_copyout_header: dead space\n");
is_write_unlock(space);
return (MACH_RCV_HEADER_ERROR|
MACH_MSG_IPC_SPACE);
@@ -2002,6 +1994,7 @@
notify_port = ipc_port_lookup_notify(space,
notify);
if (notify_port == IP_NULL) {
+ printf("ipc_kmsg_copyout_header: no notify port\n");
is_write_unlock(space);
return MACH_RCV_INVALID_NOTIFY;
}
@@ -2056,13 +2049,16 @@
if (kr != KERN_SUCCESS) {
/* space is unlocked */
- if (kr == KERN_RESOURCE_SHORTAGE)
+ if (kr == KERN_RESOURCE_SHORTAGE) {
+ printf("ipc_kmsg_copyout_header: can't grow kernel ipc space\n");
return (MACH_RCV_HEADER_ERROR|
MACH_MSG_IPC_KERNEL);
- else
+ } else {
+ printf("ipc_kmsg_copyout_header: can't grow user ipc space\n");
return (MACH_RCV_HEADER_ERROR|
MACH_MSG_IPC_SPACE);
}
+ }
/* space is locked again; start over */
continue;
@@ -2099,9 +2095,11 @@
kr = ipc_port_dngrow(reply, ITS_SIZE_NONE);
/* port is unlocked */
- if (kr != KERN_SUCCESS)
+ if (kr != KERN_SUCCESS) {
+ printf("ipc_kmsg_copyout_header: can't grow kernel ipc space2\n");
return (MACH_RCV_HEADER_ERROR|
MACH_MSG_IPC_KERNEL);
+ }
is_write_lock(space);
continue;
@@ -2121,6 +2119,7 @@
kr = ipc_right_copyout(space, reply_name, entry,
reply_type, TRUE, (ipc_object_t) reply);
/* reply port is unlocked */
+ /* XXXMAC - ipc_right_copyout() *can* fail due to MAC */
assert(kr == KERN_SUCCESS);
if (notify_port != IP_NULL)
@@ -2138,6 +2137,7 @@
is_read_lock(space);
if (!space->is_active) {
+ printf("ipc_kmsg_copyout_header: dead space2\n");
is_read_unlock(space);
return MACH_RCV_HEADER_ERROR|MACH_MSG_IPC_SPACE;
}
@@ -2148,11 +2148,13 @@
/* must check notify even though it won't be used */
if ((entry = ipc_entry_lookup(space, notify)) == IE_NULL) {
+ printf("ipc_kmsg_copyout_header: ipc_entry_lookup failed\n");
is_read_unlock(space);
return MACH_RCV_INVALID_NOTIFY;
}
if ((entry->ie_bits & MACH_PORT_TYPE_RECEIVE) == 0) {
+ printf("ipc_kmsg_copyout_header: MACH_PORT_TYPE_RECEIVE not set!\n");
is_read_unlock(space);
return MACH_RCV_INVALID_NOTIFY;
}
@@ -2537,8 +2539,10 @@
mach_msg_return_t mr;
mr = ipc_kmsg_copyout_header(&kmsg->ikm_header, space, notify);
- if (mr != MACH_MSG_SUCCESS)
+ if (mr != MACH_MSG_SUCCESS) {
+ printf("ipc_kmsg_copyout: ipc_kmsg_copyout_header failed: %d\n", mr);
return mr;
+ }
if (kmsg->ikm_header.msgh_bits & MACH_MSGH_BITS_COMPLEX) {
mr = ipc_kmsg_copyout_body(kmsg, space, map, slist);
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