bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH gnumach] Update the 64bit RPC ABI to be simpler


From: Flavio Cruz
Subject: [PATCH gnumach] Update the 64bit RPC ABI to be simpler
Date: Tue, 16 May 2023 23:03:46 -0400

* 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. I'm not
  changing any of the code but keeping the same interface using macros.
  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.

Tested with simple programs on pure 64 bit and 64/32 bit combinations.

---
Not too thrilled about the use of the macros for msgtl_name/size/number
but happy to hear about other alternatives. Potentially we could 
introduce accessors and update all the calls to use them.

Note that the follow up MiG patch is necessary to make this work.

 include/mach/message.h |  37 +++++++++++++--
 ipc/ipc_kmsg.c         |   4 ++
 x86_64/copy_user.c     | 100 ++++++++++++++++++++++++++++++++---------
 3 files changed, 115 insertions(+), 26 deletions(-)

diff --git a/include/mach/message.h b/include/mach/message.h
index 0eab9d41..d1783715 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,18 +253,23 @@ 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;
 
+/* On x86_64 this is equivalent to mach_msg_type_t.  */
 typedef        struct  {
     mach_msg_type_t    msgtl_header;
+#ifdef __x86_64__
+/* Since we don't have these separate fields, we remap them
+ * into the fields from mach_msg_type_t. */
+#define msgtl_name msgtl_header.msgt_name
+#define msgtl_size msgtl_header.msgt_size
+#define msgtl_number msgtl_header.msgt_number
+#else
     unsigned short     msgtl_name;
     unsigned short     msgtl_size;
     natural_t          msgtl_number;
+#endif
 } __attribute__ ((aligned (__alignof__ (uintptr_t)))) mach_msg_type_long_t;
 
 /*
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 f76e44c9..af0c8675 100644
--- a/x86_64/copy_user.c
+++ b/x86_64/copy_user.c
@@ -44,9 +44,9 @@ _Static_assert(sizeof(mach_msg_user_type_t) == 4);
 
 typedef struct {
   mach_msg_user_type_t msgtl_header;
-  unsigned short msgtl_name;
-  unsigned short msgtl_size;
-  natural_t msgtl_number;
+  unsigned short user_msgtl_name;
+  unsigned short user_msgtl_size;
+  natural_t user_msgtl_number;
 } mach_msg_user_type_long_t;
 _Static_assert(sizeof(mach_msg_user_type_long_t) == 12);
 #else
@@ -71,9 +71,9 @@ static inline void unpack_msg_type(vm_offset_t addr,
   if (kmt->msgt_longform)
     {
       mach_msg_type_long_t* kmtl = (mach_msg_type_long_t*)addr;
-      *name = kmtl->msgtl_name;
-      *size = kmtl->msgtl_size;
-      *number = kmtl->msgtl_number;
+      *name = kmtl->msgtl_header.msgt_name;
+      *size = kmtl->msgtl_header.msgt_size;
+      *number = kmtl->msgtl_header.msgt_number;
       *kernel_amount = sizeof(mach_msg_type_long_t);
       *user_amount = sizeof(mach_msg_user_type_long_t);
     }
@@ -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->user_msgtl_name,
+      .msgt_size = u->user_msgtl_size,
+      .msgt_number = u->user_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
+    },
+    .user_msgtl_name = k->msgtl_header.msgt_name,
+    .user_msgtl_size = k->msgtl_header.msgt_size,
+    .user_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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]