From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1792) id 393C33858D1E; Wed, 17 May 2023 21:01:08 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 393C33858D1E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1684357268; bh=BYbQkYwFhebbSuSbQXHzLJS/UGknAvnz4JAbCpQVXRo=; h=From:To:Subject:Date:From; b=arVGYgHF0m74LR7x6bB4xc2l5SZKT8rFLtfFamTn6abXfN7VTAJczAKbhVd3UBOGx FkBJJqBFZ69zF4Z+2ln3AH53XGtHa4zhX1buSpGHZPFAIvQ9xWE2pBRDZJhRuHK0bk KXciztAktzmZ8CTwc4OkMaKvQCYTjl/8FNlG4Bdk= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Samuel Thibault To: glibc-cvs@sourceware.org Subject: [glibc] hurd: Make sure to not use tcb->self X-Act-Checkin: glibc X-Git-Author: Sergey Bugaev X-Git-Refname: refs/heads/master X-Git-Oldrev: aa19c68d2bdf3a831894f609b8ac5c8f123268b2 X-Git-Newrev: c7fcce38c83a2bb665ef5dc4981bf20c7e586123 Message-Id: <20230517210108.393C33858D1E@sourceware.org> Date: Wed, 17 May 2023 21:01:08 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c7fcce38c83a2bb665ef5dc4981bf20c7e586123 commit c7fcce38c83a2bb665ef5dc4981bf20c7e586123 Author: Sergey Bugaev Date: Wed May 17 22:14:32 2023 +0300 hurd: Make sure to not use tcb->self Unlike sigstate->thread, tcb->self did not hold a Mach port reference on the thread port it names. This means that the port can be deallocated, and the name reused for something else, without anyone noticing. Using tcb->self will then lead to port use-after-free. Fortunately nothing was accessing tcb->self, other than it being intially set to then-valid thread port name upon TCB initialization. To assert that this keeps being the case without altering TCB layout, rename self -> self_do_not_use, and stop initializing it. Also, do not (re-)allocate a whole separate and unused stack for the main thread, and just exit __pthread_setup early in this case. Found upon attempting to use tcb->self and getting unexpected crashes. Signed-off-by: Sergey Bugaev Message-Id: <20230517191436.73636-7-bugaevc@gmail.com> Diff: --- sysdeps/mach/hurd/i386/tls.h | 3 +-- sysdeps/mach/hurd/x86/htl/pt-setup.c | 34 ++++++++++++---------------------- sysdeps/mach/hurd/x86_64/tls.h | 3 +-- 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h index e124fb10e9..ba283008a8 100644 --- a/sysdeps/mach/hurd/i386/tls.h +++ b/sysdeps/mach/hurd/i386/tls.h @@ -32,7 +32,7 @@ typedef struct { void *tcb; /* Points to this structure. */ dtv_t *dtv; /* Vector of pointers to TLS data. */ - thread_t self; /* This thread's control port. */ + thread_t self_do_not_use; /* This thread's control port. */ int multiple_threads; uintptr_t sysinfo; uintptr_t stack_guard; @@ -419,7 +419,6 @@ _hurd_tls_new (thread_t child, tcbhead_t *tcb) HURD_TLS_DESC_DECL (desc, tcb); tcb->tcb = tcb; - tcb->self = child; if (HURD_SEL_LDT (sel)) err = __i386_set_ldt (child, sel, &desc, 1); diff --git a/sysdeps/mach/hurd/x86/htl/pt-setup.c b/sysdeps/mach/hurd/x86/htl/pt-setup.c index 3abd92b27e..686124d70a 100644 --- a/sysdeps/mach/hurd/x86/htl/pt-setup.c +++ b/sysdeps/mach/hurd/x86/htl/pt-setup.c @@ -19,6 +19,7 @@ #include #include #include +#include #include @@ -76,35 +77,24 @@ __pthread_setup (struct __pthread *thread, void *), void *(*start_routine) (void *), void *arg) { - tcbhead_t *tcb; error_t err; - mach_port_t ktid; - thread->mcontext.pc = entry_point; - thread->mcontext.sp = stack_setup (thread, start_routine, arg); - - ktid = __mach_thread_self (); - if (thread->kernel_thread == ktid) + if (thread->kernel_thread == hurd_thread_self ()) /* Fix up the TCB for the main thread. The C library has already installed a TCB, which we want to keep using. This TCB must not be freed so don't register it in the thread structure. On the other hand, it's not yet possible to reliably release a TCB. - Leave the unused one registered so that it doesn't leak. The - only thing left to do is to correctly set the `self' member in - the already existing TCB. */ - tcb = THREAD_SELF; - else - { - err = __thread_set_pcsptp (thread->kernel_thread, - 1, thread->mcontext.pc, - 1, thread->mcontext.sp, - 1, thread->tcb); - assert_perror (err); - tcb = thread->tcb; - } - __mach_port_deallocate (__mach_task_self (), ktid); + Leave the unused one registered so that it doesn't leak. */ + return 0; + + thread->mcontext.pc = entry_point; + thread->mcontext.sp = stack_setup (thread, start_routine, arg); - tcb->self = thread->kernel_thread; + err = __thread_set_pcsptp (thread->kernel_thread, + 1, thread->mcontext.pc, + 1, thread->mcontext.sp, + 1, thread->tcb); + assert_perror (err); return 0; } diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h index 1274723a9d..35dcef447e 100644 --- a/sysdeps/mach/hurd/x86_64/tls.h +++ b/sysdeps/mach/hurd/x86_64/tls.h @@ -35,7 +35,7 @@ typedef struct { void *tcb; /* Points to this structure. */ dtv_t *dtv; /* Vector of pointers to TLS data. */ - thread_t self; /* This thread's control port. */ + thread_t self_do_no_use; /* This thread's control port. */ int __glibc_padding1; int multiple_threads; int gscope_flag; @@ -158,7 +158,6 @@ _hurd_tls_new (thread_t child, tcbhead_t *tcb) struct i386_fsgs_base_state state; tcb->tcb = tcb; - tcb->self = child; /* Install the TCB address into FS base. */ state.fs_base = (uintptr_t) tcb;