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.

diff --git a/src/stdio/__lockfile.c b/src/stdio/__lockfile.c
index 9d967d6eb8860c0522b9fcd8505d6e52fad19b6b..2ff75d8add3cea9da761d7a5290e2c20bf0d900a 100644 (file)
--- 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 a196c1efe0af11663f5e0537a0de1db02c3982ef..6b574cf0905f8ac4bd58458c2730d5e79b739634 100644 (file)
--- 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 eb13c839bf818a33a8e54016ae962785f1423726..3b1d5f20fdcc9ef1af84990c32a5464ef31c0f51 100644 (file)
--- a/src/stdio/ftrylockfile.c
+++ b/src/stdio/ftrylockfile.c
@@ -2,6 +2,8 @@
 #include "pthread_impl.h"
 #include <limits.h>
 
+#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;