From ba00d787f3469b02032766b074d4df9071fa7e24 Mon Sep 17 00:00:00 2001 From: Sergey Bugaev Date: Thu, 13 Apr 2023 14:58:12 +0300 Subject: hurd: Remove __hurd_local_reply_port Now that the signal code no longer accesses it, the only real user of it was mig-reply.c, so move the logic for managing the port there. If we're in SHARED and outside of rtld, we know that __LIBC_NO_TLS () always evaluates to 0, and a TLS reply port will always be used, not __hurd_reply_port0. Still, the compiler does not see that __hurd_reply_port0 is never used due to its address being taken. To deal with this, explicitly compile out __hurd_reply_port0 when we know we won't use it. Also, instead of accessing the port via THREAD_SELF->reply_port, this uses THREAD_GETMEM and THREAD_SETMEM directly, avoiding possible miscompilations. Signed-off-by: Sergey Bugaev --- hurd/hurd/threadvar.h | 7 ----- sysdeps/mach/hurd/dl-sysdep.c | 2 +- sysdeps/mach/hurd/mig-reply.c | 70 +++++++++++++++++++++++++++++++++++-------- sysdeps/mach/sysdep.h | 2 +- 4 files changed, 60 insertions(+), 21 deletions(-) diff --git a/hurd/hurd/threadvar.h b/hurd/hurd/threadvar.h index f5c6a278a6..c476d98880 100644 --- a/hurd/hurd/threadvar.h +++ b/hurd/hurd/threadvar.h @@ -29,11 +29,4 @@ extern unsigned long int __hurd_sigthread_stack_base; extern unsigned long int __hurd_sigthread_stack_end; -/* Store the MiG reply port reply port until we enable TLS. */ -extern mach_port_t __hurd_reply_port0; - -/* This returns either the TLS reply port variable, or a single-thread variable - when TLS is not initialized yet. */ -#define __hurd_local_reply_port (*(__LIBC_NO_TLS () ? &__hurd_reply_port0 : &THREAD_SELF->reply_port)) - #endif /* hurd/threadvar.h */ diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c index a2115f6edb..2d595d0006 100644 --- a/sysdeps/mach/hurd/dl-sysdep.c +++ b/sysdeps/mach/hurd/dl-sysdep.c @@ -238,7 +238,7 @@ _dl_sysdep_start_cleanup (void) /* Deallocate the reply port and task port rights acquired by __mach_init. We are done with them now, and the user will reacquire them for himself when he wants them. */ - __mig_dealloc_reply_port (MACH_PORT_NULL); + __mig_dealloc_reply_port (__mig_get_reply_port ()); __mach_port_deallocate (__mach_task_self (), __mach_host_self_); __mach_port_deallocate (__mach_task_self (), __mach_task_self_); } diff --git a/sysdeps/mach/hurd/mig-reply.c b/sysdeps/mach/hurd/mig-reply.c index 33ff260e36..3fdee80e5a 100644 --- a/sysdeps/mach/hurd/mig-reply.c +++ b/sysdeps/mach/hurd/mig-reply.c @@ -17,22 +17,46 @@ #include #include -#include +#include /* These functions are called by MiG-generated code. */ +#if !defined (SHARED) || IS_IN (rtld) mach_port_t __hurd_reply_port0; +#endif + +static mach_port_t +get_reply_port (void) +{ +#if !defined (SHARED) || IS_IN (rtld) + if (__LIBC_NO_TLS ()) + return __hurd_reply_port0; +#endif + return THREAD_GETMEM (THREAD_SELF, reply_port); +} + +static void +set_reply_port (mach_port_t port) +{ +#if !defined (SHARED) || IS_IN (rtld) + if (__LIBC_NO_TLS ()) + __hurd_reply_port0 = port; + else +#endif + THREAD_SETMEM (THREAD_SELF, reply_port, port); +} /* Called by MiG to get a reply port. */ mach_port_t __mig_get_reply_port (void) { - if (__hurd_local_reply_port == MACH_PORT_NULL - || (&__hurd_local_reply_port != &__hurd_reply_port0 - && __hurd_local_reply_port == __hurd_reply_port0)) - __hurd_local_reply_port = __mach_reply_port (); - - return __hurd_local_reply_port; + mach_port_t port = get_reply_port (); + if (__glibc_unlikely (port == MACH_PORT_NULL)) + { + port = __mach_reply_port (); + set_reply_port (port); + } + return port; } weak_alias (__mig_get_reply_port, mig_get_reply_port) libc_hidden_def (__mig_get_reply_port) @@ -41,12 +65,34 @@ libc_hidden_def (__mig_get_reply_port) void __mig_dealloc_reply_port (mach_port_t arg) { - mach_port_t port = __hurd_local_reply_port; - __hurd_local_reply_port = MACH_PORT_NULL; /* So the mod_refs RPC won't use it. */ + error_t err; + 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. + */ + + if (!MACH_PORT_VALID (port)) + return; + assert (port == arg); - if (MACH_PORT_VALID (port)) - __mach_port_mod_refs (__mach_task_self (), port, - MACH_PORT_RIGHT_RECEIVE, -1); + err = __mach_port_mod_refs (__mach_task_self (), port, + MACH_PORT_RIGHT_RECEIVE, -1); + assert_perror (err); } weak_alias (__mig_dealloc_reply_port, mig_dealloc_reply_port) libc_hidden_def (__mig_dealloc_reply_port) diff --git a/sysdeps/mach/sysdep.h b/sysdeps/mach/sysdep.h index 1c869f5cb2..35ad1e3920 100644 --- a/sysdeps/mach/sysdep.h +++ b/sysdeps/mach/sysdep.h @@ -45,7 +45,7 @@ #ifndef __ASSEMBLER__ #define FATAL_PREPARE_INCLUDE -#define FATAL_PREPARE __mig_dealloc_reply_port (MACH_PORT_NULL) +#define FATAL_PREPARE __mig_dealloc_reply_port (__mig_get_reply_port ()) #endif /* sysdeps/mach/MACHINE/sysdep.h should define the following macros. */ -- cgit 1.4.1