Re: BZ 20822 :powerpc: race condition in __lll_unlock_elision

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 16/11/2016 12:07, Szabolcs Nagy wrote:
> On 16/11/16 13:40, Adhemerval Zanella wrote:
>> "[...]  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. [...]"
>>
>> It seems *after* thread B destroy the mutex, it was not clear that
>> the program just calls 'pthread_mutex_destroy' or if the thread 
>> deallocates the mutex on the stack (by returning or something else).
>> First option should not trigger an invalid access, but second is 
>> definitively an program error.
>>
> 
> does not look like a program error to me:
> 
> int i=0;
> 
> void threadB()
> {
> 	pthread_mutex_t m[1];
> 	//...
> 	pthread_mutex_lock(m);
> 	if (i==1) {
> 		pthread_mutex_unlock(m);
> 		pthread_mutex_destroy(m,0); // ok A is done with m
> 		return; // drop stack frame
> 	}
> 	//...
> }
> 
> void threadA()
> {
> 	// get m from thread B
> 	pthread_mutex_lock(m);
> 	i=1;
> 	pthread_mutex_unlock(m);
> 	// no more use of m here
> }
> 
> this code can corrupt the stack of threadB if pthread_mutex_unlock
> in threadA accesses m->adapt_count after m is unlocked.
> 
> (same for rwlocks and whatever else may be used with elision.)

If this the scenario described in bug report it could the case of
the issue.  I was thinking in another scenario, where mutex unlock
was not synchronized with a global variable.

It could be case where a similar code you described unlocks the
mutex on threadA, threadB run and finishes, and then threadA
runs again an access invalid memory.

Anyway, I see the correct fix to just just move the adapt_count
to inside the transaction.