This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: BZ 20822 :powerpc: race condition in __lll_unlock_elision
- From: Torvald Riegel <triegel at redhat dot com>
- To: Rajalakshmi Srinivasaraghavan <raji at linux dot vnet dot ibm dot com>
- Cc: libc-alpha at sourceware dot org, aaron Sawdey <acsawdey at linux dot vnet dot ibm dot com>, Ulrich Weigand <Ulrich dot Weigand at de dot ibm dot com>, Steve Munroe <sjmunroe at us dot ibm dot co>, carlos at redhat dot com, adhemerval dot zanella at linaro dot org, adconrad at ubuntu dot com, wschmidt at linux dot vnet dot ibm dot com
- Date: Thu, 17 Nov 2016 15:46:50 +0100
- Subject: Re: BZ 20822 :powerpc: race condition in __lll_unlock_elision
- Authentication-results: sourceware.org; auth=none
- References: <f777eebe-4a7b-87d2-1281-ab605c7fce8b@linux.vnet.ibm.com>
On Tue, 2016-11-15 at 21:42 +0530, Rajalakshmi Srinivasaraghavan wrote:
> The initial problem reported was memory corruption in MongoDB only on
> Ubuntu 16.04 ppc64le(not on Ubuntu 15).
> The problem was not easily recreatable and debugging with many tools and
> creative ideas, the problem is narrowed down to lock elision in glibc.
> Ubuntu 16.04 has glibc 2.23 with lock elision enabled by default which
> made the testcase fail only on 16.04.
>
> As stated in BZ 20822, short description is
>
> "The update of *adapt_count after the release of the lock causes a race
> condition. Thread A unlocks, thread B continues and returns, destroying
> mutex on stack,
> then gets into another function, thread A writes to *adapt_count and
> corrupts stack.The window is very narrow and requires that the
> machine be in smt mode, so likely the two threads have to be on the same
> core in order to hit this"
>
> Looking back the file changes in
> sysdeps/unix/sysv/linux/powerpc/elision-unlock.c, suspecting the commit
> https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=fadd2ad9cc36115440d50b0eae9299e65988917d which is introduced to improve
> the performance.
>
> Thinking of the following fixes.
>
> 1) Update of adapt_count before the unlock. But I have still not
> identified if its going to affect the performance.
Because of the requirements on mutex destruction (and same for rwlock
etc.), a thread must not access any data associated with the mutex after
having unlocked it (this includes reads because it's correct to also
unmap the memory holding the mutex after destroying the mutex).
> 2) In elision-lock.c/elision-trylock.c update adapt_count right after
> the lock acquire at the end of the function.
> In either case we have to be careful not to introduce another race
> condition.
Every access to mutex data outside of transactions must use atomic
operations if there would be data races otherwise (according to the C11
memory model). I know that this isn't followed in the current mutex
code, but it really should be, and your change qualifies as new code so
please take care of this too.
I also think that if we can have concurrent accesses to a particular
variable both inside and outside of transactions, we should use atomic
accesses inside the transactions too, just for consistency.