From 7f3a2925369c00abc33457400e33632e6dacb8ae Mon Sep 17 00:00:00 2001 From: Markus Wichmann Date: Tue, 31 Oct 2023 17:03:44 +0100 Subject: synccall: add separate exit_sem to fix thread release logic bug The code intends for the sem_post() in line 97 (now 98) to only unblock target threads waiting on line 29. But after the first thread is released, the next sem_post() might also unblock a thread waiting on line 36. That would cause the thread to return to the execution of user code before all threads are done, leading to user code being executed in a mixed-credentials environment. What's more, if this happens more than once, then the mass release on line 110 (now line 111) will cause multiple threads to execute the callback at the same time, and the callbacks are currently not written to cope with that situation. Adding another semaphore allows the caller to say explicitly which threads it wants to release. --- src/thread/synccall.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src/thread/synccall.c') diff --git a/src/thread/synccall.c b/src/thread/synccall.c index a6b177c0..38597254 100644 --- a/src/thread/synccall.c +++ b/src/thread/synccall.c @@ -11,7 +11,7 @@ weak_alias(dummy_0, __tl_unlock); static int target_tid; static void (*callback)(void *), *context; -static sem_t target_sem, caller_sem; +static sem_t target_sem, caller_sem, exit_sem; static void dummy(void *p) { @@ -33,7 +33,7 @@ static void handler(int sig) /* Inform caller we've complered the callback and wait * for the caller to release us to return. */ sem_post(&caller_sem); - sem_wait(&target_sem); + sem_wait(&exit_sem); /* Inform caller we are returning and state is destroyable. */ sem_post(&caller_sem); @@ -62,6 +62,7 @@ void __synccall(void (*func)(void *), void *ctx) sem_init(&target_sem, 0, 0); sem_init(&caller_sem, 0, 0); + sem_init(&exit_sem, 0, 0); if (!libc.threads_minus_1 || __syscall(SYS_gettid) != self->tid) goto single_threaded; @@ -107,12 +108,13 @@ single_threaded: /* Only release the caught threads once all threads, including the * caller, have returned from the callback function. */ for (i=0; i