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 <buga...@gmail.com> --- NOTE: I don't really understand why sigunwind wants to destroy both its current reply port and user's reply port. Prior to commit fb304035c41c7ee2afede51e5e8568974549ba5e, it was *restoring* the user's reply port, in which case it actually made sense to destroy the current reply port. Post-fb304035c41c7ee2afede51e5e8568974549ba5e, wouldn't it be better to just keep using the current reply port, destroying the user's one? hurd/sigunwind.c | 24 +++++++++++------------- sysdeps/mach/hurd/i386/sigreturn.c | 21 +++++---------------- 2 files changed, 16 insertions(+), 29 deletions(-) diff --git a/hurd/sigunwind.c b/hurd/sigunwind.c index edd40b14..399e6900 100644 --- 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 db1a01f3..29c9629f 100644 --- 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 -- 2.39.2