From b88a8d3d1d04a73764b09d94065ff4678cde6ce3 Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Tue, 10 Jan 2017 19:26:55 +0000 Subject: [PATCH] Fix acquisition of nested write compat rtld locks. Obtaining compat rtld lock in write mode sets process signal mask to block all signals. Previous mask is stored in the global variable oldsigmask. If a lock is write-locked while another lock is already write-locked, oldsigmask is overwritten by the total mask and on the last unlock, all signals except traps appear to be blocked. Fix this by counting the write-lock nested level, and only storing to oldsigmask/restoring from it at the outermost level. Masking signals disables involuntary preemption for libc_r, and there could be no voluntary context switches in the locked code (dl_iterate_phdr(3) keeps a lock around user callback, but it was added long after libc_r was renounced). Due to this, remembering the level in the global variable after the lock is obtained should be safe, because no two libc_r threads can acquire different write locks in parallel. PR: 215826 Reported by: kami Tested by: yamagi@yamagi.org (previous version) To be reviewed by: kan Sponsored by: The FreeBSD Foundation MFC after: 2 weeks --- libexec/rtld-elf/rtld_lock.c | 51 ++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/libexec/rtld-elf/rtld_lock.c b/libexec/rtld-elf/rtld_lock.c index eeaea5e578b0..c4d2a0b0a7b1 100644 --- a/libexec/rtld-elf/rtld_lock.c +++ b/libexec/rtld-elf/rtld_lock.c @@ -64,7 +64,7 @@ typedef struct Struct_Lock { } Lock; static sigset_t fullsigmask, oldsigmask; -static int thread_flag; +static int thread_flag, wnested; static void * def_lock_create(void) @@ -117,29 +117,34 @@ def_rlock_acquire(void *lock) static void def_wlock_acquire(void *lock) { - Lock *l = (Lock *)lock; - sigset_t tmp_oldsigmask; + Lock *l; + sigset_t tmp_oldsigmask; - for ( ; ; ) { - sigprocmask(SIG_BLOCK, &fullsigmask, &tmp_oldsigmask); - if (atomic_cmpset_acq_int(&l->lock, 0, WAFLAG)) - break; - sigprocmask(SIG_SETMASK, &tmp_oldsigmask, NULL); - } - oldsigmask = tmp_oldsigmask; + l = (Lock *)lock; + for (;;) { + sigprocmask(SIG_BLOCK, &fullsigmask, &tmp_oldsigmask); + if (atomic_cmpset_acq_int(&l->lock, 0, WAFLAG)) + break; + sigprocmask(SIG_SETMASK, &tmp_oldsigmask, NULL); + } + if (atomic_fetchadd_int(&wnested, 1) == 0) + oldsigmask = tmp_oldsigmask; } static void def_lock_release(void *lock) { - Lock *l = (Lock *)lock; + Lock *l; - if ((l->lock & WAFLAG) == 0) - atomic_add_rel_int(&l->lock, -RC_INCR); - else { - atomic_add_rel_int(&l->lock, -WAFLAG); - sigprocmask(SIG_SETMASK, &oldsigmask, NULL); - } + l = (Lock *)lock; + if ((l->lock & WAFLAG) == 0) + atomic_add_rel_int(&l->lock, -RC_INCR); + else { + assert(wnested > 0); + atomic_add_rel_int(&l->lock, -WAFLAG); + if (atomic_fetchadd_int(&wnested, -1) == 1) + sigprocmask(SIG_SETMASK, &oldsigmask, NULL); + } } static int @@ -373,12 +378,12 @@ _rtld_atfork_pre(int *locks) return; /* - * Warning: this does not work with the rtld compat locks - * above, since the thread signal mask is corrupted (set to - * all signals blocked) if two locks are taken in write mode. - * The caller of the _rtld_atfork_pre() must provide the - * working implementation of the locks, and libthr locks are - * fine. + * Warning: this did not worked well with the rtld compat + * locks above, when the thread signal mask was corrupted (set + * to all signals blocked) if two locks were taken + * simultaneously in the write mode. The caller of the + * _rtld_atfork_pre() must provide the working implementation + * of the locks anyway, and libthr locks are fine. */ wlock_acquire(rtld_phdr_lock, &ls[0]); wlock_acquire(rtld_bind_lock, &ls[1]);