diff mbox series

[RFC,27/34] hurd: Don't leak __hurd_reply_port0

Message ID 20230319151017.531737-28-bugaevc@gmail.com (mailing list archive)
State Committed, archived
Headers
Series The rest of the x86_64-gnu port |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Sergey Bugaev March 19, 2023, 3:10 p.m. UTC 
  Previously, once we set up TLS, we would implicitly switch from using
__hurd_reply_port0 to reply_port inside the TCB, leaving the former
unused. But we never deallocated it, so it got leaked.

Instead, migrate the port into the new TCB's reply_port slot. This
avoids both the port leak and an extra syscall to create a new reply
port for the TCB.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/i386/tls.h   | 5 +++++
 sysdeps/mach/hurd/x86_64/tls.h | 9 ++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Samuel Thibault April 10, 2023, 10:25 p.m. UTC | #1 
Applied, thanks!

Sergey Bugaev, le dim. 19 mars 2023 18:10:10 +0300, a ecrit:
> Previously, once we set up TLS, we would implicitly switch from using
> __hurd_reply_port0 to reply_port inside the TCB, leaving the former
> unused. But we never deallocated it, so it got leaked.
> 
> Instead, migrate the port into the new TCB's reply_port slot. This
> avoids both the port leak and an extra syscall to create a new reply
> port for the TCB.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/i386/tls.h   | 5 +++++
>  sysdeps/mach/hurd/x86_64/tls.h | 9 ++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h
> index ee7b8004..0ac8917a 100644
> --- a/sysdeps/mach/hurd/i386/tls.h
> +++ b/sysdeps/mach/hurd/i386/tls.h
> @@ -141,12 +141,15 @@ _hurd_tls_init (tcbhead_t *tcb)
>    HURD_TLS_DESC_DECL (desc, tcb);
>    thread_t self = __mach_thread_self ();
>    bool success = true;
> +  extern mach_port_t __hurd_reply_port0;
>  
>    /* This field is used by TLS accesses to get our "thread pointer"
>       from the TLS point of view.  */
>    tcb->tcb = tcb;
>    /* We always at least start the sigthread anyway.  */
>    tcb->multiple_threads = 1;
> +  /* Take over the reply port we've been using.  */
> +  tcb->reply_port = __hurd_reply_port0;
>  
>    /* Get the first available selector.  */
>    int sel = -1;
> @@ -172,6 +175,8 @@ _hurd_tls_init (tcbhead_t *tcb)
>  
>    /* Now install the new selector.  */
>    asm volatile ("mov %w0, %%gs" :: "q" (sel));
> +  /* This port is now owned by the TCB.  */
> +  __hurd_reply_port0 = MACH_PORT_NULL;
>  
>  out:
>    __mach_port_deallocate (__mach_task_self (), self);
> diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
> index da504d9c..2b7135f6 100644
> --- a/sysdeps/mach/hurd/x86_64/tls.h
> +++ b/sysdeps/mach/hurd/x86_64/tls.h
> @@ -177,13 +177,20 @@ _hurd_tls_init (tcbhead_t *tcb)
>  {
>    error_t err;
>    thread_t self = __mach_thread_self ();
> +  extern mach_port_t __hurd_reply_port0;
>  
>    /* We always at least start the sigthread anyway.  */
>    tcb->multiple_threads = 1;
> +  /* Take over the reply port we've been using.  */
> +  tcb->reply_port = __hurd_reply_port0;
>  
>    err = _hurd_tls_new (self, tcb);
>    if (err == 0)
> -    __libc_tls_initialized = 1;
> +    {
> +      __libc_tls_initialized = 1;
> +      /* This port is now owned by the TCB.  */
> +      __hurd_reply_port0 = MACH_PORT_NULL;
> +    }
>    __mach_port_deallocate (__mach_task_self (), self);
>    return err == 0;
>  }
> -- 
> 2.39.2
>
diff mbox series

Patch

diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h
index ee7b8004..0ac8917a 100644
--- a/sysdeps/mach/hurd/i386/tls.h
+++ b/sysdeps/mach/hurd/i386/tls.h
@@ -141,12 +141,15 @@  _hurd_tls_init (tcbhead_t *tcb)
   HURD_TLS_DESC_DECL (desc, tcb);
   thread_t self = __mach_thread_self ();
   bool success = true;
+  extern mach_port_t __hurd_reply_port0;
 
   /* This field is used by TLS accesses to get our "thread pointer"
      from the TLS point of view.  */
   tcb->tcb = tcb;
   /* We always at least start the sigthread anyway.  */
   tcb->multiple_threads = 1;
+  /* Take over the reply port we've been using.  */
+  tcb->reply_port = __hurd_reply_port0;
 
   /* Get the first available selector.  */
   int sel = -1;
@@ -172,6 +175,8 @@  _hurd_tls_init (tcbhead_t *tcb)
 
   /* Now install the new selector.  */
   asm volatile ("mov %w0, %%gs" :: "q" (sel));
+  /* This port is now owned by the TCB.  */
+  __hurd_reply_port0 = MACH_PORT_NULL;
 
 out:
   __mach_port_deallocate (__mach_task_self (), self);
diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
index da504d9c..2b7135f6 100644
--- a/sysdeps/mach/hurd/x86_64/tls.h
+++ b/sysdeps/mach/hurd/x86_64/tls.h
@@ -177,13 +177,20 @@  _hurd_tls_init (tcbhead_t *tcb)
 {
   error_t err;
   thread_t self = __mach_thread_self ();
+  extern mach_port_t __hurd_reply_port0;
 
   /* We always at least start the sigthread anyway.  */
   tcb->multiple_threads = 1;
+  /* Take over the reply port we've been using.  */
+  tcb->reply_port = __hurd_reply_port0;
 
   err = _hurd_tls_new (self, tcb);
   if (err == 0)
-    __libc_tls_initialized = 1;
+    {
+      __libc_tls_initialized = 1;
+      /* This port is now owned by the TCB.  */
+      __hurd_reply_port0 = MACH_PORT_NULL;
+    }
   __mach_port_deallocate (__mach_task_self (), self);
   return err == 0;
 }