hurd: Improve reply port handling when exiting signal handlers

If we're doing signals, that means we've already got the signal thread
running, and that implies TLS having been set up. So we know that
__hurd_local_reply_port will resolve to THREAD_SELF->reply_port, and can
access that directly using the THREAD_GETMEM and THREAD_SETMEM macros.
This avoids potential miscompilations, and should also be a tiny bit
faster.

Also, use mach_port_mod_refs () and not mach_port_destroy () to destroy
the receive right. mach_port_destroy () should *never* be used on
mach_task_self (); this can easily lead to port use-after-free
vulnerabilities if the task has any other references to the same port.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
Message-Id: <20230319151017.531737-26-bugaevc@gmail.com>

diff --git a/hurd/sigunwind.c b/hurd/sigunwind.c
index edd40b140e463a70b93877e29ac7556074f063fa..399e69008ee6c3cc9b8d6cebfd832c17c875d1b8 100644 (file)
--- a/hurd/sigunwind.c
+++ b/hurd/sigunwind.c
@@ -18,7 +18,6 @@
 
 #include <hurd.h>
 #include <thread_state.h>
-#include <hurd/threadvar.h>
 #include <jmpbuf-unwind.h>
 #include <assert.h>
 #include <stdint.h>
@@ -39,19 +38,18 @@ _hurdsig_longjmp_from_handler (void *data, jmp_buf env, int val)
     {
       /* Destroy the MiG reply port used by the signal handler, and restore
         the reply port in use by the thread when interrupted.  */
-      mach_port_t *reply_port = &__hurd_local_reply_port;
-      if (*reply_port)
-       {
-         mach_port_t port = *reply_port;
-         /* Assigning MACH_PORT_DEAD here tells libc's mig_get_reply_port
-            not to get another reply port, but avoids mig_dealloc_reply_port
-            trying to deallocate it after the receive fails (which it will,
-            because the reply port will be bogus, regardless).  */
-         *reply_port = MACH_PORT_DEAD;
-         __mach_port_destroy (__mach_task_self (), port);
-       }
+      mach_port_t reply_port = THREAD_GETMEM (THREAD_SELF, reply_port);
+      /* Assigning MACH_PORT_DEAD here tells libc's mig_get_reply_port not to
+         get another reply port, but avoids mig_dealloc_reply_port trying to
+         deallocate it after the receive fails (which it will, because the
+         reply port will be bogus, regardless).  */
+      THREAD_SETMEM (THREAD_SELF, reply_port, MACH_PORT_DEAD);
+      if (MACH_PORT_VALID (reply_port))
+        __mach_port_mod_refs (__mach_task_self (), reply_port,
+                              MACH_PORT_RIGHT_RECEIVE, -1);
       if (scp->sc_reply_port)
-       __mach_port_destroy (__mach_task_self (), scp->sc_reply_port);
+        __mach_port_mod_refs (__mach_task_self (), scp->sc_reply_port,
+                              MACH_PORT_RIGHT_RECEIVE, -1);
     }
 
   __spin_lock (&ss->lock);

diff --git a/sysdeps/mach/hurd/i386/sigreturn.c b/sysdeps/mach/hurd/i386/sigreturn.c
index db1a01f38544e470ca12a52aa4d735f39a8761a8..29c9629f45baa2e1297d66bb8c6ba5539fae6481 100644 (file)
--- a/sysdeps/mach/hurd/i386/sigreturn.c
+++ b/sysdeps/mach/hurd/i386/sigreturn.c
@@ -19,7 +19,6 @@ register int *sp asm ("%esp");
 
 #include <hurd.h>
 #include <hurd/signal.h>
-#include <hurd/threadvar.h>
 #include <hurd/msg.h>
 #include <stdlib.h>
 #include <string.h>
@@ -59,7 +58,7 @@ __sigreturn (struct sigcontext *scp)
 {
   struct hurd_sigstate *ss;
   struct hurd_userlink *link = (void *) &scp[1];
-  mach_port_t *reply_port;
+  mach_port_t reply_port;
 
   if (scp == NULL || (scp->sc_mask & _SIG_CANT_MASK))
     {
@@ -101,20 +100,10 @@ __sigreturn (struct sigcontext *scp)
 
   /* Destroy the MiG reply port used by the signal handler, and restore the
      reply port in use by the thread when interrupted.  */
-  reply_port = &__hurd_local_reply_port;
-  if (*reply_port)
-    {
-      mach_port_t port = *reply_port;
-
-      /* Assigning MACH_PORT_DEAD here tells libc's mig_get_reply_port not to
-        get another reply port, but avoids mig_dealloc_reply_port trying to
-        deallocate it after the receive fails (which it will, because the
-        reply port will be bogus, whether we do this or not).  */
-      *reply_port = MACH_PORT_DEAD;
-
-      __mach_port_destroy (__mach_task_self (), port);
-    }
-  *reply_port = scp->sc_reply_port;
+  reply_port = THREAD_GETMEM (THREAD_SELF, reply_port);
+  THREAD_SETMEM (THREAD_SELF, reply_port, scp->sc_reply_port);
+  __mach_port_mod_refs (__mach_task_self (), reply_port,
+                        MACH_PORT_RIGHT_RECEIVE, -1);
 
   if (scp->sc_fpused)
     /* Restore the FPU state.  Mach conveniently stores the state