linux-kernel - [tip:locking/urgent] futex: Calculate the futex key based on a tail page for file-based futexes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <tip-077fa7aed17de5022e44bf07dbaf732078b7b5b2@git.kernel.org>
Date:	Wed, 8 Jun 2016 10:27:50 -0700
From:	tip-bot for Mel Gorman <tipbot@...or.com>
To:	linux-tip-commits@...r.kernel.org
Cc:	mgorman@...hsingularity.net, bigeasy@...utronix.de, hpa@...or.com,
	dave@...olabs.net, peterz@...radead.org, mingo@...nel.org,
	umgwanakikbuti@...il.com, linux-kernel@...r.kernel.org,
	mgorman@...e.de, tglx@...utronix.de
Subject: [tip:locking/urgent] futex: Calculate the futex key based on a tail
 page for file-based futexes

Commit-ID:  077fa7aed17de5022e44bf07dbaf732078b7b5b2
Gitweb:     http://git.kernel.org/tip/077fa7aed17de5022e44bf07dbaf732078b7b5b2
Author:     Mel Gorman <mgorman@...e.de>
AuthorDate: Wed, 8 Jun 2016 14:25:22 +0100
Committer:  Thomas Gleixner <tglx@...utronix.de>
CommitDate: Wed, 8 Jun 2016 19:23:54 +0200

futex: Calculate the futex key based on a tail page for file-based futexes

Mike Galbraith reported that the LTP test case futex_wake04 was broken
by commit 65d8fc777f6d ("futex: Remove requirement for lock_page()
in get_futex_key()").

This test case uses futexes backed by hugetlbfs pages and so there is an
associated inode with a futex stored on such pages. The problem is that
the key is being calculated based on the head page index of the hugetlbfs
page and not the tail page.

Prior to the optimisation, the page lock was used to stabilise mappings and
pin the inode is file-backed which is overkill. If the page was a compound
page, the head page was automatically looked up as part of the page lock
operation but the tail page index was used to calculate the futex key.

After the optimisation, the compound head is looked up early and the page
lock is only relied upon to identify truncated pages, special pages or a
shmem page moving to swapcache. The head page is looked up because without
the page lock, special care has to be taken to pin the inode correctly.
However, the tail page is still required to calculate the futex key so
this patch records the tail page.

On vanilla 4.6, the output of the test case is;

futex_wake04    0  TINFO  :  Hugepagesize 2097152
futex_wake04    1  TFAIL  :  futex_wake04.c:126: Bug: wait_thread2 did not wake after 30 secs.

With the patch applied

futex_wake04    0  TINFO  :  Hugepagesize 2097152
futex_wake04    1  TPASS  :  Hi hydra, thread2 awake!

Fixes: 65d8fc777f6d "futex: Remove requirement for lock_page() in get_futex_key()"
Reported-and-tested-by: Mike Galbraith <umgwanakikbuti@...il.com>
Signed-off-by: Mel Gorman <mgorman@...hsingularity.net>
Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>
Reviewed-by: Davidlohr Bueso <dave@...olabs.net>
Cc: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: stable@...r.kernel.org
Link: http://lkml.kernel.org/r/20160608132522.GM2469@suse.de
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>

---
 kernel/futex.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index ee25f5b..33664f7 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -469,7 +469,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
 {
 	unsigned long address = (unsigned long)uaddr;
 	struct mm_struct *mm = current->mm;
-	struct page *page;
+	struct page *page, *tail;
 	struct address_space *mapping;
 	int err, ro = 0;
 
@@ -530,7 +530,15 @@ again:
 	 * considered here and page lock forces unnecessarily serialization
 	 * From this point on, mapping will be re-verified if necessary and
 	 * page lock will be acquired only if it is unavoidable
-	 */
+	 *
+	 * Mapping checks require the head page for any compound page so the
+	 * head page and mapping is looked up now. For anonymous pages, it
+	 * does not matter if the page splits in the future as the key is
+	 * based on the address. For filesystem-backed pages, the tail is
+	 * required as the index of the page determines the key. For
+	 * base pages, there is no tail page and tail == page.
+	 */
+	tail = page;
 	page = compound_head(page);
 	mapping = READ_ONCE(page->mapping);
 
@@ -654,7 +662,7 @@ again:
 
 		key->both.offset |= FUT_OFF_INODE; /* inode-based key */
 		key->shared.inode = inode;
-		key->shared.pgoff = basepage_index(page);
+		key->shared.pgoff = basepage_index(tail);
 		rcu_read_unlock();
 	}