Re: [PATCH gnumach] Consider protected payloads in mach_msg_header_t when resizing messages.

[Samuel Thibault]

Applied, thanks!

Flavio Cruz, le dim. 12 févr. 2023 13:11:13 -0500, a ecrit:
> Protected payloads will be 8-byte longs which are the same size as
> kernel ports.
> 
> Also aligned all the structures to be 4-byte aligned since it makes it
> easier to parse them as padding won't be added to mach_msg_user_header_t
> before the protected payload.
> ---
>  include/mach/message.h |  9 ++++++++-
>  x86_64/copy_user.c     | 22 ++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/include/mach/message.h b/include/mach/message.h
> index 21c551c2..63341e6f 100644
> --- a/include/mach/message.h
> +++ b/include/mach/message.h
> @@ -132,6 +132,9 @@ 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;
> @@ -151,7 +154,10 @@ typedef  struct {
>      mach_msg_bits_t  msgh_bits;
>      mach_msg_size_t  msgh_size;
>      mach_port_name_t msgh_remote_port;
> -    mach_port_name_t msgh_local_port;
> +    union {
> +        mach_port_name_t     msgh_local_port;
> +        rpc_uintptr_t msgh_protected_payload;
> +    };
>      mach_port_seqno_t        msgh_seqno;
>      mach_msg_id_t    msgh_id;
>  } mach_msg_user_header_t;
> @@ -224,6 +230,7 @@ typedef   struct  {
>      natural_t                msgtl_number;
>  } mach_msg_type_long_t;
>  
> +#pragma pack(pop)
>  
>  /*
>   *   Known values for the msgt_name field.
> diff --git a/x86_64/copy_user.c b/x86_64/copy_user.c
> index b340d6f0..ae062645 100644
> --- a/x86_64/copy_user.c
> +++ b/x86_64/copy_user.c
> @@ -16,6 +16,7 @@
>   * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>   */
>  
> +#include <stddef.h>
>  #include <string.h>
>  
>  #include <kern/debug.h>
> @@ -181,8 +182,21 @@ int copyinmsg (const void *userbuf, void *kernelbuf, 
> const size_t usize)
>    /* kmsg->msgh_size is filled in later */
>    if (copyin_port(&umsg->msgh_remote_port, &kmsg->msgh_remote_port))
>      return 1;
> +#ifdef USER32
> +  /* This could contain a payload, but for 32 bits it will be the same size 
> as a mach_port_name_t */
> +  _Static_assert(sizeof(rpc_uintptr_t) == sizeof(mach_port_name_t),
> +                 "rpc_uintptr_t and mach_port_name_t expected to have the 
> same size");
>    if (copyin_port(&umsg->msgh_local_port, &kmsg->msgh_local_port))
>      return 1;
> +#else
> +  /* For pure 64 bits, the protected payload is as large as a port pointer. 
> */
> +  _Static_assert(sizeof(rpc_uintptr_t) == sizeof(mach_port_t),
> +                 "rpc_uintptr_t and mach_port_t expected to have the same 
> size");
> +  if (copyin((char*)umsg + offsetof(mach_msg_user_header_t, msgh_local_port),
> +              (char*)kmsg + offsetof(mach_msg_header_t, msgh_local_port),
> +           sizeof(rpc_uintptr_t)))
> +    return 1;
> +#endif
>    if (copyin(&umsg->msgh_seqno, &kmsg->msgh_seqno,
>               sizeof(kmsg->msgh_seqno) + sizeof(kmsg->msgh_id)))
>      return 1;
> @@ -275,8 +289,16 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, 
> const size_t ksize)
>    /* umsg->msgh_size is filled in later */
>    if (copyout_port(&kmsg->msgh_remote_port, &umsg->msgh_remote_port))
>      return 1;
> +#ifdef USER32
>    if (copyout_port(&kmsg->msgh_local_port, &umsg->msgh_local_port))
>      return 1;
> +#else
> +  /* Handle protected payloads correctly, same as copyinmsg. */
> +  if (copyout((char*)kmsg + offsetof(mach_msg_header_t, msgh_local_port),
> +              (char*)umsg + offsetof(mach_msg_user_header_t, 
> msgh_local_port),
> +           sizeof(rpc_uintptr_t)))
> +    return 1;
> +#endif
>    if (copyout(&kmsg->msgh_seqno, &umsg->msgh_seqno,
>               sizeof(kmsg->msgh_seqno) + sizeof(kmsg->msgh_id)))
>      return 1;
> -- 
> 2.39.1
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.