Bug 10418 - pthread_mutex_unlock may use acquire rather than release barriers
Summary: pthread_mutex_unlock may use acquire rather than release barriers
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: nptl (show other bugs)
Version: 2.10
: P1 critical
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-20 16:42 UTC by Chris Friesen
Modified: 2014-07-01 07:44 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Friesen 2009-07-20 16:42:56 UTC
When priority inheritance or priority protection is enabled,
pthread_mutex_unlock() calls atomic_compare_and_exchange_bool_acq() rather than
atomic_compare_and_exchange_bool_rel().

On mips (and possibly other architectures), this results in a memory barrier
after the atomic operation instead of before it.  This means that there is a
window where another core can do pthread_mutex_lock() and read stale information.

We have a testcase that reproduces the problem.  Changing to
atomic_compare_and_exchange_bool_rel() makes the problem go away.
Comment 1 Ulrich Drepper 2009-07-26 20:01:38 UTC
I've checked in a patch.
Comment 2 Chris Friesen 2009-07-27 17:03:29 UTC
I'm confused.  The change you've submitted is exactly opposite to what I
proposed.  The new code now has release semantics on the lock, and acquire
semantics on the unlock, which is backwards from my understanding of what they
should be.

The comment in the patch is "All commits should have happened before the mutex
lock is taken. Therefore use the _rel variant of the cmpxchg atomic op."

We're trying to ensure that writes which happen while holding the lock on one
cpu are visible to another cpu which acquires the lock.  This means that the
barrier must come before the atomic operation which releases the lock, and after
the atomic operation which acquires the lock.  I think the correct comment
should be "All commits should have happened before the mutex lock is freed,
therefore use the _rel variant of the cmpxchg atomic op."

Comment 3 Ulrich Drepper 2009-08-01 03:44:16 UTC
This is changed.
Comment 4 Chris Friesen 2009-08-01 04:17:07 UTC
looks good, thanks.