[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 7/7] replace mach_port_t with mach_port_name_t
From: |
Samuel Thibault |
Subject: |
Re: [PATCH 7/7] replace mach_port_t with mach_port_name_t |
Date: |
Wed, 18 Jan 2023 02:38:05 +0100 |
User-agent: |
NeoMutt/20170609 (1.8.3) |
Applied, thanks!
Luca Dariz, le lun. 16 janv. 2023 11:58:57 +0100, a ecrit:
> This is a cleanup following the introduction of mach_port_name_t.
> The same set of changes is applied to all files:
> - rename mach_port_t to mach_port_name_t where a port name is used,
> - use MACH_PORT_NAME_NULL and MACH_PORT_NAME_DEAD where appropriate,
> - use invalid_port_to_name() and invalid_name_to_port() for conversion
> where appropriate,
> - use regular copyout() insted of copyout_port() when we deal with
> mach_port_name_t already before copyout,
> - use the new helper ipc_kmsg_copyout_object_to_port() when we really
> want to place a port name in the space of a mach_port_t.
>
> * include/mach/notify.h: Likewise
> * ipc/ipc_entry.c: Likewise
> * ipc/ipc_kmsg.c: Likewise
> * ipc/ipc_kmsg.h: Likewise, and add ipc_kmsg_copyout_object_to_port()
> * ipc/ipc_marequest.c: Likewise
> * ipc/ipc_object.c: Likewise
> * ipc/ipc_port.c: Likewise
> * ipc/ipc_space.h: Likewise
> * ipc/mach_msg.c: Likewise
> * ipc/mach_port.c: Likewise
> * kern/exception.c: Likewise
> * kern/ipc_mig.c: Likewise
> ---
> include/mach/notify.h | 6 +++---
> ipc/ipc_entry.c | 2 +-
> ipc/ipc_kmsg.c | 40 +++++++++++++++++++---------------------
> ipc/ipc_kmsg.h | 11 +++++++++++
> ipc/ipc_marequest.c | 4 ++--
> ipc/ipc_object.c | 4 ++--
> ipc/ipc_port.c | 6 +++---
> ipc/ipc_space.h | 2 +-
> ipc/mach_msg.c | 2 +-
> ipc/mach_port.c | 14 +++++++-------
> kern/exception.c | 12 ++++++------
> kern/ipc_mig.c | 16 ++++++++--------
> 12 files changed, 64 insertions(+), 55 deletions(-)
>
> diff --git a/include/mach/notify.h b/include/mach/notify.h
> index 6d783dde..14bcd6f6 100644
> --- a/include/mach/notify.h
> +++ b/include/mach/notify.h
> @@ -58,13 +58,13 @@
> typedef struct {
> mach_msg_header_t not_header;
> mach_msg_type_t not_type; /* MACH_MSG_TYPE_PORT_NAME */
> - mach_port_t not_port;
> + mach_port_name_t not_port;
> } mach_port_deleted_notification_t;
>
> typedef struct {
> mach_msg_header_t not_header;
> mach_msg_type_t not_type; /* MACH_MSG_TYPE_PORT_NAME */
> - mach_port_t not_port;
> + mach_port_name_t not_port;
> } mach_msg_accepted_notification_t;
>
> typedef struct {
> @@ -86,7 +86,7 @@ typedef struct {
> typedef struct {
> mach_msg_header_t not_header;
> mach_msg_type_t not_type; /* MACH_MSG_TYPE_PORT_NAME */
> - mach_port_t not_port;
> + mach_port_name_t not_port;
> } mach_dead_name_notification_t;
>
> #endif /* _MACH_NOTIFY_H_ */
> diff --git a/ipc/ipc_entry.c b/ipc/ipc_entry.c
> index c24ea46c..f13c442f 100644
> --- a/ipc/ipc_entry.c
> +++ b/ipc/ipc_entry.c
> @@ -127,7 +127,7 @@ ipc_entry_alloc_name(
> kern_return_t kr;
> ipc_entry_t entry, e, *prevp;
> void **slot;
> - assert(MACH_PORT_VALID(name));
> + assert(MACH_PORT_NAME_VALID(name));
>
> if (!space->is_active) {
> return KERN_INVALID_TASK;
> diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c
> index 495c4672..2477c576 100644
> --- a/ipc/ipc_kmsg.c
> +++ b/ipc/ipc_kmsg.c
> @@ -44,6 +44,7 @@
> #include <machine/locore.h>
> #include <machine/copy_user.h>
> #include <kern/assert.h>
> +#include <kern/debug.h>
> #include <kern/kalloc.h>
> #include <vm/vm_map.h>
> #include <vm/vm_object.h>
> @@ -1078,7 +1079,7 @@ ipc_kmsg_copyin_header(
> reply_soright = soright;
> }
> }
> - } else if (!MACH_PORT_VALID(reply_name)) {
> + } else if (!MACH_PORT_NAME_VALID(reply_name)) {
> ipc_entry_t entry;
>
> /*
> @@ -1101,7 +1102,7 @@ ipc_kmsg_copyin_header(
> if (IE_BITS_TYPE(entry->ie_bits) == MACH_PORT_TYPE_NONE)
> ipc_entry_dealloc(space, dest_name, entry);
>
> - reply_port = (ipc_object_t) reply_name;
> + reply_port = (ipc_object_t) invalid_name_to_port(reply_name);
> reply_soright = IP_NULL;
> } else {
> ipc_entry_t dest_entry, reply_entry;
> @@ -1461,10 +1462,10 @@ ipc_kmsg_copyin_body(
> ((mach_msg_type_t*)type)->msgt_name = newname;
>
> for (i = 0; i < number; i++) {
> - mach_port_name_t port = (mach_port_name_t)
> objects[i];
> + mach_port_name_t port = ((mach_port_t*)data)[i];
> ipc_object_t object;
>
> - if (!MACH_PORT_VALID(port))
> + if (!MACH_PORT_NAME_VALID(port))
> continue;
>
> kr = ipc_object_copyin(space, port,
> @@ -1846,7 +1847,7 @@ ipc_kmsg_copyout_header(
> entry->ie_bits = gen | (MACH_PORT_TYPE_SEND_ONCE | 1);
> }
>
> - assert(MACH_PORT_VALID(reply_name));
> + assert(MACH_PORT_NAME_VALID(reply_name));
> entry->ie_object = (ipc_object_t) reply;
> is_write_unlock(space);
>
> @@ -2021,7 +2022,7 @@ ipc_kmsg_copyout_header(
> is_write_unlock(space);
>
> reply = IP_DEAD;
> - reply_name = MACH_PORT_DEAD;
> + reply_name = MACH_PORT_NAME_DEAD;
> goto copyout_dest;
> }
>
> @@ -2132,7 +2133,7 @@ ipc_kmsg_copyout_header(
> ip_lock(dest);
> is_read_unlock(space);
>
> - reply_name = (mach_port_name_t) reply;
> + reply_name = invalid_port_to_name(msg->msgh_local_port);
> }
>
> /*
> @@ -2201,12 +2202,12 @@ ipc_kmsg_copyout_header(
> if (ip_active(reply) ||
> IP_TIMESTAMP_ORDER(timestamp,
> reply->ip_timestamp))
> - dest_name = MACH_PORT_DEAD;
> + dest_name = MACH_PORT_NAME_DEAD;
> else
> - dest_name = MACH_PORT_NULL;
> + dest_name = MACH_PORT_NAME_NULL;
> ip_unlock(reply);
> } else
> - dest_name = MACH_PORT_DEAD;
> + dest_name = MACH_PORT_NAME_DEAD;
> }
>
> if (IP_VALID(reply))
> @@ -2254,7 +2255,7 @@ ipc_kmsg_copyout_object(
> mach_port_name_t *namep)
> {
> if (!IO_VALID(object)) {
> - *namep = (mach_port_name_t) object;
> + *namep = invalid_port_to_name((mach_port_t)object);
> return MACH_MSG_SUCCESS;
> }
>
> @@ -2324,9 +2325,9 @@ ipc_kmsg_copyout_object(
> ipc_object_destroy(object, msgt_name);
>
> if (kr == KERN_INVALID_CAPABILITY)
> - *namep = MACH_PORT_DEAD;
> + *namep = MACH_PORT_NAME_DEAD;
> else {
> - *namep = MACH_PORT_NULL;
> + *namep = MACH_PORT_NAME_NULL;
>
> if (kr == KERN_RESOURCE_SHORTAGE)
> return MACH_MSG_IPC_KERNEL;
> @@ -2436,11 +2437,8 @@ ipc_kmsg_copyout_body(
> ipc_object_t object =
> (ipc_object_t) objects[i];
>
> - /* TODO: revisit this for 64 bits since the
> size of
> - * mach_port_name_t is not the same as a
> pointer size.
> - */
> - mr |= ipc_kmsg_copyout_object(space, object,
> - name, (mach_port_name_t
> *)&objects[i]);
> + mr |= ipc_kmsg_copyout_object_to_port(space,
> object,
> + name,
> &objects[i]);
> }
> }
>
> @@ -2629,14 +2627,14 @@ ipc_kmsg_copyout_dest(
> } else {
> io_release(dest);
> io_check_unlock(dest);
> - dest_name = MACH_PORT_DEAD;
> + dest_name = MACH_PORT_NAME_DEAD;
> }
>
> if (IO_VALID(reply)) {
> ipc_object_destroy(reply, reply_type);
> - reply_name = MACH_PORT_NULL;
> + reply_name = MACH_PORT_NAME_NULL;
> } else
> - reply_name = (mach_port_name_t) reply;
> + reply_name = invalid_port_to_name((mach_port_t)reply);
>
> kmsg->ikm_header.msgh_bits = (MACH_MSGH_BITS_OTHER(mbits) |
> MACH_MSGH_BITS(reply_type, dest_type));
> diff --git a/ipc/ipc_kmsg.h b/ipc/ipc_kmsg.h
> index 16df31f5..b1eb06c7 100644
> --- a/ipc/ipc_kmsg.h
> +++ b/ipc/ipc_kmsg.h
> @@ -269,6 +269,17 @@ extern mach_msg_return_t
> ipc_kmsg_copyout_object(ipc_space_t, ipc_object_t,
> mach_msg_type_name_t, mach_port_name_t *);
>
> +static inline mach_msg_return_t
> +ipc_kmsg_copyout_object_to_port(ipc_space_t space, ipc_object_t object,
> + mach_msg_type_name_t msgt_name, mach_port_t
> *portp)
> +{
> + mach_port_name_t name;;
> + mach_msg_return_t mr;
> + mr = ipc_kmsg_copyout_object(space, object, msgt_name, &name);
> + *portp = (mach_port_t)name;
> + return mr;
> +}
> +
> extern mach_msg_return_t
> ipc_kmsg_copyout_body(ipc_kmsg_t, ipc_space_t, vm_map_t);
>
> diff --git a/ipc/ipc_marequest.c b/ipc/ipc_marequest.c
> index 526e4722..c096fe24 100644
> --- a/ipc/ipc_marequest.c
> +++ b/ipc/ipc_marequest.c
> @@ -279,7 +279,7 @@ ipc_marequest_cancel(
> *last = marequest->imar_next;
> imarb_unlock(bucket);
>
> - marequest->imar_name = MACH_PORT_NULL;
> + marequest->imar_name = MACH_PORT_NAME_NULL;
> }
>
> /*
> @@ -377,7 +377,7 @@ ipc_marequest_destroy(ipc_marequest_t marequest)
> entry->ie_bits &= ~IE_BITS_MAREQUEST;
>
> } else
> - name = MACH_PORT_NULL;
> + name = MACH_PORT_NAME_NULL;
> }
>
> is_write_unlock(space);
> diff --git a/ipc/ipc_object.c b/ipc/ipc_object.c
> index 3fd7f92b..1074fb2c 100644
> --- a/ipc/ipc_object.c
> +++ b/ipc/ipc_object.c
> @@ -804,7 +804,7 @@ ipc_object_copyout_dest(
> if (port->ip_receiver == space)
> name = port->ip_receiver_name;
> else
> - name = MACH_PORT_NULL;
> + name = MACH_PORT_NAME_NULL;
>
> ip_unlock(port);
>
> @@ -839,7 +839,7 @@ ipc_object_copyout_dest(
> ip_unlock(port);
>
> ipc_notify_send_once(port);
> - name = MACH_PORT_NULL;
> + name = MACH_PORT_NAME_NULL;
> }
>
> break;
> diff --git a/ipc/ipc_port.c b/ipc/ipc_port.c
> index c593e8b3..be6e06ac 100644
> --- a/ipc/ipc_port.c
> +++ b/ipc/ipc_port.c
> @@ -1024,12 +1024,12 @@ ipc_port_copyout_send(
> ipc_port_release_send(sright);
>
> if (kr == KERN_INVALID_CAPABILITY)
> - name = MACH_PORT_DEAD;
> + name = MACH_PORT_NAME_DEAD;
> else
> - name = MACH_PORT_NULL;
> + name = MACH_PORT_NAME_NULL;
> }
> } else
> - name = (mach_port_name_t) sright;
> + name = invalid_port_to_name((mach_port_t)sright);
>
> return name;
> }
> diff --git a/ipc/ipc_space.h b/ipc/ipc_space.h
> index 84923e7a..b4eb5ba6 100644
> --- a/ipc/ipc_space.h
> +++ b/ipc/ipc_space.h
> @@ -208,7 +208,7 @@ ipc_entry_get(
> * (See comment in ipc/ipc_table.h.)
> */
>
> - assert(MACH_PORT_VALID(new_name));
> + assert(MACH_PORT_NAME_VALID(new_name));
> assert(free_entry->ie_object == IO_NULL);
>
> space->is_size += 1;
> diff --git a/ipc/mach_msg.c b/ipc/mach_msg.c
> index 1a6d67d0..46218bb6 100644
> --- a/ipc/mach_msg.c
> +++ b/ipc/mach_msg.c
> @@ -1002,7 +1002,7 @@ mach_msg_trap(
> entry->ie_bits = gen | (MACH_PORT_TYPE_SEND_ONCE | 1);
> }
>
> - assert(MACH_PORT_VALID(reply_name));
> + assert(MACH_PORT_NAME_VALID(reply_name));
> entry->ie_object = (ipc_object_t) reply_port;
> is_write_unlock(space);
> }
> diff --git a/ipc/mach_port.c b/ipc/mach_port.c
> index 6edf9f88..67713a50 100644
> --- a/ipc/mach_port.c
> +++ b/ipc/mach_port.c
> @@ -374,7 +374,7 @@ mach_port_rename(
> if (space == IS_NULL)
> return KERN_INVALID_TASK;
>
> - if (!MACH_PORT_VALID(nname))
> + if (!MACH_PORT_NAME_VALID(nname))
> return KERN_INVALID_VALUE;
>
> return ipc_object_rename(space, oname, nname);
> @@ -422,7 +422,7 @@ mach_port_allocate_name(
> if (space == IS_NULL)
> return KERN_INVALID_TASK;
>
> - if (!MACH_PORT_VALID(name))
> + if (!MACH_PORT_NAME_VALID(name))
> return KERN_INVALID_VALUE;
>
> switch (right) {
> @@ -548,7 +548,7 @@ mach_port_destroy(
>
> kr = ipc_right_lookup_write(space, name, &entry);
> if (kr != KERN_SUCCESS) {
> - if (MACH_PORT_VALID (name) && space == current_space()) {
> + if (MACH_PORT_NAME_VALID (name) && space == current_space()) {
> printf("task %.*s destroying a bogus port %lu, most
> probably a bug.\n", (int) sizeof current_task()->name, current_task()->name,
> (unsigned long) name);
> if (mach_port_deallocate_debug)
> SoftDebugger("mach_port_deallocate");
> @@ -592,7 +592,7 @@ mach_port_deallocate(
>
> kr = ipc_right_lookup_write(space, name, &entry);
> if (kr != KERN_SUCCESS) {
> - if (MACH_PORT_VALID (name) && space == current_space()) {
> + if (MACH_PORT_NAME_VALID (name) && space == current_space()) {
> printf("task %.*s deallocating a bogus port %lu, most
> probably a bug.\n", (int) sizeof current_task()->name, current_task()->name,
> (unsigned long) name);
> if (mach_port_deallocate_debug)
> SoftDebugger("mach_port_deallocate");
> @@ -714,7 +714,7 @@ mach_port_mod_refs(
>
> kr = ipc_right_lookup_write(space, name, &entry);
> if (kr != KERN_SUCCESS) {
> - if (MACH_PORT_VALID (name) && space == current_space()) {
> + if (MACH_PORT_NAME_VALID (name) && space == current_space()) {
> printf("task %.*s %screasing a bogus port "
> "%u by %d, most probably a bug.\n",
> (int) (sizeof current_task()->name),
> @@ -1228,7 +1228,7 @@ mach_port_insert_right(
> if (space == IS_NULL)
> return KERN_INVALID_TASK;
>
> - if (!MACH_PORT_VALID(name) ||
> + if (!MACH_PORT_NAME_VALID(name) ||
> !MACH_MSG_TYPE_PORT_ANY_RIGHT(polyPoly))
> return KERN_INVALID_VALUE;
>
> @@ -1323,7 +1323,7 @@ mach_port_get_receive_status(
> statusp->mps_seqno = port->ip_seqno;
> imq_unlock(&pset->ips_messages);
> ips_unlock(pset);
> - assert(MACH_PORT_VALID(statusp->mps_pset));
> + assert(MACH_PORT_NAME_VALID(statusp->mps_pset));
> }
> } else {
> no_port_set:
> diff --git a/kern/exception.c b/kern/exception.c
> index 0d8191a7..1014b3ed 100644
> --- a/kern/exception.c
> +++ b/kern/exception.c
> @@ -269,9 +269,9 @@ exception_no_server(void)
> struct mach_exception {
> mach_msg_header_t Head;
> mach_msg_type_t threadType;
> - mach_port_name_t thread;
> + mach_port_t thread;
> mach_msg_type_t taskType;
> - mach_port_name_t task;
> + mach_port_t task;
> mach_msg_type_t exceptionType;
> integer_t exception;
> mach_msg_type_t codeType;
> @@ -658,10 +658,10 @@ exception_raise(
> * to handle the two ports in the body.
> */
>
> - mr = (ipc_kmsg_copyout_object(space, (ipc_object_t) thread_port,
> - MACH_MSG_TYPE_PORT_SEND, &exc->thread) |
> - ipc_kmsg_copyout_object(space, (ipc_object_t) task_port,
> - MACH_MSG_TYPE_PORT_SEND, &exc->task));
> + mr = (ipc_kmsg_copyout_object_to_port(space, (ipc_object_t) thread_port,
> + MACH_MSG_TYPE_PORT_SEND,
> &exc->thread) |
> + ipc_kmsg_copyout_object_to_port(space, (ipc_object_t) task_port,
> + MACH_MSG_TYPE_PORT_SEND,
> &exc->task));
> if (mr != MACH_MSG_SUCCESS) {
> (void) ipc_kmsg_put(receiver->ith_msg, kmsg,
> kmsg->ikm_header.msgh_size);
> diff --git a/kern/ipc_mig.c b/kern/ipc_mig.c
> index aa433614..a1757da3 100644
> --- a/kern/ipc_mig.c
> +++ b/kern/ipc_mig.c
> @@ -597,7 +597,7 @@ syscall_vm_map(
> if (map == VM_MAP_NULL)
> return MACH_SEND_INTERRUPTED;
>
> - if (MACH_PORT_VALID(memory_object)) {
> + if (MACH_PORT_NAME_VALID(memory_object)) {
> result = ipc_object_copyin(current_space(), memory_object,
> MACH_MSG_TYPE_COPY_SEND,
> (ipc_object_t *) &port);
> @@ -606,7 +606,7 @@ syscall_vm_map(
> return result;
> }
> } else
> - port = (ipc_port_t) memory_object;
> + port = (ipc_port_t)invalid_name_to_port(memory_object);
>
> copyin_address(address, &addr);
> result = vm_map(map, &addr, size, mask, anywhere,
> @@ -683,7 +683,7 @@ kern_return_t syscall_task_create(
> (void) ipc_kmsg_copyout_object(current_space(),
> (ipc_object_t) port,
> MACH_MSG_TYPE_PORT_SEND, &name);
> - copyout_port(&name, child_task);
> + copyout(&name, child_task, sizeof(mach_port_name_t));
> }
> task_deallocate(t);
>
> @@ -733,7 +733,7 @@ kern_return_t syscall_task_set_special_port(
> if (t == TASK_NULL)
> return MACH_SEND_INTERRUPTED;
>
> - if (MACH_PORT_VALID(port_name)) {
> + if (MACH_PORT_NAME_VALID(port_name)) {
> result = ipc_object_copyin(current_space(), port_name,
> MACH_MSG_TYPE_COPY_SEND,
> (ipc_object_t *) &port);
> @@ -742,7 +742,7 @@ kern_return_t syscall_task_set_special_port(
> return result;
> }
> } else
> - port = (ipc_port_t) port_name;
> + port = (ipc_port_t)invalid_name_to_port(port_name);
>
> result = task_set_special_port(t, which_port, port);
> if ((result != KERN_SUCCESS) && IP_VALID(port))
> @@ -769,7 +769,7 @@ syscall_mach_port_allocate(
> kr = mach_port_allocate(space, right, &name);
> if (kr == KERN_SUCCESS)
> {
> - copyout_port(&name, namep);
> + copyout(&name, namep, sizeof(mach_port_name_t));
> }
> is_release(space);
>
> @@ -834,7 +834,7 @@ syscall_mach_port_insert_right(
> return KERN_INVALID_VALUE;
> }
>
> - if (MACH_PORT_VALID(right)) {
> + if (MACH_PORT_NAME_VALID(right)) {
> kr = ipc_object_copyin(current_space(), right, rightType,
> &object);
> if (kr != KERN_SUCCESS) {
> @@ -842,7 +842,7 @@ syscall_mach_port_insert_right(
> return kr;
> }
> } else
> - object = (ipc_object_t) right;
> + object = (ipc_object_t)invalid_name_to_port(right);
> newtype = ipc_object_copyin_type(rightType);
>
> kr = mach_port_insert_right(space, name, (ipc_port_t) object, newtype);
> --
> 2.30.2
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
- [PATCH 0/7] update rpc for x86_64, Luca Dariz, 2023/01/16
- [PATCH 1/7] add msg_user_header_t for user-side msg structure, Luca Dariz, 2023/01/16
- [PATCH 3/7] update syscall signature with rpc_vm_* and mach_port_name_t, Luca Dariz, 2023/01/16
- [PATCH 4/7] update writev syscall signature with rpc types, Luca Dariz, 2023/01/16
- [PATCH 7/7] replace mach_port_t with mach_port_name_t, Luca Dariz, 2023/01/16
- Re: [PATCH 7/7] replace mach_port_t with mach_port_name_t,
Samuel Thibault <=
- [PATCH 5/7] adjust rdxtree key to the correct size, Luca Dariz, 2023/01/16
- [PATCH 2/7] x86_64: expand and shrink messages in copy{in, out}msg routines, Luca Dariz, 2023/01/16
- [PATCH 6/7] add conversion helpers for invalid mach port names, Luca Dariz, 2023/01/16