This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] pthread_cond_wait: close window against priority inversion
- From: Steven Walter <stevenrwalter at gmail dot com>
- To: libc-alpha at sourceware dot org
- Cc: Steven Walter <swalter at lpdev dot prtdev dot lexmark dot com>
- Date: Fri, 24 Sep 2010 21:42:14 -0400
- Subject: [PATCH] pthread_cond_wait: close window against priority inversion
From: Steven Walter <swalter@lpdev.prtdev.lexmark.com>
Consider the situation where you have a condvar that is used in
conjunction with a priority-inheriting mutex. When you call
pthread_cond_wait, the priority-inheriting mutex gets dropped, but not
before we grab the internal condvar lock. If the PI mutex was
contended, we will get preempted with no PI protection. If another
thread then calls pthread_cond_wait, it will block on the internal condvar
lock, while holding the PI mutex. We are now priority inverted,
dependent on the scheduler to run the low-priority holder of the condvar
lock.
Fortunately, it seems that we can simply hold the PI lock for longer (in
fact, for the entire time we hold the internal condvar lock) to protect
against this window.
2010-09-24 Steven Walter <stevenrwalter@gmail.com>
* nptl/pthread_cond_wait.c (__pthread_cond_wait): Hold
priority-inheriting mutexes until after dropping
cond->__data.__lock. Holding the PI mutex longer prevents
priority-inversion from occurring in __lock, which is not
priority-inheriting.
---
nptl/pthread_cond_wait.c | 29 +++++++++++++++++++++++++----
1 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 670fba5..71a3273 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -98,6 +98,7 @@ __pthread_cond_wait (cond, mutex)
struct _pthread_cleanup_buffer buffer;
struct _condvar_cleanup_buffer cbuffer;
int err;
+ int unlock = 1;
int pshared = (cond->__data.__mutex == (void *) ~0l)
? LLL_SHARED : LLL_PRIVATE;
@@ -105,11 +106,18 @@ __pthread_cond_wait (cond, mutex)
lll_lock (cond->__data.__lock, pshared);
/* Now we can release the mutex. */
- err = __pthread_mutex_unlock_usercnt (mutex, 0);
- if (__builtin_expect (err, 0))
+ /* SRW: we only want to release the mutex now if it is not
+ * priority-inheriting. If it *is* priority inheriting, we want to release
+ * it at the last possible moment so that we remain protected against PI
+ * for as long as possible */
+ if (!(mutex->__data.__kind & PTHREAD_MUTEX_PRIO_INHERIT_NP))
{
- lll_unlock (cond->__data.__lock, pshared);
- return err;
+ err = __pthread_mutex_unlock_usercnt (mutex, 0);
+ if (__builtin_expect (err, 0))
+ {
+ lll_unlock (cond->__data.__lock, pshared);
+ return err;
+ }
}
/* We have one new user of the condvar. */
@@ -146,6 +154,19 @@ __pthread_cond_wait (cond, mutex)
/* Prepare to wait. Release the condvar futex. */
lll_unlock (cond->__data.__lock, pshared);
+ /* SRW: go ahead and release the PI mutex now that we're no longer
+ * holding the condvar's mutex. */
+ if ((mutex->__data.__kind & PTHREAD_MUTEX_PRIO_INHERIT_NP) && unlock)
+ {
+ unlock = 0;
+ err = __pthread_mutex_unlock_usercnt (mutex, 0);
+ if (__builtin_expect (err, 0))
+ {
+ lll_unlock (cond->__data.__lock, pshared);
+ return err;
+ }
+ }
+
/* Enable asynchronous cancellation. Required by the standard. */
cbuffer.oldtype = __pthread_enable_asynccancel ();
--
1.7.3.4.g4d78d