Applied, thanks! Sergey Bugaev, le mer. 17 mai 2023 22:14:32 +0300, a ecrit: > 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 <buga...@gmail.com> > --- > 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 e124fb10..ba283008 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 3abd92b2..686124d7 100644 > --- a/sysdeps/mach/hurd/x86/htl/pt-setup.c > +++ b/sysdeps/mach/hurd/x86/htl/pt-setup.c > @@ -19,6 +19,7 @@ > #include <stdint.h> > #include <assert.h> > #include <mach.h> > +#include <hurd.h> > > #include <pt-internal.h> > > @@ -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 1274723a..35dcef44 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; > -- > 2.40.1 > >
-- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria.