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 <bugaevc@gmail.com>
Message-Id: <20230517191436.73636-7-bugaevc@gmail.com>
This commit is contained in:
Sergey Bugaev 2023-05-17 22:14:32 +03:00 committed by Samuel Thibault
parent aa19c68d2b
commit c7fcce38c8
3 changed files with 14 additions and 26 deletions

View File

@ -32,7 +32,7 @@ typedef struct
{ {
void *tcb; /* Points to this structure. */ void *tcb; /* Points to this structure. */
dtv_t *dtv; /* Vector of pointers to TLS data. */ 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; int multiple_threads;
uintptr_t sysinfo; uintptr_t sysinfo;
uintptr_t stack_guard; uintptr_t stack_guard;
@ -419,7 +419,6 @@ _hurd_tls_new (thread_t child, tcbhead_t *tcb)
HURD_TLS_DESC_DECL (desc, tcb); HURD_TLS_DESC_DECL (desc, tcb);
tcb->tcb = tcb; tcb->tcb = tcb;
tcb->self = child;
if (HURD_SEL_LDT (sel)) if (HURD_SEL_LDT (sel))
err = __i386_set_ldt (child, sel, &desc, 1); err = __i386_set_ldt (child, sel, &desc, 1);

View File

@ -19,6 +19,7 @@
#include <stdint.h> #include <stdint.h>
#include <assert.h> #include <assert.h>
#include <mach.h> #include <mach.h>
#include <hurd.h>
#include <pt-internal.h> #include <pt-internal.h>
@ -76,35 +77,24 @@ __pthread_setup (struct __pthread *thread,
void *), void *(*start_routine) (void *), void *), void *(*start_routine) (void *),
void *arg) void *arg)
{ {
tcbhead_t *tcb;
error_t err; error_t err;
mach_port_t ktid;
thread->mcontext.pc = entry_point; if (thread->kernel_thread == hurd_thread_self ())
thread->mcontext.sp = stack_setup (thread, start_routine, arg);
ktid = __mach_thread_self ();
if (thread->kernel_thread == ktid)
/* Fix up the TCB for the main thread. The C library has already /* 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 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 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. other hand, it's not yet possible to reliably release a TCB.
Leave the unused one registered so that it doesn't leak. The Leave the unused one registered so that it doesn't leak. */
only thing left to do is to correctly set the `self' member in return 0;
the already existing TCB. */
tcb = THREAD_SELF; thread->mcontext.pc = entry_point;
else thread->mcontext.sp = stack_setup (thread, start_routine, arg);
{
err = __thread_set_pcsptp (thread->kernel_thread, err = __thread_set_pcsptp (thread->kernel_thread,
1, thread->mcontext.pc, 1, thread->mcontext.pc,
1, thread->mcontext.sp, 1, thread->mcontext.sp,
1, thread->tcb); 1, thread->tcb);
assert_perror (err); assert_perror (err);
tcb = thread->tcb;
}
__mach_port_deallocate (__mach_task_self (), ktid);
tcb->self = thread->kernel_thread;
return 0; return 0;
} }

View File

@ -35,7 +35,7 @@ typedef struct
{ {
void *tcb; /* Points to this structure. */ void *tcb; /* Points to this structure. */
dtv_t *dtv; /* Vector of pointers to TLS data. */ 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 __glibc_padding1;
int multiple_threads; int multiple_threads;
int gscope_flag; int gscope_flag;
@ -158,7 +158,6 @@ _hurd_tls_new (thread_t child, tcbhead_t *tcb)
struct i386_fsgs_base_state state; struct i386_fsgs_base_state state;
tcb->tcb = tcb; tcb->tcb = tcb;
tcb->self = child;
/* Install the TCB address into FS base. */ /* Install the TCB address into FS base. */
state.fs_base = (uintptr_t) tcb; state.fs_base = (uintptr_t) tcb;