diff mbox

Fix lll_unlock twice in pthread_cond_broadcast

Message ID 1398850798-5648-1-git-send-email-yangyingliang@huawei.com
State Committed
Headers

Commit Message

Yang Yingliang April 30, 2014, 9:39 a.m. UTC 
  lll_unlock() will be called again if it goes to "wake_all"
in pthread_cond_broadcast(). This may make another thread
which is waiting for lock in pthread_cond_timedwait() unlock.
So there are more than one threads get the lock, it will break
the shared data.

It's introduced by commit 8313cb997d2d("FUTEX_*_REQUEUE_PI support for non-x86 code")
---
 nptl/pthread_cond_broadcast.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Siddhesh Poyarekar April 30, 2014, 10:43 a.m. UTC | #1 
On Wed, Apr 30, 2014 at 05:39:58PM +0800, Yang Yingliang wrote:
> lll_unlock() will be called again if it goes to "wake_all"
> in pthread_cond_broadcast(). This may make another thread
> which is waiting for lock in pthread_cond_timedwait() unlock.
> So there are more than one threads get the lock, it will break
> the shared data.
> 
> It's introduced by commit 8313cb997d2d("FUTEX_*_REQUEUE_PI support
> for non-x86 code")

Thanks for the patch.  It looks good to me and is quite small (and
hence won't need a copyright assignment), so I'll pushed it.  For any
significant changes you intend to post in future, please consider
assigning copyright of your contributions in glibc to the FSF.

Thanks,
Siddhesh

> ---
>  nptl/pthread_cond_broadcast.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/nptl/pthread_cond_broadcast.c b/nptl/pthread_cond_broadcast.c
> index ed30e7c60bc7..7c6c9ea9a2d5 100644
> --- a/nptl/pthread_cond_broadcast.c
> +++ b/nptl/pthread_cond_broadcast.c
> @@ -81,6 +81,7 @@ __pthread_cond_broadcast (cond)
>  
>  wake_all:
>        lll_futex_wake (&cond->__data.__futex, INT_MAX, pshared);
> +      return 0;
>      }
>  
>    /* We are done.  */
> -- 
> 1.8.0
> 
>
Florian Weimer Feb. 18, 2015, 2:36 p.m. UTC | #2 
On 04/30/2014 11:39 AM, Yang Yingliang wrote:
> lll_unlock() will be called again if it goes to "wake_all"
> in pthread_cond_broadcast(). This may make another thread
> which is waiting for lock in pthread_cond_timedwait() unlock.
> So there are more than one threads get the lock, it will break
> the shared data.
> 
> It's introduced by commit 8313cb997d2d("FUTEX_*_REQUEUE_PI support for non-x86 code")

Is there a bug for this?

Lack of mutual exclusion could be a potential security issue.  Can this
happen with regular mutexes/condition variables?
diff mbox

Patch

diff --git a/nptl/pthread_cond_broadcast.c b/nptl/pthread_cond_broadcast.c
index ed30e7c60bc7..7c6c9ea9a2d5 100644
--- a/nptl/pthread_cond_broadcast.c
+++ b/nptl/pthread_cond_broadcast.c
@@ -81,6 +81,7 @@  __pthread_cond_broadcast (cond)
 
 wake_all:
       lll_futex_wake (&cond->__data.__futex, INT_MAX, pshared);
+      return 0;
     }
 
   /* We are done.  */