From c21f750727515602a9e84f2a190ee8a0a2aeb2a1 Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Tue, 17 Apr 2018 23:59:41 -0400
Subject: 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(-)

(limited to 'src')

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 <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;
-- 
cgit v1.2.1