Re: [RFC PATCH glibc 27/34] hurd: Don't leak __hurd_reply_port0

[Samuel Thibault]

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
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.