pthread_mutex_unlock potentially cause invalid access

[Jakub Jelinek]

On Mon, Feb 13, 2012 at 09:39:34AM -0500, Carlos O'Donell wrote:
> > Â  Â struct obj {
> > Â  Â pthread_mutex_t om;
> > Â  Â  Â  Â int refcnt;
> > Â  Â  Â  Â ...
> > Â  Â };
> >
> > Â  Â obj_done(struct obj *op)
> > Â  Â {
> > Â  Â  Â  Â pthread_mutex_lock(&op->om);
> > Â  Â  Â  Â if (--op->refcnt == 0) {
> > Â  Â  Â  Â  Â  Â pthread_mutex_unlock(&op->om);
> > Â  Â (A) Â  Â  pthread_mutex_destroy(&op->om);
> > Â  Â (B) Â  Â  free(op);
> > Â  Â  Â  Â } else
> > Â  Â (C) Â  Â  pthread_mutex_unlock(&op->om);
> > Â  Â }
> >
> > Â  Â In this case obj is reference counted and obj_done() is called
> > Â  Â whenever a reference to the object is dropped. Implementations are
> > Â  Â required to allow an object to be destroyed and freed and potentially
> > Â  Â unmapped (for example, lines A and B) immediately after the object is
> > Â  Â unlocked (line C).
> > ------------------------------------------------------------------------
> >
> > In this example, (A) and (B) can be executed in middle of (C) execution.
> > Thus, problem can be happen even on a fully conforming program, no?
> 
> No. (A) and (B) can not be executed in the middle of (C).

It can, the reporter is correct, the bug can be either fixed by ensuring in
the generic code (pthread_mutex_*lock.c etc.) that
lll_unlock/lll_robust_unlock is always
called with either constant or some automatic variable as second argument
(i.e. initialize a temporary from the currently used macros, then pass that
down), or by adjusting all target's lll_unlock/lll_robust_unlock macros,
or by turning them into inline functions.  The atomic operation will act as
a compiler/CPU barrier, so all that is needed is to make sure the read
in the source doesn't happen after the sync operation.

	Jakub