Hello,
So, is anybody against making this change?
Samuel
Flavio Cruz, le mar. 13 juin 2023 00:01:25 -0400, a ecrit:
> * Make full use of the 8 bytes available in mach_msg_type_t by moving
> into the unused 4 bytes. This allows us to use 32bits for
> mach_msg_type_number_t whether we use the longform or not.
> * Make mach_msg_type_long_t exactly the same as mach_msg_type_t.
> Updating MiG is strongly encouraged since it will generate better code
> to handle this new format.
>
> After this change, any compatibility with compiled binaries for Hurd x86_64
> will break since the message format is different. However, the new
> schema simplifies the overall ABI, without having "holes" and also
> avoids the need to have a 16 byte mach_msg_type_long_t.
>
> Was able to boot a basic system up to a bash shell.
> ---
> include/mach/message.h | 52 ++++++++++++++++++++++---
> ipc/ipc_kmsg.c | 4 ++
> x86_64/copy_user.c | 88 ++++++++++++++++++++++++++++++++++--------
> 3 files changed, 123 insertions(+), 21 deletions(-)
>
> diff --git a/include/mach/message.h b/include/mach/message.h
> index 0eab9d41..2177343a 100644
> --- a/include/mach/message.h
> +++ b/include/mach/message.h
> @@ -222,6 +222,30 @@ typedef unsigned int mach_msg_type_size_t;
> typedef natural_t mach_msg_type_number_t;
>
> typedef struct {
> +#ifdef __x86_64__
> + /*
> + * For 64 bits, this struct is 8 bytes long so we
> + * can pack the same amount of information as mach_msg_type_long_t.
> + * Note that for 64 bit userland, msgt_size only needs to be 8 bits long
> + * but for kernel compatibility with 32 bit userland we allow it to be
> + * 16 bits long.
> + *
> + * Effectively, we don't need mach_msg_type_long_t but we are keeping it
> + * for a while to make the code similar between 32 and 64 bits.
> + *
> + * We also keep the msgt_longform bit around simply because it makes it
> + * very easy to convert messages from a 32 bit userland into a 64 bit
> + * kernel. Otherwise, we would have to replicate some of the MiG logic
> + * internally in the kernel.
> + */
> + unsigned int msgt_inline : 1,
> + msgt_longform : 1,
> + msgt_deallocate : 1,
> + msgt_name : 8,
> + msgt_size : 16,
> + msgt_unused : 5;
> + mach_msg_type_number_t msgt_number;
> +#else
> unsigned int msgt_name : 8,
> msgt_size : 8,
> msgt_number : 12,
> @@ -229,20 +253,38 @@ typedef struct {
> msgt_longform : 1,
> msgt_deallocate : 1,
> msgt_unused : 1;
> -#ifdef __x86_64__
> - /* TODO: We want to eventually use this in favor of mach_msg_type_long_t
> - * as it makes the mach_msg protocol require only mach_msg_type_t. */
> - mach_msg_type_number_t unused_msgtl_number;
> #endif
> } __attribute__ ((aligned (__alignof__ (uintptr_t)))) mach_msg_type_t;
>
> -typedef struct {
> +typedef struct {
> +#ifdef __x86_64__
> + union {
> + /* On x86_64 this is equivalent to mach_msg_type_t so use
> + * union to overlay with the old field names. */
> + mach_msg_type_t msgtl_header;
> + struct {
> + unsigned int msgtl_inline : 1,
> + msgtl_longform : 1,
> + msgtl_deallocate : 1,
> + msgtl_name : 8,
> + msgtl_size : 16,
> + msgtl_unused : 5;
> + mach_msg_type_number_t msgtl_number;
> + };
> + };
> +#else
> mach_msg_type_t msgtl_header;
> unsigned short msgtl_name;
> unsigned short msgtl_size;
> natural_t msgtl_number;
> +#endif
> } __attribute__ ((aligned (__alignof__ (uintptr_t)))) mach_msg_type_long_t;
>
> +#ifdef __x86_64__
> +_Static_assert (sizeof (mach_msg_type_t) == sizeof (mach_msg_type_long_t),
> + "mach_msg_type_t and mach_msg_type_long_t need to have the
> same size.");
> +#endif
> +
> /*
> * Known values for the msgt_name field.
> *
> diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c
> index 1988da45..d1c4675c 100644
> --- a/ipc/ipc_kmsg.c
> +++ b/ipc/ipc_kmsg.c
> @@ -1343,9 +1343,11 @@ ipc_kmsg_copyin_body(
> is_port = MACH_MSG_TYPE_PORT_ANY(name);
>
> if ((is_port && (size != PORT_T_SIZE_IN_BITS)) ||
> +#ifndef __x86_64__
> (longform && ((type->msgtl_header.msgt_name != 0) ||
> (type->msgtl_header.msgt_size != 0) ||
> (type->msgtl_header.msgt_number != 0))) ||
> +#endif
> (((mach_msg_type_t*)type)->msgt_unused != 0) ||
> (dealloc && is_inline)) {
> ipc_kmsg_clean_partial(kmsg, taddr, FALSE, 0);
> @@ -2833,9 +2835,11 @@ ipc_msg_print(mach_msg_header_t *msgh)
> is_port = MACH_MSG_TYPE_PORT_ANY(name);
>
> if ((is_port && (size != PORT_T_SIZE_IN_BITS)) ||
> +#ifndef __x86_64__
> (longform && ((type->msgtl_header.msgt_name != 0) ||
> (type->msgtl_header.msgt_size != 0) ||
> (type->msgtl_header.msgt_number != 0))) ||
> +#endif
> (((mach_msg_type_t*)type)->msgt_unused != 0) ||
> (dealloc && is_inline)) {
> db_printf("*** invalid type\n");
> diff --git a/x86_64/copy_user.c b/x86_64/copy_user.c
> index 6ff50e12..548d9d7d 100644
> --- a/x86_64/copy_user.c
> +++ b/x86_64/copy_user.c
> @@ -87,12 +87,74 @@ static inline void unpack_msg_type(vm_offset_t addr,
> }
> }
>
> +#ifdef USER32
> +static inline void mach_msg_user_type_to_kernel(const mach_msg_user_type_t
> *u,
> + mach_msg_type_t* k) {
> + k->msgt_name = u->msgt_name;
> + k->msgt_size = u->msgt_size;
> + k->msgt_number = u->msgt_number;
> + k->msgt_inline = u->msgt_inline;
> + k->msgt_longform = u->msgt_longform;
> + k->msgt_deallocate = u->msgt_deallocate;
> + k->msgt_unused = 0;
> +}
> +
> +static inline void mach_msg_user_type_to_kernel_long(const
> mach_msg_user_type_long_t *u,
> + mach_msg_type_long_t* k) {
> + const mach_msg_type_long_t kernel = {
> + .msgtl_header = {
> + .msgt_name = u->msgtl_name,
> + .msgt_size = u->msgtl_size,
> + .msgt_number = u->msgtl_number,
> + .msgt_inline = u->msgtl_header.msgt_inline,
> + .msgt_longform = u->msgtl_header.msgt_longform,
> + .msgt_deallocate = u->msgtl_header.msgt_deallocate,
> + .msgt_unused = 0
> + }
> + };
> + *k = kernel;
> +}
> +
> +static inline void mach_msg_kernel_type_to_user(const mach_msg_type_t *k,
> + mach_msg_user_type_t *u) {
> + u->msgt_name = k->msgt_name;
> + u->msgt_size = k->msgt_size;
> + u->msgt_number = k->msgt_number;
> + u->msgt_inline = k->msgt_inline;
> + u->msgt_longform = k->msgt_longform;
> + u->msgt_deallocate = k->msgt_deallocate;
> + u->msgt_unused = 0;
> +}
> +
> +static inline void mach_msg_kernel_type_to_user_long(const
> mach_msg_type_long_t *k,
> + mach_msg_user_type_long_t *u) {
> + const mach_msg_user_type_long_t user = {
> + .msgtl_header = {
> + .msgt_name = 0,
> + .msgt_size = 0,
> + .msgt_number = 0,
> + .msgt_inline = k->msgtl_header.msgt_inline,
> + .msgt_longform = k->msgtl_header.msgt_longform,
> + .msgt_deallocate = k->msgtl_header.msgt_deallocate,
> + .msgt_unused = 0
> + },
> + .msgtl_name = k->msgtl_header.msgt_name,
> + .msgtl_size = k->msgtl_header.msgt_size,
> + .msgtl_number = k->msgtl_header.msgt_number
> + };
> + *u = user;
> +}
> +#endif
> +
> static inline int copyin_mach_msg_type(const rpc_vm_offset_t *uaddr,
> mach_msg_type_t *kaddr) {
> #ifdef USER32
> - int ret;
> - ret = copyin(uaddr, kaddr, sizeof(mach_msg_user_type_t));
> - kaddr->unused_msgtl_number = 0;
> - return ret;
> + mach_msg_user_type_t user;
> + int ret = copyin(uaddr, &user, sizeof(mach_msg_user_type_t));
> + if (ret) {
> + return ret;
> + }
> + mach_msg_user_type_to_kernel(&user, kaddr);
> + return 0;
> #else
> return copyin(uaddr, kaddr, sizeof(mach_msg_type_t));
> #endif
> @@ -100,7 +162,9 @@ static inline int copyin_mach_msg_type(const
> rpc_vm_offset_t *uaddr, mach_msg_ty
>
> static inline int copyout_mach_msg_type(const mach_msg_type_t *kaddr,
> rpc_vm_offset_t *uaddr) {
> #ifdef USER32
> - return copyout(kaddr, uaddr, sizeof(mach_msg_user_type_t));
> + mach_msg_user_type_t user;
> + mach_msg_kernel_type_to_user(kaddr, &user);
> + return copyout(&user, uaddr, sizeof(mach_msg_user_type_t));
> #else
> return copyout(kaddr, uaddr, sizeof(mach_msg_type_t));
> #endif
> @@ -109,15 +173,10 @@ static inline int copyout_mach_msg_type(const
> mach_msg_type_t *kaddr, rpc_vm_off
> static inline int copyin_mach_msg_type_long(const rpc_vm_offset_t *uaddr,
> mach_msg_type_long_t *kaddr) {
> #ifdef USER32
> mach_msg_user_type_long_t user;
> - int ret;
> - ret = copyin(uaddr, &user, sizeof(mach_msg_user_type_long_t));
> + int ret = copyin(uaddr, &user, sizeof(mach_msg_user_type_long_t));
> if (ret)
> return ret;
> - /* The first 4 bytes are laid out in the same way. */
> - memcpy(&kaddr->msgtl_header, &user.msgtl_header,
> sizeof(mach_msg_user_type_t));
> - kaddr->msgtl_name = user.msgtl_name;
> - kaddr->msgtl_size = user.msgtl_size;
> - kaddr->msgtl_number = user.msgtl_number;
> + mach_msg_user_type_to_kernel_long(&user, kaddr);
> return 0;
> #else
> return copyin(uaddr, kaddr, sizeof(mach_msg_type_long_t));
> @@ -127,10 +186,7 @@ static inline int copyin_mach_msg_type_long(const
> rpc_vm_offset_t *uaddr, mach_m
> static inline int copyout_mach_msg_type_long(const mach_msg_type_long_t
> *kaddr, rpc_vm_offset_t *uaddr) {
> #ifdef USER32
> mach_msg_user_type_long_t user;
> - memcpy(&user.msgtl_header, &kaddr->msgtl_header,
> sizeof(mach_msg_user_type_t));
> - user.msgtl_name = kaddr->msgtl_name;
> - user.msgtl_size = kaddr->msgtl_size;
> - user.msgtl_number = kaddr->msgtl_number;
> + mach_msg_kernel_type_to_user_long(kaddr, &user);
> return copyout(&user, uaddr, sizeof(mach_msg_user_type_long_t));
> #else
> return copyout(kaddr, uaddr, sizeof(mach_msg_type_long_t));
> --
> 2.39.2
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
