[glibc] hurd: Replace reply port with a dead name on failed interruption

[glibc] hurd: Replace reply port with a dead name on failed interruption

[Samuel Thibault]

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=4e506f67cbe6cd935377da65909f0606014459aa

commit 4e506f67cbe6cd935377da65909f0606014459aa
Author: Sergey Bugaev <bugaevc@gmail.com>
Date:   Sat Apr 29 23:18:19 2023 +0300

    hurd: Replace reply port with a dead name on failed interruption
    
    If we're trying to interrupt an interruptible RPC, but the server fails
    to respond to our __interrupt_operation () call, we instead destroy the
    reply port we were expecting the reply to the RPC on.
    
    Instead of deallocating the name completely, replace it with a dead
    name, so the name won't get reused for some other right, and deallocate
    it in _hurd_intr_rpc_mach_msg once we return from the signal handler.
    
    Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
    Message-Id: <20230429201822.2605207-4-bugaevc@gmail.com>

Diff:
---
 hurd/hurdsig.c                | 15 ++++++++++++---
 hurd/intr-msg.c               |  1 +
 sysdeps/mach/hurd/mig-reply.c | 25 +++++++------------------
 3 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c
index b3808f9e49..78ea59d97d 100644
--- a/hurd/hurdsig.c
+++ b/hurd/hurdsig.c
@@ -477,9 +477,18 @@ _hurdsig_abort_rpcs (struct hurd_sigstate *ss, int signo, int sigthread,
           if (reply)
             {
               /* The interrupt didn't work.
-                 Destroy the receive right the thread is blocked on.  */
-              __mach_port_destroy (__mach_task_self (), *reply);
-              *reply = MACH_PORT_NULL;
+                 Destroy the receive right the thread is blocked on, and
+                 replace it with a dead name to keep the name from reuse until
+                 the therad is done with it.  To do this atomically, first
+                 insert a send right, and then destroy the receive right,
+                 turning the send right into a dead name.  */
+              err = __mach_port_insert_right (__mach_task_self (),
+                                              *reply, *reply,
+                                              MACH_MSG_TYPE_MAKE_SEND);
+              assert_perror (err);
+              err = __mach_port_mod_refs (__mach_task_self (), *reply,
+                                          MACH_PORT_RIGHT_RECEIVE, -1);
+              assert_perror (err);
             }
 
           /* The system call return value register now contains
diff --git a/hurd/intr-msg.c b/hurd/intr-msg.c
index 1a086b5141..716d87ab6a 100644
--- a/hurd/intr-msg.c
+++ b/hurd/intr-msg.c
@@ -305,6 +305,7 @@ _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
 	{
 	  /* Make sure we have a valid reply port.  The one we were using
 	     may have been destroyed by interruption.  */
+	  __mig_dealloc_reply_port (rcv_name);
 	  m->header.msgh_local_port = rcv_name = __mig_get_reply_port ();
 	  m->header.msgh_bits = msgh_bits;
 	  option = user_option;
diff --git a/sysdeps/mach/hurd/mig-reply.c b/sysdeps/mach/hurd/mig-reply.c
index 3fdee80e5a..7ea001df0a 100644
--- a/sysdeps/mach/hurd/mig-reply.c
+++ b/sysdeps/mach/hurd/mig-reply.c
@@ -69,29 +69,18 @@ __mig_dealloc_reply_port (mach_port_t arg)
   mach_port_t port = get_reply_port ();
 
   set_reply_port (MACH_PORT_NULL);	/* So the mod_refs RPC won't use it.  */
-
-  /* Normally, ARG should be the same as PORT that we store.  However, if a
-     signal has interrupted the RPC, the stored PORT has been deallocated and
-     reset to MACH_PORT_NULL (or possibly MACH_PORT_DEAD).  In this case the
-     MIG routine still has the old name, which it passes to us here.  We must
-     not deallocate (or otherwise touch) it, since it may be already allocated
-     to another port right.  Fortunately MIG itself doesn't do anything with
-     the reply port on errors either, other than immediately calling this
-     function.
-
-     And so:
-     1. Assert that things are sane, i.e. and PORT is either invalid or same
-        as ARG.
-     2. Only deallocate the name if our stored PORT still names it.  In that
-        case we're sure the right has not been deallocated / the name reused.
-    */
-
+  assert (port == arg);
   if (!MACH_PORT_VALID (port))
     return;
-  assert (port == arg);
 
   err = __mach_port_mod_refs (__mach_task_self (), port,
                               MACH_PORT_RIGHT_RECEIVE, -1);
+  if (err == KERN_INVALID_RIGHT)
+    /* It could be that during signal handling, the receive right had been
+       replaced with a dead name.  */
+    err = __mach_port_mod_refs (__mach_task_self (), port,
+                                MACH_PORT_RIGHT_DEAD_NAME, -1);
+
   assert_perror (err);
 }
 weak_alias (__mig_dealloc_reply_port, mig_dealloc_reply_port)