[PATCH gnumach] Align mach_msg_type_t and mach_msg_type_long_t with the same alignment as uintptr_t.

[Flavio Cruz]

With this change, any 64 bit code using the IPC subsystem without relying on
MiG will work without any changes. We have a few examples of this inside 
gnumach but
also in the Hurd servers. For example, in hurd/console/display.c

  typedef struct
  {
    mach_msg_header_t Head;
    mach_msg_type_t ticknoType;
    natural_t tickno;
    mach_msg_type_t changeType;
    file_changed_type_t change;
    mach_msg_type_t startType;
    loff_t start;
    mach_msg_type_t endType;
    loff_t end;
  } Request;

This will now work correctly in 64 bits, without requiring any explicit padding.

As a follow up, we can simplify mach_msg_type_long_t so that we
only need an 8 byte structure where the second field will include the
number of elements for the long form. This is already included in
mach_msg_type_t as unused_msgtl_number.
---
 i386/i386/copy_user.h  |   4 +-
 include/mach/message.h |  14 ++--
 x86_64/copy_user.c     | 186 ++++++++++++++++++++++++++++++++---------
 3 files changed, 156 insertions(+), 48 deletions(-)

diff --git a/i386/i386/copy_user.h b/i386/i386/copy_user.h
index c7e8b4e1..5cdbfa80 100644
--- a/i386/i386/copy_user.h
+++ b/i386/i386/copy_user.h
@@ -71,7 +71,7 @@ static inline int copyout_address(const vm_offset_t *kaddr, 
rpc_vm_offset_t *uad
 
 static inline int copyin_port(const mach_port_name_t *uaddr, mach_port_t 
*kaddr)
 {
-#ifdef __x86_64
+#ifdef __x86_64__
   return copyin_32to64(uaddr, kaddr);
 #else /* __x86_64__ */
   return copyin(uaddr, kaddr, sizeof(*uaddr));
@@ -80,7 +80,7 @@ static inline int copyin_port(const mach_port_name_t *uaddr, 
mach_port_t *kaddr)
 
 static inline int copyout_port(const mach_port_t *kaddr, mach_port_name_t 
*uaddr)
 {
-#ifdef __x86_64
+#ifdef __x86_64__
   return copyout_64to32(kaddr, uaddr);
 #else /* __x86_64__ */
   return copyout(kaddr, uaddr, sizeof(*kaddr));
diff --git a/include/mach/message.h b/include/mach/message.h
index 22a17b03..0eab9d41 100644
--- a/include/mach/message.h
+++ b/include/mach/message.h
@@ -132,9 +132,6 @@ typedef     unsigned int mach_msg_size_t;
 typedef natural_t mach_msg_seqno_t;
 typedef integer_t mach_msg_id_t;
 
-/* Force 4-byte alignment to simplify how the kernel parses the messages */
-#pragma pack(push, 4)
-
 /* full header structure, may have different size in user/kernel spaces */
 typedef        struct mach_msg_header {
     mach_msg_bits_t    msgh_bits;
@@ -232,16 +229,19 @@ typedef struct  {
                        msgt_longform : 1,
                        msgt_deallocate : 1,
                        msgt_unused : 1;
-} mach_msg_type_t;
+#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  {
     mach_msg_type_t    msgtl_header;
     unsigned short     msgtl_name;
     unsigned short     msgtl_size;
     natural_t          msgtl_number;
-} mach_msg_type_long_t;
-
-#pragma pack(pop)
+} __attribute__ ((aligned (__alignof__ (uintptr_t)))) mach_msg_type_long_t;
 
 /*
  *     Known values for the msgt_name field.
diff --git a/x86_64/copy_user.c b/x86_64/copy_user.c
index e429f259..b5084996 100644
--- a/x86_64/copy_user.c
+++ b/x86_64/copy_user.c
@@ -29,16 +29,42 @@
 #define descsize_to_bytes(n) (n / 8)
 #define bytes_to_descsize(n) (n * 8)
 
+#ifdef USER32
+/* Versions of mach_msg_type_t and mach_msg_type_long that are expected from 
the 32 bit userland. */
+typedef struct {
+  unsigned int msgt_name : 8,
+               msgt_size : 8,
+               msgt_number : 12,
+               msgt_inline : 1,
+               msgt_longform : 1,
+               msgt_deallocate : 1,
+               msgt_unused : 1;
+} mach_msg_user_type_t;
+_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;
+} mach_msg_user_type_long_t;
+_Static_assert(sizeof(mach_msg_user_type_long_t) == 12);
+#else
+typedef mach_msg_type_t mach_msg_user_type_t;
+typedef mach_msg_type_long_t mach_msg_user_type_long_t;
+#endif  /* USER32 */
 
 /*
 * Helper to unpack the relevant fields of a msg type; the fields are different
 * depending on whether is long form or not.
 .*/
-static inline vm_size_t unpack_msg_type(vm_offset_t addr,
-                                        mach_msg_type_name_t *name,
-                                        mach_msg_type_size_t *size,
-                                        mach_msg_type_number_t *number,
-                                        boolean_t *is_inline)
+static inline void unpack_msg_type(vm_offset_t addr,
+                                   mach_msg_type_name_t *name,
+                                   mach_msg_type_size_t *size,
+                                   mach_msg_type_number_t *number,
+                                   boolean_t *is_inline,
+                                   vm_size_t *user_amount,
+                                   vm_size_t *kernel_amount)
 {
   mach_msg_type_t* kmt = (mach_msg_type_t*)addr;
   *is_inline = kmt->msgt_inline;
@@ -48,17 +74,69 @@ static inline vm_size_t unpack_msg_type(vm_offset_t addr,
       *name = kmtl->msgtl_name;
       *size = kmtl->msgtl_size;
       *number = kmtl->msgtl_number;
-      return sizeof(mach_msg_type_long_t);
+      *kernel_amount = sizeof(mach_msg_type_long_t);
+      *user_amount = sizeof(mach_msg_user_type_long_t);
     }
   else
     {
       *name = kmt->msgt_name;
       *size = kmt->msgt_size;
       *number = kmt->msgt_number;
-      return sizeof(mach_msg_type_t);
+      *kernel_amount = sizeof(mach_msg_type_t);
+      *user_amount = sizeof(mach_msg_user_type_t);
     }
 }
 
+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;
+#else
+  return copyin(uaddr, kaddr, sizeof(mach_msg_type_t));
+#endif
+}
+
+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));
+#else
+  return copyout(kaddr, uaddr, sizeof(mach_msg_type_t));
+#endif
+}
+
+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));
+  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;
+  return 0;
+#else
+  return copyin(uaddr, kaddr, sizeof(mach_msg_type_long_t));
+#endif
+}
+
+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;
+  return copyout(&user, uaddr, sizeof(mach_msg_user_type_long_t));
+#else
+  return copyout(kaddr, uaddr, sizeof(mach_msg_type_long_t));
+#endif
+}
+
 /* Optimized version of unpack_msg_type(), including proper copyin() */
 static inline int copyin_unpack_msg_type(vm_offset_t uaddr,
                                          vm_offset_t kaddr,
@@ -66,28 +144,31 @@ static inline int copyin_unpack_msg_type(vm_offset_t uaddr,
                                          mach_msg_type_size_t *size,
                                          mach_msg_type_number_t *number,
                                          boolean_t *is_inline,
-                                         vm_size_t *amount)
+                                         vm_size_t *user_amount,
+                                         vm_size_t *kernel_amount)
 {
   mach_msg_type_t *kmt = (mach_msg_type_t*)kaddr;
-  if (copyin((void*)uaddr, kmt, sizeof(mach_msg_type_t)))
+  if (copyin_mach_msg_type((void *)uaddr, kmt))
     return 1;
   *is_inline = kmt->msgt_inline;
   if (kmt->msgt_longform)
     {
       mach_msg_type_long_t* kmtl = (mach_msg_type_long_t*)kaddr;
-      if (copyin((void*)uaddr, kmtl, sizeof(mach_msg_type_long_t)))
+      if (copyin_mach_msg_type_long((void *)uaddr, kmtl))
         return 1;
       *name = kmtl->msgtl_name;
       *size = kmtl->msgtl_size;
       *number = kmtl->msgtl_number;
-      *amount = sizeof(mach_msg_type_long_t);
+      *user_amount = sizeof(mach_msg_user_type_long_t);
+      *kernel_amount = sizeof(mach_msg_type_long_t);
     }
   else
     {
       *name = kmt->msgt_name;
       *size = kmt->msgt_size;
       *number = kmt->msgt_number;
-      *amount = sizeof(mach_msg_type_t);
+      *user_amount = sizeof(mach_msg_user_type_t);
+      *kernel_amount = sizeof(mach_msg_type_t);
     }
   return 0;
 }
@@ -110,6 +191,46 @@ static inline void adjust_msg_type_size(vm_offset_t addr, 
int amount)
     }
 }
 
+/* Optimized version of unpack_msg_type(), including proper copyout() */
+static inline int copyout_unpack_msg_type(vm_offset_t kaddr,
+                                          vm_offset_t uaddr,
+                                          mach_msg_type_name_t *name,
+                                          mach_msg_type_size_t *size,
+                                          mach_msg_type_number_t *number,
+                                          boolean_t *is_inline,
+                                          vm_size_t *user_amount,
+                                          vm_size_t *kernel_amount)
+{
+  mach_msg_type_t *kmt = (mach_msg_type_t*)kaddr;
+  *is_inline = kmt->msgt_inline;
+  if (kmt->msgt_longform)
+    {
+      mach_msg_type_long_t* kmtl = (mach_msg_type_long_t*)kaddr;
+      if (MACH_MSG_TYPE_PORT_ANY(kmtl->msgtl_name))
+        kmtl->msgtl_size = bytes_to_descsize(sizeof(mach_port_name_t));
+      if (copyout_mach_msg_type_long(kmtl, (void*)uaddr))
+        return 1;
+      *name = kmtl->msgtl_name;
+      *size = kmtl->msgtl_size;
+      *number = kmtl->msgtl_number;
+      *user_amount = sizeof(mach_msg_user_type_long_t);
+      *kernel_amount = sizeof(mach_msg_type_long_t);
+    }
+  else
+    {
+      if (MACH_MSG_TYPE_PORT_ANY(kmt->msgt_name))
+        kmt->msgt_size = bytes_to_descsize(sizeof(mach_port_name_t));
+      if (copyout_mach_msg_type(kmt, (void *)uaddr))
+        return 1;
+      *name = kmt->msgt_name;
+      *size = kmt->msgt_size;
+      *number = kmt->msgt_number;
+      *user_amount = sizeof(mach_msg_user_type_t);
+      *kernel_amount = sizeof(mach_msg_type_t);
+    }
+  return 0;
+}
+
 /*
  * Compute the user-space size of a message still in the kernel.
  * The message may be originating from userspace (in which case we could
@@ -129,15 +250,15 @@ size_t msg_usize(const mach_msg_header_t *kmsg)
       eaddr = saddr + ksize - sizeof(mach_msg_header_t);
       while (saddr < (eaddr - sizeof(mach_msg_type_t)))
         {
-          vm_size_t amount;
+          vm_size_t user_amount, kernel_amount;
           mach_msg_type_name_t name;
           mach_msg_type_size_t size;
           mach_msg_type_number_t number;
           boolean_t is_inline;
-          amount = unpack_msg_type(saddr, &name, &size, &number, &is_inline);
-          saddr += amount;
+          unpack_msg_type(saddr, &name, &size, &number, &is_inline, 
&user_amount, &kernel_amount);
+          saddr += kernel_amount;
           saddr = mach_msg_kernel_align(saddr);
-          usize += amount;
+          usize += user_amount;
           usize = mach_msg_user_align(usize);
 
           if (is_inline)
@@ -211,23 +332,23 @@ int copyinmsg (const void *userbuf, void *kernelbuf, 
const size_t usize)
   if (usize > sizeof(mach_msg_user_header_t))
     {
       /* check we have at least space for an empty descryptor */
-      while (usaddr < (ueaddr - sizeof(mach_msg_type_t)))
+      while (usaddr < (ueaddr - sizeof(mach_msg_user_type_t)))
         {
-          vm_size_t amount;
+          vm_size_t user_amount, kernel_amount;
           mach_msg_type_name_t name;
           mach_msg_type_size_t size;
           mach_msg_type_number_t number;
           boolean_t is_inline;
           if (copyin_unpack_msg_type(usaddr, ksaddr, &name, &size, &number,
-                                     &is_inline, &amount))
+                                     &is_inline, &user_amount, &kernel_amount))
             return 1;
 
           // keep a reference to the current field descriptor, we
           // might need to adjust it later depending on the type
           vm_offset_t ktaddr = ksaddr;
-          usaddr += amount;
+          usaddr += user_amount;
           usaddr = mach_msg_user_align(usaddr);
-          ksaddr += amount;
+          ksaddr += kernel_amount;
           ksaddr = mach_msg_kernel_align(ksaddr);
 
           if (is_inline)
@@ -313,28 +434,23 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, 
const size_t ksize)
     {
       while (ksaddr < keaddr)
         {
-          vm_size_t amount;
+          vm_size_t user_amount, kernel_amount;
           mach_msg_type_name_t name;
           mach_msg_type_size_t size;
           mach_msg_type_number_t number;
           boolean_t is_inline;
-          amount = unpack_msg_type(ksaddr, &name, &size, &number, &is_inline);
-          // TODO: optimize and bring here type adjustment??
-          vm_offset_t utaddr=usaddr, ktaddr=ksaddr;
-          if (copyout((void*)ksaddr, (void*)usaddr, amount))
+          if (copyout_unpack_msg_type(ksaddr, usaddr, &name, &size, &number,
+                                      &is_inline, &user_amount, 
&kernel_amount))
             return 1;
-          usaddr += amount;
+          usaddr += user_amount;
           usaddr = mach_msg_user_align(usaddr);
-          ksaddr += amount;
+          ksaddr += kernel_amount;
           ksaddr = mach_msg_kernel_align(ksaddr);
 
           if (is_inline)
             {
               if (MACH_MSG_TYPE_PORT_ANY(name))
                 {
-                  adjust_msg_type_size(ktaddr, (int)sizeof(mach_port_name_t) - 
(int)sizeof(mach_port_t));
-                  if (copyout((void*)ktaddr, (void*)utaddr, amount))
-                    return 1;
                   for (int i=0; i<number; i++)
                     {
                       if (copyout_port((mach_port_t*)ksaddr, 
(mach_port_name_t*)usaddr))
@@ -355,14 +471,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, 
const size_t ksize)
             }
           else
             {
-              // out-of-line port arrays are shrinked in 
ipc_kmsg_copyout_body()
-              if (MACH_MSG_TYPE_PORT_ANY(name))
-                {
-                  adjust_msg_type_size(ktaddr, -4);
-                  if (copyout((void*)ktaddr, (void*)utaddr, amount))
-                    return 1;
-                }
-
               if (copyout_address((vm_offset_t*)ksaddr, 
(rpc_vm_offset_t*)usaddr))
                 return 1;
               // advance one pointer
-- 
2.39.2