On 09/26/2015 09:52 PM, Jonathan Wakely wrote:

> Would changes like this be suitable for _FORTIFY_SOURCE?

> diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
> index 5e5ced1..074bf26 100644
> --- a/libstdc++-v3/include/std/mutex
> +++ b/libstdc++-v3/include/std/mutex
> @@ -70,7 +70,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      __recursive_mutex_base& operator=(const __recursive_mutex_base&) = 
> delete;
>  
>  #ifdef __GTHREAD_RECURSIVE_MUTEX_INIT
> +# if _GLIBCXX_ASSERTIONS && defined(PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP)
> +    // Use an error-checking mutex type when assertions are enabled.
> +    __native_type  _M_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
> +# else
>      __native_type  _M_mutex = __GTHREAD_RECURSIVE_MUTEX_INIT;
> +# endif

I think this is incorrect.

If you try to lock an error-checking mutex recursively, the operation
fails, and it does *not* increment the internal lock counter (the mutex
may not even have one).  This means a subsequent unlock operation will
release the mutex too early.

The trylock will be have differently, too.

POSIX recursive mutexes are already error-checking in that sense
(self-deadlock cannot happen, and an unlock when not lock is defined to
return an error), so I don't think anything like that is even needed.

-- 
Florian Weimer / Red Hat Product Security

Reply via email to