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.
This commit is contained in:
Markus Wichmann 2023-10-31 17:03:44 +01:00 committed by Rich Felker
parent 5baf2d92d3
commit 7f3a292536

View File

@ -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<count; i++)
sem_post(&target_sem);
sem_post(&exit_sem);
for (i=0; i<count; i++)
sem_wait(&caller_sem);
sem_destroy(&caller_sem);
sem_destroy(&target_sem);
sem_destroy(&exit_sem);
pthread_setcancelstate(cs, 0);
__tl_unlock();