All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: linux-kernel@vger.kernel.org
Cc: Matteo Franchin <Matteo.Franchin@arm.com>,
	Davidlohr Bueso <davidlohr@hp.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Darren Hart <dvhart@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Subject: [PATCH] futex: Ensure get_futex_key_refs() always implies a barrier
Date: Fri, 17 Oct 2014 17:38:49 +0100	[thread overview]
Message-ID: <1413563929-2664-1-git-send-email-catalin.marinas@arm.com> (raw)

Commit b0c29f79ecea (futexes: Avoid taking the hb->lock if there's
nothing to wake up) changes the futex code to avoid taking a lock when
there are no waiters. This code has been subsequently fixed in commit
11d4616bd07f (futex: revert back to the explicit waiter counting code).
Both the original commit and the fix-up rely on get_futex_key_refs() to
always imply a barrier.

However, for private futexes, none of the cases in the switch statement
of get_futex_key_refs() would be hit and the function completes without
a memory barrier as required before checking the "waiters" in
futex_wake() -> hb_waiters_pending(). The consequence is a race with a
thread waiting on a futex on another CPU, allowing the waker thread to
read "waiters == 0" while the waiter thread to have read "futex_val ==
locked" (in kernel).

Without this fix, the problem (user space deadlocks) can be seen with
Android bionic's mutex implementation on an arm64 multi-cluster system.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Matteo Franchin <Matteo.Franchin@arm.com>
Fixes: b0c29f79ecea (futexes: Avoid taking the hb->lock if there's nothing to wake up)
Cc: <stable@vger.kernel.org>
Cc: Davidlohr Bueso <davidlohr@hp.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Darren Hart <dvhart@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/futex.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/futex.c b/kernel/futex.c
index 815d7af2ffe8..f3a3a071283c 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -343,6 +343,8 @@ static void get_futex_key_refs(union futex_key *key)
 	case FUT_OFF_MMSHARED:
 		futex_get_mm(key); /* implies MB (B) */
 		break;
+	default:
+		smp_mb(); /* explicit MB (B) */
 	}
 }
 

             reply	other threads:[~2014-10-17 16:38 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-17 16:38 Catalin Marinas [this message]
2014-10-18  6:54 ` [PATCH] futex: Ensure get_futex_key_refs() always implies a barrier Mike Galbraith
2014-10-18  7:09   ` Mike Galbraith
2014-10-18 15:28   ` Linus Torvalds
2014-10-18 16:15     ` Mike Galbraith
2014-10-18  7:33 ` Davidlohr Bueso
2014-10-18 19:58   ` Davidlohr Bueso
2014-10-18 20:50     ` Linus Torvalds
2014-10-19  2:16       ` Davidlohr Bueso
2014-10-20  9:11         ` Thomas Gleixner
2014-10-20 10:49           ` Catalin Marinas
2014-10-20 15:32             ` Linus Torvalds
2014-10-20 16:00               ` Catalin Marinas
2014-10-18 19:32 ` Darren Hart
2014-10-18 20:19   ` Davidlohr Bueso
2014-10-18 20:25     ` Darren Hart
2014-10-20 10:15     ` Catalin Marinas
2014-10-24  3:27 ` [PATCH] futex: Mention key referencing differences between shared and private futexes Davidlohr Bueso
2014-10-24  9:11   ` Catalin Marinas
2014-10-26 15:22   ` [tip:locking/urgent] " tip-bot for Davidlohr Bueso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1413563929-2664-1-git-send-email-catalin.marinas@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=Matteo.Franchin@arm.com \
    --cc=davidlohr@hp.com \
    --cc=dvhart@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.