From e4235d70672d9751d7718ddc2b52d0b426430768 Mon Sep 17 00:00:00 2001 From: Rich Felker Date: Sat, 16 Feb 2019 09:13:45 -0500 Subject: [PATCH] rewrite __synccall in terms of global thread list the __synccall mechanism provides stop-the-world synchronous execution of a callback in all threads of the process. it is used to implement multi-threaded setuid/setgid operations, since Linux lacks them at the kernel level, and for some other less-critical purposes. this change eliminates dependency on /proc/self/task to determine the set of live threads, which in addition to being an unwanted dependency and a potential point of resource-exhaustion failure, turned out to be inaccurate. test cases provided by Alexey Izbyshev showed that it could fail to reflect newly created threads. due to how the presignaling phase worked, this usually yielded a deadlock if hit, but in the worst case it could also result in threads being silently missed (allowed to continue running without executing the callback). --- src/internal/pthread_impl.h | 1 - src/thread/pthread_create.c | 4 - src/thread/synccall.c | 178 ++++++++++++------------------------ 3 files changed, 59 insertions(+), 124 deletions(-) diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h index 508b40b5..d5d969ec 100644 --- a/src/internal/pthread_impl.h +++ b/src/internal/pthread_impl.h @@ -139,7 +139,6 @@ hidden void __pthread_tsd_run_dtors(); hidden void __pthread_key_delete_synccall(void (*)(void *), void *); hidden int __pthread_key_delete_impl(pthread_key_t); -extern hidden volatile int __block_new_threads; extern hidden volatile size_t __pthread_tsd_size; extern hidden void *__pthread_tsd_main[]; extern hidden volatile int __aio_fut; diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c index 03cdea0a..cec82157 100644 --- a/src/thread/pthread_create.c +++ b/src/thread/pthread_create.c @@ -201,8 +201,6 @@ weak_alias(dummy, __pthread_tsd_size); static void *dummy_tsd[1] = { 0 }; weak_alias(dummy_tsd, __pthread_tsd_main); -volatile int __block_new_threads = 0; - static FILE *volatile dummy_file = 0; weak_alias(dummy_file, __stdin_used); weak_alias(dummy_file, __stdout_used); @@ -247,8 +245,6 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att attr._a_guardsize = __default_guardsize; } - if (__block_new_threads) __wait(&__block_new_threads, 0, 1, 1); - if (attr._a_stackaddr) { size_t need = libc.tls_size + __pthread_tsd_size; size = attr._a_stacksize; diff --git a/src/thread/synccall.c b/src/thread/synccall.c index cc66bd24..648a6ad4 100644 --- a/src/thread/synccall.c +++ b/src/thread/synccall.c @@ -1,46 +1,42 @@ #include "pthread_impl.h" #include -#include -#include #include -#include -#include "futex.h" -#include "atomic.h" -#include "../dirent/__dirent.h" -#include "lock.h" - -static struct chain { - struct chain *next; - int tid; - sem_t target_sem, caller_sem; -} *volatile head; - -static volatile int synccall_lock[1]; -static volatile int target_tid; + +static void dummy_0(void) +{ +} + +weak_alias(dummy_0, __tl_lock); +weak_alias(dummy_0, __tl_unlock); + +static int target_tid; static void (*callback)(void *), *context; -static volatile int dummy = 0; -weak_alias(dummy, __block_new_threads); +static sem_t target_sem, caller_sem; + +static void dummy(void *p) +{ +} static void handler(int sig) { - struct chain ch; - int old_errno = errno; + if (__pthread_self()->tid != target_tid) return; - sem_init(&ch.target_sem, 0, 0); - sem_init(&ch.caller_sem, 0, 0); + int old_errno = errno; - ch.tid = __syscall(SYS_gettid); + /* Inform caller we have received signal and wait for + * the caller to let us make the callback. */ + sem_post(&caller_sem); + sem_wait(&target_sem); - do ch.next = head; - while (a_cas_p(&head, ch.next, &ch) != ch.next); + callback(context); - if (a_cas(&target_tid, ch.tid, 0) == (ch.tid | 0x80000000)) - __syscall(SYS_futex, &target_tid, FUTEX_UNLOCK_PI|FUTEX_PRIVATE); + /* 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(&ch.target_sem); - callback(context); - sem_post(&ch.caller_sem); - sem_wait(&ch.target_sem); + /* Inform caller we are returning and state is destroyable. */ + sem_post(&caller_sem); errno = old_errno; } @@ -48,12 +44,10 @@ static void handler(int sig) void __synccall(void (*func)(void *), void *ctx) { sigset_t oldmask; - int cs, i, r, pid, self;; - DIR dir = {0}; - struct dirent *de; + int cs, i, r; struct sigaction sa = { .sa_flags = SA_RESTART, .sa_handler = handler }; - struct chain *cp, *next; - struct timespec ts; + pthread_t self = __pthread_self(), td; + int count = 0; /* Blocking signals in two steps, first only app-level signals * before taking the lock, then all signals after taking the lock, @@ -62,98 +56,45 @@ void __synccall(void (*func)(void *), void *ctx) * any until after the lock would allow re-entry in the same thread * with the lock already held. */ __block_app_sigs(&oldmask); - LOCK(synccall_lock); + __tl_lock(); __block_all_sigs(0); pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs); - head = 0; + sem_init(&target_sem, 0, 0); + sem_init(&caller_sem, 0, 0); - if (!libc.threaded) goto single_threaded; + if (!libc.threads_minus_1) goto single_threaded; callback = func; context = ctx; - /* This atomic store ensures that any signaled threads will see the - * above stores, and prevents more than a bounded number of threads, - * those already in pthread_create, from creating new threads until - * the value is cleared to zero again. */ - a_store(&__block_new_threads, 1); - /* Block even implementation-internal signals, so that nothing * interrupts the SIGSYNCCALL handlers. The main possible source * of trouble is asynchronous cancellation. */ memset(&sa.sa_mask, -1, sizeof sa.sa_mask); __libc_sigaction(SIGSYNCCALL, &sa, 0); - pid = __syscall(SYS_getpid); - self = __syscall(SYS_gettid); - - /* Since opendir is not AS-safe, the DIR needs to be setup manually - * in automatic storage. Thankfully this is easy. */ - dir.fd = open("/proc/self/task", O_RDONLY|O_DIRECTORY|O_CLOEXEC); - if (dir.fd < 0) goto out; - - /* Initially send one signal per counted thread. But since we can't - * synchronize with thread creation/exit here, there could be too - * few signals. This initial signaling is just an optimization, not - * part of the logic. */ - for (i=libc.threads_minus_1; i; i--) - __syscall(SYS_kill, pid, SIGSYNCCALL); - - /* Loop scanning the kernel-provided thread list until it shows no - * threads that have not already replied to the signal. */ - for (;;) { - int miss_cnt = 0; - while ((de = readdir(&dir))) { - if (!isdigit(de->d_name[0])) continue; - int tid = atoi(de->d_name); - if (tid == self || !tid) continue; - - /* Set the target thread as the PI futex owner before - * checking if it's in the list of caught threads. If it - * adds itself to the list after we check for it, then - * it will see its own tid in the PI futex and perform - * the unlock operation. */ - a_store(&target_tid, tid); - - /* Thread-already-caught is a success condition. */ - for (cp = head; cp && cp->tid != tid; cp=cp->next); - if (cp) continue; - - r = -__syscall(SYS_tgkill, pid, tid, SIGSYNCCALL); - - /* Target thread exit is a success condition. */ - if (r == ESRCH) continue; - - /* The FUTEX_LOCK_PI operation is used to loan priority - * to the target thread, which otherwise may be unable - * to run. Timeout is necessary because there is a race - * condition where the tid may be reused by a different - * process. */ - clock_gettime(CLOCK_REALTIME, &ts); - ts.tv_nsec += 10000000; - if (ts.tv_nsec >= 1000000000) { - ts.tv_sec++; - ts.tv_nsec -= 1000000000; - } - r = -__syscall(SYS_futex, &target_tid, - FUTEX_LOCK_PI|FUTEX_PRIVATE, 0, &ts); - - /* Obtaining the lock means the thread responded. ESRCH - * means the target thread exited, which is okay too. */ - if (!r || r == ESRCH) continue; - - miss_cnt++; + + for (td=self->next; td!=self; td=td->next) { + target_tid = td->tid; + while ((r = -__syscall(SYS_tkill, td->tid, SIGSYNCCALL)) == EAGAIN); + if (r) { + /* If we failed to signal any thread, nop out the + * callback to abort the synccall and just release + * any threads already caught. */ + callback = func = dummy; + break; } - if (!miss_cnt) break; - rewinddir(&dir); + sem_wait(&caller_sem); + count++; } - close(dir.fd); + target_tid = 0; - /* Serialize execution of callback in caught threads. */ - for (cp=head; cp; cp=cp->next) { - sem_post(&cp->target_sem); - sem_wait(&cp->caller_sem); + /* Serialize execution of callback in caught threads, or just + * release them all if synccall is being aborted. */ + for (i=0; inext; - sem_post(&cp->target_sem); - } + for (i=0; i