From c21f750727515602a9e84f2a190ee8a0a2aeb2a1 Mon Sep 17 00:00:00 2001 From: Rich Felker Date: Tue, 17 Apr 2018 23:59:41 -0400 Subject: [PATCH] fix stdio lock dependency on read-after-free not faulting instead of using a waiters count, add a bit to the lock field indicating that the lock may have waiters. threads which obtain the lock after contending for it will perform a potentially-spurious wake when they release the lock. --- src/stdio/__lockfile.c | 29 +++++++++++++---------------- src/stdio/flockfile.c | 6 +++++- src/stdio/ftrylockfile.c | 9 ++++++--- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/stdio/__lockfile.c b/src/stdio/__lockfile.c index 9d967d6e..2ff75d8a 100644 --- a/src/stdio/__lockfile.c +++ b/src/stdio/__lockfile.c @@ -1,28 +1,25 @@ #include "stdio_impl.h" #include "pthread_impl.h" +#define MAYBE_WAITERS 0x40000000 + int __lockfile(FILE *f) { - int owner, tid = __pthread_self()->tid; - if (f->lock == tid) + int owner = f->lock, tid = __pthread_self()->tid; + if ((owner & ~MAYBE_WAITERS) == tid) return 0; - while ((owner = a_cas(&f->lock, 0, tid))) - __wait(&f->lock, &f->waiters, owner, 1); + for (;;) { + owner = a_cas(&f->lock, 0, tid); + if (!owner) return 1; + if (a_cas(&f->lock, owner, owner|MAYBE_WAITERS)==owner) break; + } + while ((owner = a_cas(&f->lock, 0, tid|MAYBE_WAITERS))) + __futexwait(&f->lock, owner, 1); return 1; } void __unlockfile(FILE *f) { - a_store(&f->lock, 0); - - /* The following read is technically invalid under situations - * of self-synchronized destruction. Another thread may have - * called fclose as soon as the above store has completed. - * Nonetheless, since FILE objects always live in memory - * obtained by malloc from the heap, it's safe to assume - * the dereferences below will not fault. In the worst case, - * a spurious syscall will be made. If the implementation of - * malloc changes, this assumption needs revisiting. */ - - if (f->waiters) __wake(&f->lock, 1, 1); + if (a_swap(&f->lock, 0) & MAYBE_WAITERS) + __wake(&f->lock, 1, 1); } diff --git a/src/stdio/flockfile.c b/src/stdio/flockfile.c index a196c1ef..6b574cf0 100644 --- a/src/stdio/flockfile.c +++ b/src/stdio/flockfile.c @@ -1,10 +1,14 @@ #include "stdio_impl.h" #include "pthread_impl.h" +#define MAYBE_WAITERS 0x40000000 + void flockfile(FILE *f) { while (ftrylockfile(f)) { int owner = f->lock; - if (owner) __wait(&f->lock, &f->waiters, owner, 1); + if (!owner) continue; + a_cas(&f->lock, owner, owner|MAYBE_WAITERS); + __futexwait(&f->lock, owner|MAYBE_WAITERS, 1); } } diff --git a/src/stdio/ftrylockfile.c b/src/stdio/ftrylockfile.c index eb13c839..3b1d5f20 100644 --- a/src/stdio/ftrylockfile.c +++ b/src/stdio/ftrylockfile.c @@ -2,6 +2,8 @@ #include "pthread_impl.h" #include +#define MAYBE_WAITERS 0x40000000 + void __do_orphaned_stdio_locks() { FILE *f; @@ -22,14 +24,15 @@ int ftrylockfile(FILE *f) { pthread_t self = __pthread_self(); int tid = self->tid; - if (f->lock == tid) { + int owner = f->lock; + if ((owner & ~MAYBE_WAITERS) == tid) { if (f->lockcount == LONG_MAX) return -1; f->lockcount++; return 0; } - if (f->lock < 0) f->lock = 0; - if (f->lock || a_cas(&f->lock, 0, tid)) + if (owner < 0) f->lock = owner = 0; + if (owner || a_cas(&f->lock, 0, tid)) return -1; f->lockcount = 1; f->prev_locked = 0; -- 2.25.1