pthread_mutex_unlock potentially cause invalid access

pthread_mutex_unlock potentially cause invalid access

[Atsushi Nemoto]

It seems pthread_mutex_unlock() potentially cause invalid access on
most platforms (except for i386 and x86_64).

In nptl/pthread_mutex_unlock.c, lll_unlock() is called like this:
      lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));

And PTHREAD_MUTEX_PSHARED() is defined like this:
# define PTHREAD_MUTEX_PSHARED(m) \
  ((m)->__data.__kind & 128)

On most platforms, lll_unlock() is defined as a macro like this:
#define lll_unlock(lock, private) \
  ((void) ({						      \
    int *__futex = &(lock);				      \
    int __val = atomic_exchange_rel (__futex, 0);	      \
    if (__builtin_expect (__val > 1, 0))		      \
      lll_futex_wake (__futex, 1, private);		      \
  }))

Thus, the lll_unlock() call in pthread_mutex_unlock.c will be expanded as:
    int *__futex = &(mutex->__data.__lock);
    int __val = atomic_exchange_rel (__futex, 0);
    if (__builtin_expect (__val > 1, 0))		/* A */
      lll_futex_wake (__futex, 1, ((mutex)->__data.__kind & 128)); /* B */

On point "A", the mutex is actually unlocked, so other threads can
lock the mutex, unlock, destroy and free.  If the mutex was destroyed
and freed by other thread, reading '__kind' on point "B" is not valid.

Possible fix would be copying the 'private' argument to an internal
local variable before atomic_exchange_rel().  Is it an appropriate fix?

---
Atsushi Nemoto

Re: pthread_mutex_unlock potentially cause invalid access

[Roland McGrath]

> Possible fix would be copying the 'private' argument to an internal
> local variable before atomic_exchange_rel().  Is it an appropriate fix?

That seems right to me off hand.  But I'm not really the expert on the
subtleties of this code.


Thanks,
Roland

pthread_mutex_unlock potentially cause invalid access

[Atsushi Nemoto]

It seems pthread_mutex_unlock() potentially cause invalid access on
most platforms (except for i386 and x86_64).

# Resend with correct ML address.  Excuse me for duplication.

In nptl/pthread_mutex_unlock.c, lll_unlock() is called like this:
      lll_unlock (mutex->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex));

And PTHREAD_MUTEX_PSHARED() is defined like this:
# define PTHREAD_MUTEX_PSHARED(m) \
  ((m)->__data.__kind & 128)

On most platforms, lll_unlock() is defined as a macro like this:
#define lll_unlock(lock, private) \
  ((void) ({						      \
    int *__futex = &(lock);				      \
    int __val = atomic_exchange_rel (__futex, 0);	      \
    if (__builtin_expect (__val > 1, 0))		      \
      lll_futex_wake (__futex, 1, private);		      \
  }))

Thus, the lll_unlock() call in pthread_mutex_unlock.c will be expanded as:
    int *__futex = &(mutex->__data.__lock);
    int __val = atomic_exchange_rel (__futex, 0);
    if (__builtin_expect (__val > 1, 0))		/* A */
      lll_futex_wake (__futex, 1, ((mutex)->__data.__kind & 128)); /* B */

On point "A", the mutex is actually unlocked, so other threads can
lock the mutex, unlock, destroy and free.  If the mutex was destroyed
and freed by other thread, reading '__kind' on point "B" is not valid.

Possible fix would be copying the 'private' argument to an internal
local variable before atomic_exchange_rel().  Is it an appropriate fix?

---
Atsushi Nemoto