syscall: msg: validate iovec array itself as well as the buffers it points to

when locking a pair of objects, the object with the lesser memory address
is always locked first. the pair is unlocked in the opposite order.

include/kernel/channel.h

@@ -4,7 +4,7 @@
 #include <kernel/object.h>
 #include <kernel/sched.h>
 
-struct kmsg;
+struct msg;
 
 struct channel {
 	struct object c_base;
@@ -21,17 +21,19 @@ extern struct channel *channel_create(void);
 
 extern kern_status_t channel_enqueue_msg(
 	struct channel *channel,
-	struct kmsg *msg);
+	struct msg *msg);
 
 extern kern_status_t channel_recv_msg(
 	struct channel *channel,
-	struct msg *out_msg,
 	msgid_t *out_id,
+	struct iovec *out_data,
+	size_t out_data_count,
 	unsigned long *irq_flags);
 extern kern_status_t channel_reply_msg(
 	struct channel *channel,
 	msgid_t id,
-	const struct msg *resp,
+	const struct iovec *resp_data,
+	size_t resp_data_count,
 	unsigned long *irq_flags);
 
 extern kern_status_t channel_read_msg(

include/kernel/handle.h

@@ -54,12 +54,4 @@ extern struct handle *handle_table_get_handle(
 	struct handle_table *tab,
 	kern_handle_t handle);
 
-extern kern_status_t handle_list_transfer(
-	struct handle_table *dest,
-	struct handle_list *dest_list,
-	size_t dest_list_count,
-	struct handle_table *src,
-	const struct handle_list *src_list,
-	size_t src_list_count);
-
 #endif

include/kernel/msg.h

@@ -15,7 +15,7 @@ enum kmsg_status {
 	KMSG_REPLY_SENT,
 };
 
-struct kmsg {
+struct msg {
 	spin_lock_t msg_lock;
 	enum kmsg_status msg_status;
 	struct btree_node msg_node;
@@ -23,8 +23,10 @@ struct kmsg {
 	kern_status_t msg_result;
 	struct port *msg_sender_port;
 	struct thread *msg_sender_thread;
-	struct msg msg_req;
+	const struct iovec *msg_req_data;
-	struct msg msg_resp;
+	size_t msg_req_data_count;
+	struct iovec *msg_resp_data;
+	size_t msg_resp_data_count;
 };
 
 #endif

include/kernel/object.h

@@ -31,6 +31,20 @@ extern "C" {
 		unsigned long flags)                                           \
 	{                                                                      \
 		object_unlock_irqrestore(&p->base, flags);                     \
+	}                                                                      \
+	static inline void object_name##_lock_pair_irqsave(                    \
+		struct object_name *a,                                         \
+		struct object_name *b,                                         \
+		unsigned long *flags)                                          \
+	{                                                                      \
+		object_lock_pair_irqsave(&a->base, &b->base, flags);           \
+	}                                                                      \
+	static inline void object_name##_unlock_pair_irqrestore(               \
+		struct object_name *a,                                         \
+		struct object_name *b,                                         \
+		unsigned long flags)                                           \
+	{                                                                      \
+		object_unlock_pair_irqrestore(&a->base, &b->base, flags);      \
 	}
 
 #define OBJECT_MAGIC    0xBADDCAFE
@@ -92,6 +106,15 @@ extern void object_unlock(struct object *obj);
 extern void object_lock_irqsave(struct object *obj, unsigned long *flags);
 extern void object_unlock_irqrestore(struct object *obj, unsigned long flags);
 
+extern void object_lock_pair_irqsave(
+	struct object *a,
+	struct object *b,
+	unsigned long *flags);
+extern void object_unlock_pair_irqrestore(
+	struct object *a,
+	struct object *b,
+	unsigned long flags);
+
 #ifdef __cplusplus
 }
 #endif

include/kernel/port.h

@@ -32,8 +32,10 @@ extern kern_status_t port_connect(struct port *port, struct channel *remote);
 extern kern_status_t port_disconnect(struct port *port);
 extern kern_status_t port_send_msg(
 	struct port *port,
-	const struct msg *req,
+	const struct iovec *req_data,
-	struct msg *resp,
+	size_t req_data_count,
+	struct iovec *resp_data,
+	size_t resp_data_count,
 	unsigned long *lock_flags);
 
 DEFINE_OBJECT_LOCK_FUNCTION(port, p_base)

include/kernel/sched.h

@@ -114,7 +114,7 @@ struct thread {
 	virt_addr_t tr_cpu_user_sp, tr_cpu_kernel_sp;
 
 	struct runqueue *tr_rq;
-	struct kmsg tr_msg;
+	struct msg tr_msg;
 
 	struct queue_entry tr_parent_entry;
 	struct queue_entry tr_rqentry;

include/kernel/syscall.h

@@ -2,16 +2,14 @@
 #define KERNEL_SYSCALL_H_
 
 #include <kernel/handle.h>
+#include <kernel/sched.h>
+#include <kernel/vm-region.h>
 #include <kernel/vm.h>
 #include <mango/status.h>
 #include <mango/syscall.h>
 
 #define validate_access(task, ptr, len, flags)                                 \
-	vm_region_validate_access(                                             \
+	__validate_access(task, (const void *)ptr, len, flags)
-		task->t_address_space,                                         \
-		(virt_addr_t)ptr,                                              \
-		len,                                                           \
-		flags | VM_PROT_USER)
 #define validate_access_r(task, ptr, len)                                      \
 	validate_access(task, ptr, len, VM_PROT_READ | VM_PROT_USER)
 #define validate_access_w(task, ptr, len)                                      \
@@ -23,6 +21,23 @@
 		len,                                                           \
 		VM_PROT_READ | VM_PROT_WRITE | VM_PROT_USER)
 
+static inline bool __validate_access(
+	struct task *task,
+	const void *ptr,
+	size_t len,
+	vm_prot_t flags)
+{
+	unsigned long irq_flags;
+	vm_region_lock_irqsave(task->t_address_space, &irq_flags);
+	bool result = vm_region_validate_access(
+		task->t_address_space,
+		(virt_addr_t)ptr,
+		len,
+		flags | VM_PROT_USER);
+	vm_region_unlock_irqrestore(task->t_address_space, irq_flags);
+	return result;
+}
+
 extern kern_status_t sys_task_exit(int status);
 extern kern_status_t sys_task_self(kern_handle_t *out);
 extern kern_status_t sys_task_create(
@@ -128,10 +143,7 @@ extern kern_status_t sys_kern_config_set(
 	const void *ptr,
 	size_t len);
 
-extern kern_status_t sys_channel_create(
+extern kern_status_t sys_channel_create(unsigned int id, kern_handle_t *out);
-	unsigned int id,
-	channel_flags_t flags,
-	kern_handle_t *out);
 extern kern_status_t sys_port_create(kern_handle_t *out);
 extern kern_status_t sys_port_connect(
 	kern_handle_t port,
@@ -141,22 +153,20 @@ extern kern_status_t sys_port_disconnect(kern_handle_t port);
 
 extern kern_status_t sys_msg_send(
 	kern_handle_t port,
-	msg_flags_t flags,
+	const struct iovec *req_data,
-	const struct msg *req,
+	size_t req_data_count,
-	struct msg *resp);
+	struct iovec *resp_data,
-
+	size_t resp_data_count);
 extern kern_status_t sys_msg_recv(
 	kern_handle_t channel,
-	msg_flags_t flags,
 	msgid_t *out_id,
-	struct msg *out_msg);
+	struct iovec *out_data,
-
+	size_t out_data_count);
 extern kern_status_t sys_msg_reply(
 	kern_handle_t channel,
-	msg_flags_t flags,
 	msgid_t id,
-	const struct msg *reply);
+	const struct iovec *reply_data,
-
+	size_t reply_data_count);
 extern kern_status_t sys_msg_read(
 	kern_handle_t channel_handle,
 	msgid_t id,
@@ -164,25 +174,13 @@ extern kern_status_t sys_msg_read(
 	const struct iovec *iov,
 	size_t iov_count,
 	size_t *nr_read);
-extern kern_status_t sys_msg_read_handles(
-	kern_handle_t channel,
-	msgid_t id,
-	size_t offset,
-	struct handle_list *out,
-	size_t nr_out);
-
 extern kern_status_t sys_msg_write(
 	kern_handle_t channel,
 	msgid_t id,
 	size_t offset,
 	const struct iovec *in,
-	size_t nr_in);
+	size_t nr_in,
-extern kern_status_t sys_msg_write_handles(
+	size_t *nr_written);
-	kern_handle_t channel,
-	msgid_t id,
-	size_t offset,
-	const struct handle_list *in,
-	size_t nr_in);
 
 extern virt_addr_t syscall_get_function(unsigned int sysid);
 

kernel/channel.c

@@ -1,5 +1,6 @@
 #include <kernel/channel.h>
 #include <kernel/msg.h>
+#include <kernel/port.h>
 #include <kernel/util.h>
 #include <kernel/vm-region.h>
 
@@ -11,7 +12,7 @@ static struct object_type channel_type = {
 	.ob_header_offset = offsetof(struct channel, c_base),
 };
 
-BTREE_DEFINE_SIMPLE_GET(struct kmsg, msgid_t, msg_node, msg_id, get_msg_with_id)
+BTREE_DEFINE_SIMPLE_GET(struct msg, msgid_t, msg_node, msg_id, get_msg_with_id)
 
 kern_status_t channel_type_init(void)
 {
@@ -35,7 +36,7 @@ extern struct channel *channel_create(void)
 	return channel;
 }
 
-static bool try_enqueue(struct btree *tree, struct kmsg *msg)
+static bool try_enqueue(struct btree *tree, struct msg *msg)
 {
 	if (!tree->b_root) {
 		tree->b_root = &msg->msg_node;
@@ -45,8 +46,8 @@ static bool try_enqueue(struct btree *tree, struct kmsg *msg)
 
 	struct btree_node *cur = tree->b_root;
 	while (1) {
-		struct kmsg *cur_node
+		struct msg *cur_node
-			= BTREE_CONTAINER(struct kmsg, msg_node, cur);
+			= BTREE_CONTAINER(struct msg, msg_node, cur);
 		struct btree_node *next = NULL;
 
 		if (msg->msg_id > cur_node->msg_id) {
@@ -75,26 +76,28 @@ static bool try_enqueue(struct btree *tree, struct kmsg *msg)
 }
 
 static void kmsg_reply_error(
-	struct kmsg *msg,
+	struct msg *msg,
 	kern_status_t status,
 	unsigned long *lock_flags)
 {
 	msg->msg_status = KMSG_REPLY_SENT;
+	msg->msg_sender_port->p_status = PORT_READY;
 	msg->msg_result = status;
 	thread_awaken(msg->msg_sender_thread);
 	spin_unlock_irqrestore(&msg->msg_lock, *lock_flags);
 }
 
-static struct kmsg *get_next_msg(
+static struct msg *get_next_msg(
 	struct channel *channel,
 	unsigned long *lock_flags)
 {
 	struct btree_node *cur = btree_first(&channel->c_msg);
 	while (cur) {
-		struct kmsg *msg = BTREE_CONTAINER(struct kmsg, msg_node, cur);
+		struct msg *msg = BTREE_CONTAINER(struct msg, msg_node, cur);
 		spin_lock_irqsave(&msg->msg_lock, lock_flags);
 		if (msg->msg_status == KMSG_WAIT_RECEIVE) {
 			msg->msg_status = KMSG_WAIT_REPLY;
+			msg->msg_sender_port->p_status = PORT_REPLY_BLOCKED;
 			return msg;
 		}
 
@@ -107,7 +110,7 @@ static struct kmsg *get_next_msg(
 
 extern kern_status_t channel_enqueue_msg(
 	struct channel *channel,
-	struct kmsg *msg)
+	struct msg *msg)
 {
 	fill_random(&msg->msg_id, sizeof msg->msg_id);
 	while (!try_enqueue(&channel->c_msg, msg)) {
@@ -121,13 +124,14 @@ extern kern_status_t channel_enqueue_msg(
 
 extern kern_status_t channel_recv_msg(
 	struct channel *channel,
-	struct msg *out_msg,
 	msgid_t *out_id,
+	struct iovec *out_data,
+	size_t out_data_count,
 	unsigned long *irq_flags)
 {
 	struct wait_item waiter;
 	struct thread *self = current_thread();
-	struct kmsg *msg = NULL;
+	struct msg *msg = NULL;
 	unsigned long msg_lock_flags;
 
 	wait_item_init(&waiter, self);
@@ -149,29 +153,24 @@ extern kern_status_t channel_recv_msg(
 	struct task *sender = msg->msg_sender_thread->tr_parent;
 	struct task *receiver = self->tr_parent;
 
+	struct vm_region *src = sender->t_address_space,
+			 *dst = receiver->t_address_space;
+	unsigned long f;
+	vm_region_lock_pair_irqsave(src, dst, &f);
+
 	kern_status_t status = vm_region_memmove_v(
-		receiver->t_address_space,
+		dst,
 		0,
-		out_msg->msg_data,
+		out_data,
-		out_msg->msg_data_count,
+		out_data_count,
-		sender->t_address_space,
+		src,
 		0,
-		msg->msg_req.msg_data,
+		msg->msg_req_data,
-		msg->msg_req.msg_data_count,
+		msg->msg_req_data_count,
 		VM_REGION_COPY_ALL,
 		NULL);
-	if (status != KERN_OK) {
+	vm_region_unlock_pair_irqrestore(src, dst, f);
-		kmsg_reply_error(msg, status, &msg_lock_flags);
-		return status;
-	}
 
-	status = handle_list_transfer(
-		receiver->t_handles,
-		out_msg->msg_handles,
-		out_msg->msg_handles_count,
-		sender->t_handles,
-		msg->msg_req.msg_handles,
-		msg->msg_req.msg_handles_count);
 	if (status != KERN_OK) {
 		kmsg_reply_error(msg, status, &msg_lock_flags);
 		return status;
@@ -187,11 +186,12 @@ extern kern_status_t channel_recv_msg(
 extern kern_status_t channel_reply_msg(
 	struct channel *channel,
 	msgid_t id,
-	const struct msg *resp,
+	const struct iovec *resp_data,
+	size_t resp_data_count,
 	unsigned long *irq_flags)
 {
 	unsigned long msg_lock_flags;
-	struct kmsg *msg = get_msg_with_id(&channel->c_msg, id);
+	struct msg *msg = get_msg_with_id(&channel->c_msg, id);
 	if (!msg) {
 		return KERN_INVALID_ARGUMENT;
 	}
@@ -208,29 +208,24 @@ extern kern_status_t channel_reply_msg(
 	/* the task that is about to send the response */
 	struct task *sender = self->tr_parent;
 
+	struct vm_region *src = sender->t_address_space,
+			 *dst = receiver->t_address_space;
+	unsigned long f;
+	vm_region_lock_pair_irqsave(src, dst, &f);
+
 	kern_status_t status = vm_region_memmove_v(
-		receiver->t_address_space,
+		dst,
 		0,
-		msg->msg_resp.msg_data,
+		msg->msg_resp_data,
-		msg->msg_resp.msg_data_count,
+		msg->msg_resp_data_count,
-		sender->t_address_space,
+		src,
 		0,
-		resp->msg_data,
+		resp_data,
-		resp->msg_data_count,
+		resp_data_count,
 		VM_REGION_COPY_ALL,
 		NULL);
-	if (status != KERN_OK) {
+	vm_region_unlock_pair_irqrestore(src, dst, f);
-		kmsg_reply_error(msg, status, &msg_lock_flags);
-		return status;
-	}
 
-	status = handle_list_transfer(
-		receiver->t_handles,
-		msg->msg_resp.msg_handles,
-		msg->msg_resp.msg_handles_count,
-		sender->t_handles,
-		resp->msg_handles,
-		resp->msg_handles_count);
 	if (status != KERN_OK) {
 		kmsg_reply_error(msg, status, &msg_lock_flags);
 		return status;
@@ -251,7 +246,7 @@ extern kern_status_t channel_read_msg(
 	size_t *nr_read)
 {
 	unsigned long msg_lock_flags;
-	struct kmsg *msg = get_msg_with_id(&channel->c_msg, id);
+	struct msg *msg = get_msg_with_id(&channel->c_msg, id);
 	if (!msg) {
 		return KERN_INVALID_ARGUMENT;
 	}
@@ -262,17 +257,24 @@ extern kern_status_t channel_read_msg(
 		return KERN_INVALID_ARGUMENT;
 	}
 
+	struct vm_region *src_region
+		= msg->msg_sender_thread->tr_parent->t_address_space;
+
+	unsigned long f;
+	vm_region_lock_pair_irqsave(src_region, dest_region, &f);
+
 	kern_status_t status = vm_region_memmove_v(
 		dest_region,
 		0,
 		dest_iov,
 		dest_iov_count,
-		msg->msg_sender_thread->tr_parent->t_address_space,
+		src_region,
 		offset,
-		msg->msg_req.msg_data,
+		msg->msg_req_data,
-		msg->msg_req.msg_data_count,
+		msg->msg_req_data_count,
 		VM_REGION_COPY_ALL,
 		nr_read);
+	vm_region_unlock_pair_irqrestore(src_region, dest_region, f);
 
 	spin_unlock_irqrestore(&msg->msg_lock, msg_lock_flags);
 
@@ -281,12 +283,45 @@ extern kern_status_t channel_read_msg(
 
 extern kern_status_t channel_write_msg(
 	struct channel *channel,
-	msgid_t msg,
+	msgid_t id,
 	size_t offset,
 	struct vm_region *src_region,
 	const struct iovec *src_iov,
 	size_t src_iov_count,
 	size_t *nr_written)
 {
-	return KERN_UNIMPLEMENTED;
+	unsigned long msg_lock_flags;
+	struct msg *msg = get_msg_with_id(&channel->c_msg, id);
+	if (!msg) {
+		return KERN_INVALID_ARGUMENT;
+	}
+
+	spin_lock_irqsave(&msg->msg_lock, &msg_lock_flags);
+	if (msg->msg_status != KMSG_WAIT_REPLY) {
+		spin_unlock_irqrestore(&msg->msg_lock, msg_lock_flags);
+		return KERN_INVALID_ARGUMENT;
+	}
+
+	struct vm_region *dest_region
+		= msg->msg_sender_thread->tr_parent->t_address_space;
+
+	unsigned long f;
+	vm_region_lock_pair_irqsave(src_region, dest_region, &f);
+
+	kern_status_t status = vm_region_memmove_v(
+		dest_region,
+		0,
+		msg->msg_resp_data,
+		msg->msg_resp_data_count,
+		src_region,
+		offset,
+		src_iov,
+		src_iov_count,
+		VM_REGION_COPY_ALL,
+		nr_written);
+	vm_region_unlock_pair_irqrestore(src_region, dest_region, f);
+
+	spin_unlock_irqrestore(&msg->msg_lock, msg_lock_flags);
+
+	return status;
 }

kernel/handle.c

@@ -191,123 +191,3 @@ struct handle *handle_table_get_handle(
 
 	return &tab->t_handles.t_handle_list[handle_index];
 }
-
-struct handle_list_iterator {
-	struct handle_list *it_list;
-	size_t it_list_count;
-	size_t it_list_ptr;
-
-	kern_handle_t *it_handles;
-	size_t it_nr_handles;
-};
-
-static void handle_list_iterator_begin(
-	struct handle_list_iterator *it,
-	struct handle_list *list,
-	size_t list_count)
-{
-	memset(it, 0x0, sizeof *it);
-	it->it_list = list;
-	it->it_list_count = list_count;
-
-	while (it->it_list_ptr < list_count) {
-		if (list[it->it_list_ptr].l_nr_handles > 0) {
-			break;
-		}
-
-		it->it_list_ptr++;
-	}
-
-	if (it->it_list_ptr >= list_count) {
-		return;
-	}
-
-	it->it_handles = list[it->it_list_ptr].l_handles;
-	it->it_nr_handles = list[it->it_list_ptr].l_nr_handles;
-}
-
-static void handle_list_iterator_seek(
-	struct handle_list_iterator *it,
-	size_t nr_handles)
-{
-	if (nr_handles > it->it_nr_handles) {
-		nr_handles = it->it_nr_handles;
-	}
-
-	if (nr_handles < it->it_nr_handles) {
-		it->it_handles += nr_handles;
-		it->it_nr_handles -= nr_handles;
-		return;
-	}
-
-	it->it_list_ptr++;
-	while (it->it_list_ptr < it->it_list_count) {
-		if (it->it_list[it->it_list_ptr].l_nr_handles > 0) {
-			break;
-		}
-
-		it->it_list_ptr++;
-	}
-
-	if (it->it_list_ptr >= it->it_list_count) {
-		return;
-	}
-
-	it->it_handles = it->it_list[it->it_list_ptr].l_handles;
-	it->it_nr_handles = it->it_list[it->it_list_ptr].l_nr_handles;
-}
-
-kern_status_t handle_list_transfer(
-	struct handle_table *dest_table,
-	struct handle_list *dest_list,
-	size_t dest_list_count,
-	struct handle_table *src_table,
-	const struct handle_list *src_list,
-	size_t src_list_count)
-{
-	struct handle_list_iterator src, dest;
-	handle_list_iterator_begin(
-		&src,
-		(struct handle_list *)src_list,
-		src_list_count);
-	handle_list_iterator_begin(&dest, dest_list, dest_list_count);
-
-	while (src.it_nr_handles && dest.it_nr_handles) {
-		size_t to_copy = MIN(src.it_nr_handles, dest.it_nr_handles);
-		for (size_t i = 0; i < to_copy; i++) {
-			kern_handle_t handle_v = src.it_handles[i];
-			struct handle *handle
-				= handle_table_get_handle(src_table, handle_v);
-			if (!handle) {
-				return KERN_HANDLE_INVALID;
-			}
-
-			struct object *obj = object_ref(handle->h_object);
-			handle_flags_t flags = handle->h_flags;
-
-			handle_table_free_handle(src_table, handle_v);
-
-			struct handle *dest_slot = NULL;
-			kern_status_t status = handle_table_alloc_handle(
-				dest_table,
-				&dest_slot,
-				&handle_v);
-			if (status != KERN_OK) {
-				return status;
-			}
-
-			dest_slot->h_object = obj;
-			dest_slot->h_flags = flags;
-
-			object_add_handle(obj);
-			object_unref(obj);
-
-			dest.it_handles[i] = handle_v;
-		}
-
-		handle_list_iterator_seek(&src, to_copy);
-		handle_list_iterator_seek(&dest, to_copy);
-	}
-
-	return KERN_OK;
-}

kernel/object.c

@@ -178,6 +178,38 @@ void object_unlock_irqrestore(struct object *obj, unsigned long flags)
 	spin_unlock_irqrestore(&obj->ob_lock, flags);
 }
 
+void object_lock_pair_irqsave(
+	struct object *a,
+	struct object *b,
+	unsigned long *flags)
+{
+	if (a == b) {
+		object_lock_irqsave(a, flags);
+	} else if (a < b) {
+		object_lock_irqsave(a, flags);
+		object_lock(b);
+	} else {
+		object_lock_irqsave(b, flags);
+		object_lock(a);
+	}
+}
+
+void object_unlock_pair_irqrestore(
+	struct object *a,
+	struct object *b,
+	unsigned long flags)
+{
+	if (a == b) {
+		object_unlock_irqrestore(a, flags);
+	} else if (a < b) {
+		object_unlock(b);
+		object_unlock_irqrestore(a, flags);
+	} else {
+		object_unlock(a);
+		object_unlock_irqrestore(b, flags);
+	}
+}
+
 void *object_data(struct object *obj)
 {
 	return (char *)obj + sizeof *obj;

kernel/port.c

@@ -20,7 +20,7 @@ struct port *port_cast(struct object *obj)
 	return PORT_CAST(obj);
 }
 
-static void wait_for_reply(struct kmsg *msg, unsigned long *lock_flags)
+static void wait_for_reply(struct msg *msg, unsigned long *lock_flags)
 {
 	struct wait_item waiter;
 	struct thread *self = current_thread();
@@ -78,8 +78,10 @@ kern_status_t port_disconnect(struct port *port)
 
 kern_status_t port_send_msg(
 	struct port *port,
-	const struct msg *req,
+	const struct iovec *req_data,
-	struct msg *resp,
+	size_t req_data_count,
+	struct iovec *resp_data,
+	size_t resp_data_count,
 	unsigned long *lock_flags)
 {
 	if (port->p_status != PORT_READY) {
@@ -87,13 +89,15 @@ kern_status_t port_send_msg(
 	}
 
 	struct thread *self = current_thread();
-	struct kmsg *msg = &self->tr_msg;
+	struct msg *msg = &self->tr_msg;
 	memset(msg, 0x0, sizeof *msg);
 	msg->msg_status = KMSG_WAIT_RECEIVE;
 	msg->msg_sender_thread = self;
 	msg->msg_sender_port = port;
-	msg->msg_req = *req;
+	msg->msg_req_data = req_data;
-	msg->msg_resp = *resp;
+	msg->msg_req_data_count = req_data_count;
+	msg->msg_resp_data = resp_data;
+	msg->msg_resp_data_count = resp_data_count;
 
 	unsigned long flags;
 	channel_lock_irqsave(port->p_remote, &flags);
@@ -103,5 +107,9 @@ kern_status_t port_send_msg(
 
 	wait_for_reply(msg, lock_flags);
 
+	channel_lock_irqsave(port->p_remote, &flags);
+	btree_delete(&port->p_remote->c_msg, &msg->msg_node);
+	channel_unlock_irqrestore(port->p_remote, flags);
+
 	return msg->msg_result;
 }

libmango/arch/x86_64/syscall.S

@@ -82,15 +82,13 @@ SYSCALL_GATE kern_handle_close SYS_KERN_HANDLE_CLOSE 1
 SYSCALL_GATE kern_config_get SYS_KERN_CONFIG_GET 3
 SYSCALL_GATE kern_config_set SYS_KERN_CONFIG_SET 3
 
-SYSCALL_GATE channel_create SYS_CHANNEL_CREATE 3
+SYSCALL_GATE channel_create SYS_CHANNEL_CREATE 2
 SYSCALL_GATE port_create SYS_PORT_CREATE 1
 SYSCALL_GATE port_connect SYS_PORT_CONNECT 3
 SYSCALL_GATE port_disconnect SYS_PORT_DISCONNECT 1
-SYSCALL_GATE msg_send SYS_MSG_SEND 4
+SYSCALL_GATE msg_send SYS_MSG_SEND 5
 SYSCALL_GATE msg_recv SYS_MSG_RECV 4
 SYSCALL_GATE msg_reply SYS_MSG_REPLY 4
-SYSCALL_GATE msg_read SYS_MSG_READ 5
+SYSCALL_GATE msg_read SYS_MSG_READ 6
-SYSCALL_GATE msg_read_handles SYS_MSG_READ_HANDLES 5
+SYSCALL_GATE msg_write SYS_MSG_WRITE 6
-SYSCALL_GATE msg_write SYS_MSG_WRITE 5
-SYSCALL_GATE msg_write_handles SYS_MSG_WRITE_HANDLES 5
 

libmango/include-user/mango/msg.h

@@ -4,10 +4,7 @@
 #include <mango/status.h>
 #include <mango/types.h>
 
-extern kern_status_t channel_create(
+extern kern_status_t channel_create(unsigned int id, kern_handle_t *out);
-	unsigned int id,
-	channel_flags_t flags,
-	kern_handle_t *out);
 extern kern_status_t port_create(kern_handle_t *out);
 extern kern_status_t port_connect(
 	kern_handle_t port,
@@ -17,21 +14,22 @@ extern kern_status_t port_disconnect(kern_handle_t port);
 
 extern kern_status_t msg_send(
 	kern_handle_t port,
-	msg_flags_t flags,
+	const struct iovec *req_data,
-	const struct msg *req,
+	size_t req_data_count,
-	struct msg *resp);
+	struct iovec *resp_data,
+	size_t resp_data_count);
 
 extern kern_status_t msg_recv(
 	kern_handle_t channel,
-	msg_flags_t flags,
 	msgid_t *out_id,
-	struct msg *out_msg);
+	struct iovec *out_data,
+	size_t out_data_count);
 
 extern kern_status_t msg_reply(
 	kern_handle_t channel,
-	msg_flags_t flags,
 	msgid_t id,
-	const struct msg *reply);
+	const struct iovec *reply_data,
+	size_t reply_data_count);
 
 extern kern_status_t msg_read(
 	kern_handle_t channel,
@@ -40,24 +38,13 @@ extern kern_status_t msg_read(
 	struct iovec *out,
 	size_t out_count,
 	size_t *nr_read);
-extern kern_status_t msg_read_handles(
-	kern_handle_t channel,
-	msgid_t id,
-	size_t offset,
-	struct handle_list *out,
-	size_t nr_out);
 
 extern kern_status_t msg_write(
 	kern_handle_t channel,
 	msgid_t id,
 	size_t offset,
 	const struct iovec *in,
-	size_t nr_in);
+	size_t nr_in,
-extern kern_status_t msg_write_handles(
+	size_t *nr_written);
-	kern_handle_t channel,
-	msgid_t id,
-	size_t offset,
-	const struct handle_list *in,
-	size_t nr_in);
 
 #endif

libmango/include/mango/syscall.h

@@ -27,9 +27,7 @@
 #define SYS_MSG_RECV                 19
 #define SYS_MSG_REPLY                20
 #define SYS_MSG_READ                 21
-#define SYS_MSG_READ_HANDLES         22
 #define SYS_MSG_WRITE                23
-#define SYS_MSG_WRITE_HANDLES        24
 #define SYS_CHANNEL_CREATE           25
 #define SYS_PORT_CREATE              26
 #define SYS_PORT_CONNECT             27

libmango/include/mango/types.h

@@ -50,8 +50,6 @@ typedef unsigned int tid_t;
 typedef uint32_t kern_handle_t;
 typedef uint32_t kern_config_key_t;
 typedef uint32_t vm_prot_t;
-typedef uint32_t channel_flags_t;
-typedef uint32_t msg_flags_t;
 
 typedef unsigned int umode_t;
 
@@ -60,17 +58,4 @@ struct iovec {
 	size_t io_len;
 };
 
-struct handle_list {
-	kern_handle_t *l_handles;
-	size_t l_nr_handles;
-};
-
-struct msg {
-	struct iovec *msg_data;
-	size_t msg_data_count;
-
-	struct handle_list *msg_handles;
-	size_t msg_handles_count;
-};
-
 #endif

syscall/dispatch.c

@@ -35,9 +35,7 @@ static const virt_addr_t syscall_table[] = {
 	SYSCALL_TABLE_ENTRY(MSG_RECV, msg_recv),
 	SYSCALL_TABLE_ENTRY(MSG_REPLY, msg_reply),
 	SYSCALL_TABLE_ENTRY(MSG_READ, msg_read),
-	SYSCALL_TABLE_ENTRY(MSG_READ_HANDLES, msg_read_handles),
 	SYSCALL_TABLE_ENTRY(MSG_WRITE, msg_write),
-	SYSCALL_TABLE_ENTRY(MSG_WRITE_HANDLES, msg_write_handles),
 };
 static const size_t syscall_table_count
 	= sizeof syscall_table / sizeof syscall_table[0];

syscall/log.c

@@ -4,6 +4,6 @@
 kern_status_t sys_kern_log(const char *s)
 {
 	struct task *task = current_task();
-	printk("%s: %s", task->t_name, s);
+	printk("%s[%d]: %s", task->t_name, task->t_id, s);
 	return KERN_OK;
 }

syscall/msg.c

@@ -5,10 +5,7 @@
 #include <kernel/syscall.h>
 #include <kernel/vm-region.h>
 
-kern_status_t sys_channel_create(
+kern_status_t sys_channel_create(unsigned int id, kern_handle_t *out)
-	unsigned int id,
-	channel_flags_t flags,
-	kern_handle_t *out)
 {
 	struct task *self = current_task();
 	if (!validate_access_w(self, out, sizeof *out)) {
@@ -168,6 +165,10 @@ static bool validate_iovec(
 	size_t count,
 	bool rw)
 {
+	if (!validate_access_r(task, iov, count * sizeof(struct iovec))) {
+		return false;
+	}
+
 	for (size_t i = 0; i < count; i++) {
 		bool ok = false;
 		const struct iovec *vec = &iov[i];
@@ -185,69 +186,21 @@ static bool validate_iovec(
 	return true;
 }
 
-static bool validate_msg(struct task *task, const struct msg *msg, bool rw)
-{
-	if (!validate_access_r(task, msg, sizeof *msg)) {
-		return false;
-	}
-
-	if (msg->msg_data_count
-	    && !validate_access_r(
-		    task,
-		    msg->msg_data,
-		    sizeof(struct iovec) * msg->msg_data_count)) {
-		return false;
-	}
-
-	if (msg->msg_handles_count
-	    && !validate_access_r(
-		    task,
-		    msg->msg_handles,
-		    sizeof(struct handle_list) * msg->msg_handles_count)) {
-		return false;
-	}
-
-	if (!validate_iovec(task, msg->msg_data, msg->msg_data_count, rw)) {
-		return false;
-	}
-
-	for (size_t i = 0; i < msg->msg_handles_count; i++) {
-		bool ok = false;
-		const struct handle_list *list = &msg->msg_handles[i];
-		if (rw) {
-			ok = validate_access_w(
-				task,
-				list->l_handles,
-				list->l_nr_handles * sizeof(kern_handle_t));
-		} else {
-			ok = validate_access_r(
-				task,
-				list->l_handles,
-				list->l_nr_handles * sizeof(kern_handle_t));
-		}
-
-		if (!ok) {
-			return false;
-		}
-	}
-
-	return true;
-}
-
 kern_status_t sys_msg_send(
 	kern_handle_t port_handle,
-	msg_flags_t msg_flags,
+	const struct iovec *req_data,
-	const struct msg *req,
+	size_t req_data_count,
-	struct msg *resp)
+	struct iovec *resp_data,
+	size_t resp_data_count)
 {
 	struct task *self = current_task();
 
-	if (!validate_msg(self, req, false)) {
+	if (!validate_iovec(self, req_data, req_data_count, false)) {
-		return KERN_MEMORY_FAULT;
+		return false;
 	}
 
-	if (!validate_msg(self, resp, true)) {
+	if (!validate_iovec(self, resp_data, resp_data_count, true)) {
-		return KERN_MEMORY_FAULT;
+		return false;
 	}
 
 	unsigned long flags;
@@ -277,7 +230,13 @@ kern_status_t sys_msg_send(
 	}
 
 	port_lock_irqsave(port, &flags);
-	status = port_send_msg(port, req, resp, &flags);
+	status = port_send_msg(
+		port,
+		req_data,
+		req_data_count,
+		resp_data,
+		resp_data_count,
+		&flags);
 	port_unlock_irqrestore(port, flags);
 	object_unref(port_obj);
 
@@ -286,9 +245,9 @@ kern_status_t sys_msg_send(
 
 kern_status_t sys_msg_recv(
 	kern_handle_t channel_handle,
-	msg_flags_t msg_flags,
 	msgid_t *out_id,
-	struct msg *out_msg)
+	struct iovec *out_data,
+	size_t out_data_count)
 {
 	struct task *self = current_task();
 
@@ -296,7 +255,7 @@ kern_status_t sys_msg_recv(
 		return KERN_MEMORY_FAULT;
 	}
 
-	if (!validate_msg(self, out_msg, true)) {
+	if (!validate_iovec(self, out_data, out_data_count, true)) {
 		return KERN_MEMORY_FAULT;
 	}
 
@@ -327,7 +286,12 @@ kern_status_t sys_msg_recv(
 	}
 
 	channel_lock_irqsave(channel, &flags);
-	status = channel_recv_msg(channel, out_msg, out_id, &flags);
+	status = channel_recv_msg(
+		channel,
+		out_id,
+		out_data,
+		out_data_count,
+		&flags);
 	channel_unlock_irqrestore(channel, flags);
 	object_unref(channel_obj);
 
@@ -336,13 +300,13 @@ kern_status_t sys_msg_recv(
 
 kern_status_t sys_msg_reply(
 	kern_handle_t channel_handle,
-	msg_flags_t msg_flags,
 	msgid_t id,
-	const struct msg *reply)
+	const struct iovec *reply_data,
+	size_t reply_data_count)
 {
 	struct task *self = current_task();
 
-	if (!validate_msg(self, reply, false)) {
+	if (!validate_iovec(self, reply_data, reply_data_count, false)) {
 		return KERN_MEMORY_FAULT;
 	}
 
@@ -373,7 +337,12 @@ kern_status_t sys_msg_reply(
 	}
 
 	channel_lock_irqsave(channel, &flags);
-	status = channel_reply_msg(channel, id, reply, &flags);
+	status = channel_reply_msg(
+		channel,
+		id,
+		reply_data,
+		reply_data_count,
+		&flags);
 	channel_unlock_irqrestore(channel, flags);
 	object_unref(channel_obj);
 
@@ -390,6 +359,14 @@ kern_status_t sys_msg_read(
 {
 	struct task *self = current_task();
 
+	if (nr_read && !validate_access_w(self, nr_read, sizeof *nr_read)) {
+		return KERN_MEMORY_FAULT;
+	}
+
+	if (!validate_iovec(self, iov, iov_count, true)) {
+		return KERN_MEMORY_FAULT;
+	}
+
 	unsigned long flags;
 
 	task_lock_irqsave(self, &flags);
@@ -417,7 +394,6 @@ kern_status_t sys_msg_read(
 	}
 
 	channel_lock_irqsave(channel, &flags);
-	vm_region_lock(self->t_address_space);
 	status = channel_read_msg(
 		channel,
 		id,
@@ -426,39 +402,68 @@ kern_status_t sys_msg_read(
 		iov,
 		iov_count,
 		nr_read);
-	vm_region_unlock(self->t_address_space);
 	channel_unlock_irqrestore(channel, flags);
 	object_unref(channel_obj);
 
 	return status;
 }
 
-kern_status_t sys_msg_read_handles(
-	kern_handle_t channel,
-	msgid_t id,
-	size_t offset,
-	struct handle_list *out,
-	size_t nr_out)
-{
-	return KERN_UNIMPLEMENTED;
-}
-
 kern_status_t sys_msg_write(
-	kern_handle_t channel,
+	kern_handle_t channel_handle,
 	msgid_t id,
 	size_t offset,
-	const struct iovec *in,
+	const struct iovec *iov,
-	size_t nr_in)
+	size_t iov_count,
+	size_t *nr_written)
 {
-	return KERN_UNIMPLEMENTED;
+	struct task *self = current_task();
-}
 
-kern_status_t sys_msg_write_handles(
+	if (nr_written
-	kern_handle_t channel,
+	    && !validate_access_w(self, nr_written, sizeof *nr_written)) {
-	msgid_t id,
+		return KERN_MEMORY_FAULT;
-	size_t offset,
+	}
-	const struct handle_list *in,
+
-	size_t nr_in)
+	if (!validate_iovec(self, iov, iov_count, false)) {
-{
+		return KERN_MEMORY_FAULT;
-	return KERN_UNIMPLEMENTED;
+	}
+
+	unsigned long flags;
+
+	task_lock_irqsave(self, &flags);
+
+	struct object *channel_obj = NULL;
+	handle_flags_t channel_handle_flags = 0;
+	kern_status_t status = task_resolve_handle(
+		self,
+		channel_handle,
+		&channel_obj,
+		&channel_handle_flags);
+	if (status != KERN_OK) {
+		return status;
+	}
+
+	/* add a reference to the port object to make sure it isn't deleted
+	 * while we're using it */
+	object_ref(channel_obj);
+	task_unlock_irqrestore(self, flags);
+
+	struct channel *channel = channel_cast(channel_obj);
+	if (!channel) {
+		object_unref(channel_obj);
+		return KERN_INVALID_ARGUMENT;
+	}
+
+	channel_lock_irqsave(channel, &flags);
+	status = channel_write_msg(
+		channel,
+		id,
+		offset,
+		self->t_address_space,
+		iov,
+		iov_count,
+		nr_written);
+	channel_unlock_irqrestore(channel, flags);
+	object_unref(channel_obj);
+
+	return status;
 }

syscall/task.c

@@ -6,9 +6,10 @@
 
 extern kern_status_t sys_task_exit(int status)
 {
+	struct task *self = current_task();
+	printk("%s[%d]: task_exit(%d)", self->t_name, self->t_id, status);
 	while (1) {
-		printk("sys_exit(%d)", status);
+		milli_sleep(5000);
-		milli_sleep(1000);
 	}
 
 	return KERN_UNIMPLEMENTED;

vm/vm-region.c

@@ -36,6 +36,15 @@
 	region_find_free_area_linear(region, length)
 #endif
 
+#define unlock_mapping_parent(p, root)                                         \
+	do {                                                                   \
+		struct vm_region *parent                                       \
+			= region_from_entry(p->m_entry.e_parent);              \
+		if (parent != root) {                                          \
+			vm_region_unlock(parent);                              \
+		}                                                              \
+	} while (0)
+
 /* iterates over a range of mapped virtual memory in a region, and provides
  * a moving buffer through which the memory can be accessed */
 struct vm_iterator {
@@ -296,12 +305,13 @@ static struct vm_region *region_get_child_region_recursive(
  * this function should be called with `region` locked. if a mapping is found,
  * it will be returned with its immediate parent locked. */
 static struct vm_region_mapping *region_get_mapping_recursive(
-	struct vm_region *region,
+	struct vm_region *root,
 	off_t *offp,
 	size_t len)
 {
 	off_t offset = *offp;
-	region = region_get_child_region_recursive(region, &offset, len);
+	struct vm_region *region
+		= region_get_child_region_recursive(root, &offset, len);
 	if (!region) {
 		return NULL;
 	}
@@ -311,6 +321,14 @@ static struct vm_region_mapping *region_get_mapping_recursive(
 	struct vm_region_entry *entry = region_get_entry(region, offset, len);
 	*offp = offset;
 
+	if (!entry) {
+		if (region != root) {
+			vm_region_unlock(region);
+		}
+
+		return NULL;
+	}
+
 	/* return the mapping with the parent region still locked */
 	return mapping_from_entry(entry);
 }
@@ -593,7 +611,12 @@ static void vm_iterator_begin(
 
 	off_t offset = base - vm_region_get_base_address(region);
 	it->it_mapping = region_get_mapping_recursive(region, &offset, 1);
-	if (!it->it_mapping || (it->it_mapping->m_prot & prot) != prot) {
+	if (!it->it_mapping) {
+		return;
+	}
+
+	if ((it->it_mapping->m_prot & prot) != prot) {
+		unlock_mapping_parent(it->it_mapping, region);
 		return;
 	}
 
@@ -612,6 +635,7 @@ static void vm_iterator_begin(
 	}
 
 	if (!pg) {
+		unlock_mapping_parent(it->it_mapping, region);
 		return;
 	}
 
@@ -643,15 +667,6 @@ static void vm_iterator_begin(
 
 static kern_status_t vm_iterator_seek(struct vm_iterator *it, size_t nr_bytes)
 {
-#define UNLOCK_MAPPING_PARENT(p)                                               \
-	do {                                                                   \
-		struct vm_region *parent                                       \
-			= region_from_entry(p->m_entry.e_parent);              \
-		if (parent != it->it_region) {                                 \
-			vm_region_unlock(parent);                              \
-		}                                                              \
-	} while (0)
-
 	if (nr_bytes < it->it_max) {
 		it->it_base += nr_bytes;
 		it->it_buf = (char *)it->it_buf + nr_bytes;
@@ -661,7 +676,7 @@ static kern_status_t vm_iterator_seek(struct vm_iterator *it, size_t nr_bytes)
 
 	/* the parent region of it->it_mapping is locked here. if it is
 	 * different from it->it_region, it must be unlocked */
-	UNLOCK_MAPPING_PARENT(it->it_mapping);
+	unlock_mapping_parent(it->it_mapping, it->it_region);
 
 	it->it_base += nr_bytes;
 	off_t offset = it->it_base - vm_region_get_base_address(it->it_region);
@@ -674,13 +689,13 @@ static kern_status_t vm_iterator_seek(struct vm_iterator *it, size_t nr_bytes)
 		return KERN_MEMORY_FAULT;
 	}
 
-	/* past this point, if we encounter an error, must remember to unlock
+	/* past this point, if we encounter an error, must remember to
-	 * the parent region of next_mapping */
+	 * unlock the parent region of next_mapping */
 
 	if ((next_mapping->m_prot & it->it_prot) != it->it_prot) {
 		it->it_buf = NULL;
 		it->it_max = 0;
-		UNLOCK_MAPPING_PARENT(next_mapping);
+		unlock_mapping_parent(next_mapping, it->it_region);
 		return KERN_MEMORY_FAULT;
 	}
 
@@ -699,7 +714,7 @@ static kern_status_t vm_iterator_seek(struct vm_iterator *it, size_t nr_bytes)
 	}
 
 	if (!pg) {
-		UNLOCK_MAPPING_PARENT(next_mapping);
+		unlock_mapping_parent(next_mapping, it->it_region);
 		return KERN_NO_MEMORY;
 	}
 
@@ -730,9 +745,20 @@ static kern_status_t vm_iterator_seek(struct vm_iterator *it, size_t nr_bytes)
 	return KERN_OK;
 }
 
-/* this function must be called with `root` locked. `root` will be the first
+/* this function must be called when you are finished with a
- * entry visited by the iterator. from there, child entries are visited in
+ * vm_iterator, to ensure that all held locks are released. */
- * depth-first order. */
+static void vm_iterator_finish(struct vm_iterator *it)
+{
+	if (it->it_mapping) {
+		unlock_mapping_parent(it->it_mapping, it->it_region);
+	}
+
+	memset(it, 0x0, sizeof *it);
+}
+
+/* this function must be called with `root` locked. `root` will be the
+ * first entry visited by the iterator. from there, child entries are
+ * visited in depth-first order. */
 static void entry_iterator_begin(
 	struct entry_iterator *it,
 	struct vm_region *root)
@@ -742,8 +768,8 @@ static void entry_iterator_begin(
 	it->it_entry = &root->vr_entry;
 }
 
-/* this function must be called when you are finished with an entry_iterator,
+/* this function must be called when you are finished with an
- * to ensure that all held locks are released. */
+ * entry_iterator, to ensure that all held locks are released. */
 static void entry_iterator_finish(struct entry_iterator *it)
 {
 	struct vm_region_entry *cur = it->it_entry;
@@ -771,10 +797,10 @@ static void entry_iterator_finish(struct entry_iterator *it)
 /* move to the next entry in the traversal order.
  * when this function returns:
  *   1. if the visited entry is a region, it will be locked.
- *   2. if the visited entry is a mapping, its parent region will be locked.
+ *   2. if the visited entry is a mapping, its parent region will be
- * a region will remain locked until all of its children and n-grand-children
+ * locked. a region will remain locked until all of its children and
- * have been visited. once iteration is finished, only `it->it_root` will be
+ * n-grand-children have been visited. once iteration is finished, only
- * locked.
+ * `it->it_root` will be locked.
  */
 static void entry_iterator_move_next(struct entry_iterator *it)
 {
@@ -791,8 +817,9 @@ static void entry_iterator_move_next(struct entry_iterator *it)
 		if (entry->e_type == VM_REGION_ENTRY_REGION) {
 			struct vm_region *child_region
 				= region_from_entry(entry);
-			/* since `region` is locked, interrupts are already
+			/* since `region` is locked, interrupts are
-			 * disabled, so don't use lock_irq() here */
+			 * already disabled, so don't use lock_irq()
+			 * here */
 			vm_region_lock(child_region);
 		}
 
@@ -839,8 +866,9 @@ static void entry_iterator_move_next(struct entry_iterator *it)
 	}
 }
 
-/* erase the current entry and move to the next entry in the traversal order.
+/* erase the current entry and move to the next entry in the traversal
- * the current entry MUST be a mapping, otherwise nothing will happen.
+ * order. the current entry MUST be a mapping, otherwise nothing will
+ * happen.
  */
 static void entry_iterator_erase(struct entry_iterator *it)
 {
@@ -999,7 +1027,8 @@ struct vm_region *vm_region_cast(struct object *obj)
 	return VM_REGION_CAST(obj);
 }
 
-/* this function should be called with `parent` locked (if parent is non-NULL)
+/* this function should be called with `parent` locked (if parent is
+ * non-NULL)
  */
 kern_status_t vm_region_create(
 	struct vm_region *parent,
@@ -1088,9 +1117,9 @@ kern_status_t vm_region_kill(
 			= region_from_entry(region->vr_entry.e_parent);
 		region->vr_entry.e_parent = NULL;
 
-		/* locks must be acquired in parent->child order. since we're
+		/* locks must be acquired in parent->child order. since
-		 * going backwards here, unlock `region` before locking its
+		 * we're going backwards here, unlock `region` before
-		 * parent */
+		 * locking its parent */
 		vm_region_unlock_irqrestore(region, *lock_flags);
 		vm_region_lock_irqsave(parent, lock_flags);
 		btree_delete(&parent->vr_entries, &region->vr_entry.e_node);
@@ -1175,8 +1204,8 @@ kern_status_t vm_region_map_object(
 			root,
 			&region_offset,
 			length);
-		/* if `region` != `root`, it will need to be unlocked at the end
+		/* if `region` != `root`, it will need to be unlocked at
-		 * of the function */
+		 * the end of the function */
 	}
 
 	if (region->vr_status != VM_REGION_ONLINE) {
@@ -1257,8 +1286,8 @@ kern_status_t vm_region_map_object(
 	return KERN_OK;
 }
 
-/* unmap some pages in the middle of a mapping, splitting it into two separate
+/* unmap some pages in the middle of a mapping, splitting it into two
- * mappings */
+ * separate mappings */
 static kern_status_t split_mapping(
 	struct vm_region_mapping *mapping,
 	struct vm_region *root,
@@ -1488,7 +1517,8 @@ kern_status_t vm_region_unmap(
 				unmap_area_offset,
 				unmap_area_limit);
 		} else {
-			panic("don't know what to do with this mapping");
+			panic("don't know what to do with this "
+			      "mapping");
 		}
 
 		if (delete) {
@@ -1557,15 +1587,18 @@ bool vm_region_validate_access(
 			return false;
 		}
 
-		if ((mapping->m_prot & prot) != prot) {
+		bool ok = (mapping->m_prot & prot) == prot;
-			return false;
-		}
 
 		struct vm_region *parent
 			= region_from_entry(mapping->m_entry.e_parent);
+
 		if (parent != region) {
 			vm_region_unlock(parent);
 		}
+
+		if (!ok) {
+			return false;
+		}
 	}
 
 	return true;
@@ -1671,6 +1704,8 @@ kern_status_t vm_region_read_kernel(
 		dest += to_move;
 	}
 
+	vm_iterator_finish(&src);
+
 	if (nr_read) {
 		*nr_read = r;
 	}
@@ -1727,6 +1762,9 @@ kern_status_t vm_region_memmove(
 		r += to_move;
 	}
 
+	vm_iterator_finish(&src);
+	vm_iterator_finish(&dest);
+
 	if (nr_moved) {
 		*nr_moved = r;
 	}