bug-hurd
[Top][All Lists]
Advanced

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

Re: [PATCH gnumach 1/2] Add MACH_MSG_TYPE_COPY_SEND_ONCE


From: Samuel Thibault
Subject: Re: [PATCH gnumach 1/2] Add MACH_MSG_TYPE_COPY_SEND_ONCE
Date: Mon, 1 Apr 2024 14:23:50 +0200
User-agent: NeoMutt/20170609 (1.8.3)

Applied, thanks!

Samuel

Sergey Bugaev, le lun. 01 avril 2024 12:00:17 +0300, a ecrit:
> Mach allows messages to carry port rights along with data; the rights
> contained in a message are transferred from the sender's IPC space to
> the receiver.  The sender has to specify exactly how a right that the
> message will carry is to be created from the right(s) that the sender
> holds: in particular, a send right can be made (created) from a receive
> right, copied from another send right, or a send right can be *moved*,
> in which case the sender loses its right.
> 
> Send-once rights, new in Mach 3, similarly can be made from a receive
> right, or moved.  However, it would appear that the original Mach
> designers forgot to implement the functionality of *copying* a send-once
> right when sending it in a message.
> 
> This patch fixes the oversight by adding the missing
> MACH_MSG_TYPE_COPY_SEND_ONCE (aka copy-sonce) message type.  It behaves
> very similar to MACH_MSG_TYPE_COPY_SEND: the sender must supply an
> existing send-once right, so count of send-once references to the port
> (ip_sorights) is incremented, and the new send-once right gets carried
> in the message.  The receiver sees it as MACH_MSG_TYPE_PORT_SEND_ONCE
> (equal to MACH_MSG_TYPE_MOVE_SEND_ONCE), it cannot tell (and should not
> care) whether the sender has used make-sonce, move-sonce, or copy-sonce.
> It is also possible to supply a dead name when using copy-sonce (no
> matter whether the name originally denoted a send or send-once right);
> in this case the receiver will get MACH_PORT_DEAD instead of any right,
> and the sender keeps its dead name reference.
> 
> This patch also adds the mach_port_copy_send_once_t MIG type, which can
> be used in routine definitions.  A corresponding MIG patch is required
> to make MIG recognise MACH_MSG_TYPE_COPY_SEND_ONCE as a valid type.  To
> bootstrap support for MACH_MSG_TYPE_COPY_SEND_ONCE in Mach and MIG, you
> should run 'make install-data' in Mach first, then build and install MIG
> with make-sonce support, the build Mach.
> 
> Note that the other potential COPY type, MACH_MSG_TYPE_COPY_RECEIVE,
> makes no sense for obvious reasons: there can only ever be one receive
> right for a port, and Mach relies on this being true in a number of
> places (e.g. ip_receiver/ip_receiver_name).
> 
> Note also that fully supporting MACH_MSG_TYPE_COPY_SEND_ONCE in Mach is
> required to take advantage of hardware port translation acceleration on
> AArch64 CPUs that implement the FEAT_AFD extension: the MPTTR_EL1 (Mach
> Port Translation Type Register) system register must hold a value of
> port translation type, which can include MACH_MSG_TYPE_COPY_SEND_ONCE.
> ---
>  doc/mach.texi               |  1 +
>  include/mach/mach_port.defs |  1 +
>  include/mach/message.h      | 18 +++++++----
>  include/mach/std_types.defs |  2 ++
>  ipc/ipc_kmsg.c              |  3 ++
>  ipc/ipc_object.c            | 21 +++++++++++-
>  ipc/ipc_right.c             | 64 +++++++++++++++++++++++++++++++++----
>  7 files changed, 95 insertions(+), 15 deletions(-)
> 
> diff --git a/doc/mach.texi b/doc/mach.texi
> index f85288e0..317a5930 100644
> --- a/doc/mach.texi
> +++ b/doc/mach.texi
> @@ -1445,6 +1445,7 @@ should be used in preference to 
> @code{MACH_MSG_TYPE_INTEGER_32}.
>  @item MACH_MSG_TYPE_COPY_SEND
>  @item MACH_MSG_TYPE_MAKE_SEND
>  @item MACH_MSG_TYPE_MAKE_SEND_ONCE
> +@item MACH_MSG_TYPE_COPY_SEND_ONCE
>  @end table
>  
>  The type @code{MACH_MSG_TYPE_PROTECTED_PAYLOAD} is used by the kernel
> diff --git a/include/mach/mach_port.defs b/include/mach/mach_port.defs
> index 3823bb14..c291a268 100644
> --- a/include/mach/mach_port.defs
> +++ b/include/mach/mach_port.defs
> @@ -277,6 +277,7 @@ routine mach_port_insert_right(
>   *           MACH_MSG_TYPE_MOVE_SEND
>   *           MACH_MSG_TYPE_MAKE_SEND_ONCE
>   *           MACH_MSG_TYPE_MOVE_SEND_ONCE
> + *           MACH_MSG_TYPE_COPY_SEND_ONCE
>   */
>  
>  routine mach_port_extract_right(
> diff --git a/include/mach/message.h b/include/mach/message.h
> index 9790ef98..aa068cab 100644
> --- a/include/mach/message.h
> +++ b/include/mach/message.h
> @@ -357,8 +357,9 @@ _Static_assert (sizeof (mach_msg_type_t) == sizeof 
> (mach_msg_type_long_t),
>  #define MACH_MSG_TYPE_PORT_SEND_ONCE MACH_MSG_TYPE_MOVE_SEND_ONCE
>  
>  #define MACH_MSG_TYPE_PROTECTED_PAYLOAD      23
> +#define MACH_MSG_TYPE_COPY_SEND_ONCE 24      /* Must hold send-once rights */
>  
> -#define MACH_MSG_TYPE_LAST           23              /* Last assigned */
> +#define MACH_MSG_TYPE_LAST           24      /* Last assigned */
>  
>  /*
>   *  A dummy value.  Mostly used to indicate that the actual value
> @@ -372,16 +373,19 @@ _Static_assert (sizeof (mach_msg_type_t) == sizeof 
> (mach_msg_type_long_t),
>   */
>  
>  #define MACH_MSG_TYPE_PORT_ANY(x)                    \
> -     (((x) >= MACH_MSG_TYPE_MOVE_RECEIVE) &&         \
> -      ((x) <= MACH_MSG_TYPE_MAKE_SEND_ONCE))
> +     ((((x) >= MACH_MSG_TYPE_MOVE_RECEIVE) &&        \
> +      ((x) <= MACH_MSG_TYPE_MAKE_SEND_ONCE)) ||      \
> +      ((x) == MACH_MSG_TYPE_COPY_SEND_ONCE))
>  
>  #define      MACH_MSG_TYPE_PORT_ANY_SEND(x)                  \
> -     (((x) >= MACH_MSG_TYPE_MOVE_SEND) &&            \
> -      ((x) <= MACH_MSG_TYPE_MAKE_SEND_ONCE))
> +     ((((x) >= MACH_MSG_TYPE_MOVE_SEND) &&           \
> +      ((x) <= MACH_MSG_TYPE_MAKE_SEND_ONCE)) ||      \
> +      ((x) == MACH_MSG_TYPE_COPY_SEND_ONCE))
>  
>  #define      MACH_MSG_TYPE_PORT_ANY_RIGHT(x)                 \
> -     (((x) >= MACH_MSG_TYPE_MOVE_RECEIVE) &&         \
> -      ((x) <= MACH_MSG_TYPE_MOVE_SEND_ONCE))
> +     ((((x) >= MACH_MSG_TYPE_MOVE_RECEIVE) &&        \
> +      ((x) <= MACH_MSG_TYPE_MOVE_SEND_ONCE)) ||      \
> +      ((x) == MACH_MSG_TYPE_COPY_SEND_ONCE))
>  
>  typedef integer_t mach_msg_option_t;
>  
> diff --git a/include/mach/std_types.defs b/include/mach/std_types.defs
> index b461f062..a687bdb5 100644
> --- a/include/mach/std_types.defs
> +++ b/include/mach/std_types.defs
> @@ -85,6 +85,8 @@ type mach_port_make_send_once_t =   
> MACH_MSG_TYPE_MAKE_SEND_ONCE
>       ctype: mach_port_t;
>  type mach_port_move_send_once_t =    MACH_MSG_TYPE_MOVE_SEND_ONCE
>       ctype: mach_port_t;
> +type mach_port_copy_send_once_t =    MACH_MSG_TYPE_COPY_SEND_ONCE
> +     ctype: mach_port_t;
>  
>  type mach_port_receive_t =           MACH_MSG_TYPE_PORT_RECEIVE
>       ctype: mach_port_t;
> diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c
> index 179c43fa..6109fcb8 100644
> --- a/ipc/ipc_kmsg.c
> +++ b/ipc/ipc_kmsg.c
> @@ -2731,6 +2731,9 @@ ipc_type_name(
>               case MACH_MSG_TYPE_MAKE_SEND_ONCE:
>               return "make_send_once";
>  
> +             case MACH_MSG_TYPE_COPY_SEND_ONCE:
> +             return "copy_send_once";
> +
>               default:
>               return (char *) 0;
>       }
> diff --git a/ipc/ipc_object.c b/ipc/ipc_object.c
> index 1074fb2c..fe84e7d0 100644
> --- a/ipc/ipc_object.c
> +++ b/ipc/ipc_object.c
> @@ -370,6 +370,7 @@ ipc_object_copyin_type(
>  
>           case MACH_MSG_TYPE_MOVE_SEND_ONCE:
>           case MACH_MSG_TYPE_MAKE_SEND_ONCE:
> +         case MACH_MSG_TYPE_COPY_SEND_ONCE:
>               return MACH_MSG_TYPE_PORT_SEND_ONCE;
>  
>           case MACH_MSG_TYPE_MOVE_SEND:
> @@ -416,7 +417,8 @@ ipc_object_copyin(
>       /*
>        *      Could first try a read lock when doing
>        *      MACH_MSG_TYPE_COPY_SEND, MACH_MSG_TYPE_MAKE_SEND,
> -      *      and MACH_MSG_TYPE_MAKE_SEND_ONCE.
> +      *      MACH_MSG_TYPE_COPY_SEND_ONCE, and
> +      *      MACH_MSG_TYPE_MAKE_SEND_ONCE.
>        */
>  
>       kr = ipc_right_lookup_write(space, name, &entry);
> @@ -459,6 +461,10 @@ ipc_object_copyin(
>   *                   The port gains a reference and a send-once right.
>   *           MACH_MSG_TYPE_MOVE_SEND_ONCE
>   *                   Consumes a naked send-once right.
> + *           MACH_MSG_TYPE_COPY_SEND_ONCE
> + *                   A naked send-once right must be supplied.
> + *                   The port gains a reference, and a send-once
> + *                   right if the port is still active.
>   *   Conditions:
>   *           Nothing locked.
>   */
> @@ -539,6 +545,19 @@ ipc_object_copyin_from_kernel(
>               /* move naked send-once right into the message */
>               break;
>  
> +         case MACH_MSG_TYPE_COPY_SEND_ONCE: {
> +             ipc_port_t port = (ipc_port_t) object;
> +
> +             ip_lock(port);
> +             if (ip_active(port)) {
> +                     assert(port->ip_sorights > 0);
> +                     port->ip_sorights++;
> +             }
> +             ip_reference(port);
> +             ip_unlock(port);
> +             break;
> +         }
> +
>           default:
>  #if MACH_ASSERT
>               assert(!"ipc_object_copyin_from_kernel: strange rights");
> diff --git a/ipc/ipc_right.c b/ipc/ipc_right.c
> index 79f70c3d..829e9b05 100644
> --- a/ipc/ipc_right.c
> +++ b/ipc/ipc_right.c
> @@ -1258,7 +1258,8 @@ ipc_right_copyin_check(
>  
>           case MACH_MSG_TYPE_COPY_SEND:
>           case MACH_MSG_TYPE_MOVE_SEND:
> -         case MACH_MSG_TYPE_MOVE_SEND_ONCE: {
> +         case MACH_MSG_TYPE_MOVE_SEND_ONCE:
> +         case MACH_MSG_TYPE_COPY_SEND_ONCE: {
>               ipc_port_t port;
>               boolean_t active;
>  
> @@ -1279,7 +1280,8 @@ ipc_right_copyin_check(
>                       break;
>               }
>  
> -             if (msgt_name == MACH_MSG_TYPE_MOVE_SEND_ONCE) {
> +             if (msgt_name == MACH_MSG_TYPE_MOVE_SEND_ONCE ||
> +                 msgt_name == MACH_MSG_TYPE_COPY_SEND_ONCE) {
>                       if ((bits & MACH_PORT_TYPE_SEND_ONCE) == 0)
>                               return FALSE;
>               } else {
> @@ -1600,6 +1602,49 @@ ipc_right_copyin(
>               break;
>           }
>  
> +         case MACH_MSG_TYPE_COPY_SEND_ONCE: {
> +             ipc_port_t port;
> +
> +             if (bits & MACH_PORT_TYPE_DEAD_NAME)
> +                     goto copy_dead;
> +
> +             if ((bits & MACH_PORT_TYPE_SEND_RIGHTS) == 0)
> +                     goto invalid_right;
> +
> +             assert(IE_BITS_UREFS(bits) > 0);
> +
> +             port = (ipc_port_t) entry->ie_object;
> +             assert(port != IP_NULL);
> +
> +             if (ipc_right_check(space, port, name, entry)) {
> +                     bits = entry->ie_bits;
> +                     goto copy_dead;
> +             }
> +
> +             /* port is locked and active */
> +
> +             if ((bits & MACH_PORT_TYPE_SEND_ONCE) == 0) {
> +                     assert(bits & MACH_PORT_TYPE_SEND);
> +                     assert(port->ip_srights > 0);
> +
> +                     ip_unlock(port);
> +                     goto invalid_right;
> +             }
> +
> +             assert(IE_BITS_TYPE(bits) == MACH_PORT_TYPE_SEND_ONCE);
> +             assert(IE_BITS_UREFS(bits) == 1);
> +             assert((bits & IE_BITS_MAREQUEST) == 0);
> +             assert(port->ip_sorights > 0);
> +
> +             port->ip_sorights++;
> +             ip_reference(port);
> +             ip_unlock(port);
> +
> +             *objectp = (ipc_object_t) port;
> +             *sorightp = IP_NULL;
> +             break;
> +         }
> +
>           default:
>  #if MACH_ASSERT
>               assert(!"ipc_right_copyin: strange rights");
> @@ -1672,7 +1717,8 @@ ipc_right_copyin_undo(
>  
>       assert((msgt_name == MACH_MSG_TYPE_MOVE_SEND) ||
>              (msgt_name == MACH_MSG_TYPE_COPY_SEND) ||
> -            (msgt_name == MACH_MSG_TYPE_MOVE_SEND_ONCE));
> +            (msgt_name == MACH_MSG_TYPE_MOVE_SEND_ONCE) ||
> +            (msgt_name == MACH_MSG_TYPE_COPY_SEND_ONCE));
>  
>       if (soright != IP_NULL) {
>               assert((msgt_name == MACH_MSG_TYPE_MOVE_SEND) ||
> @@ -1695,20 +1741,24 @@ ipc_right_copyin_undo(
>               assert(object == IO_DEAD);
>               assert(IE_BITS_UREFS(bits) > 0);
>  
> -             if (msgt_name != MACH_MSG_TYPE_COPY_SEND) {
> +             if (msgt_name != MACH_MSG_TYPE_COPY_SEND &&
> +                 msgt_name != MACH_MSG_TYPE_COPY_SEND_ONCE) {
>                       assert(IE_BITS_UREFS(bits) < MACH_PORT_UREFS_MAX);
>  
>                       entry->ie_bits = bits+1; /* increment urefs */
>               }
>       } else {
>               assert((msgt_name == MACH_MSG_TYPE_MOVE_SEND) ||
> -                    (msgt_name == MACH_MSG_TYPE_COPY_SEND));
> -             assert(IE_BITS_TYPE(bits) == MACH_PORT_TYPE_SEND);
> +                    (msgt_name == MACH_MSG_TYPE_COPY_SEND) ||
> +                    (msgt_name == MACH_MSG_TYPE_COPY_SEND_ONCE));
> +             assert(IE_BITS_TYPE(bits) == MACH_PORT_TYPE_SEND ||
> +                    IE_BITS_TYPE(bits) == MACH_PORT_TYPE_SEND_ONCE);
>               assert(object != IO_DEAD);
>               assert(entry->ie_object == object);
>               assert(IE_BITS_UREFS(bits) > 0);
>  
> -             if (msgt_name != MACH_MSG_TYPE_COPY_SEND) {
> +             if (msgt_name != MACH_MSG_TYPE_COPY_SEND &&
> +                 msgt_name != MACH_MSG_TYPE_COPY_SEND_ONCE) {
>                       assert(IE_BITS_UREFS(bits) < MACH_PORT_UREFS_MAX-1);
>  
>                       entry->ie_bits = bits+1; /* increment urefs */
> -- 
> 2.44.0



reply via email to

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