linux-kernel - Re: [patch] futex: Fix v4.6+ ltp futex

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 7 Jun 2016 11:50:56 +0100
From:	Mel Gorman <mgorman@...e.de>
To:	Mike Galbraith <umgwanakikbuti@...il.com>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	dbueso@...e.de
Subject: Re: [patch] futex: Fix v4.6+ ltp futex_wait04 regression

On Mon, Jun 06, 2016 at 11:40:41AM +0200, Mike Galbraith wrote:
> Hi Mel (who is out of the office today),
> 

Adding Davidlohr who did a lot of the work debugging my initial
prototype.

> I initially reported this on the rt-users list, thinking at the time
> that it was only showing up in -rt kernels, but that turned out to not
> be the case, it just requires an enterprise config for some reason mm
> folks will likely recognize instantly.  I just happen to use same when
> building devel -rt trees to at least get some build warning coverage.
> 

Can you post the exact config? I used an enterprise config and was unable
to reproduce the problem on 4.6. I'll be trying 4.7-rc1 after finishing
this mail even though the optimisation was introduced before 4.6.

> Below is a stab at fixing the thing up while you're off doing whatever
> an Irishman does on a national holiday (hm;).  If it's ok as is, fine,
> I saved you a couple minutes.  If not, oh well, consider it diagnosis.
> 
> 65d8fc77 futex: Remove requirement for lock_page() in get_futex_key()
> introduced a regression of futex_wait04 when an enterprise size config is
> used.  Per trace_printk(), when we assign page = compound_head(page), we're
> subsequently using a different page than the one we locked into memory,
> thus mucking up references.
> 
> Fixes: 65d8fc77 futex: Remove requirement for lock_page() in get_futex_key()
> Signed-off-by: Mike Galbraith <umgwanakikbuit@...il.com>

As I cannot reproduce the problem yet, I cannot see the impact of the
fix but I'm scratching my head trying to figure out why this would work
and be explained by 65d8fc77. The primary path you alter covers the case
where a filesystem is backing the page containing the futex. If this was a
transparent hugepage anonymous mapping, it would be handled by the PageAnon
path so I'm ruling that out. The only case where a filesystem is using a
compound page is hugetlbfs and indeed the test case uses MAP_HUGETLB. The
compound state of such pages are stable between futex invocations as they
cannot split and the basepage_index should be returning the key for the
head page for hugetlbfs pages.

-- 
Mel Gorman
SUSE Labs