linux-kernel - Re: [PATCH] futex: Remove unnecessary warning from get_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 for Android: free password hash cracker in your pocket

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

Message-ID: <20170809150834.ostlfg3kw2ecwlgf@hirez.programming.kicks-ass.net>
Date:   Wed, 9 Aug 2017 17:08:34 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Mel Gorman <mgorman@...e.de>
Cc:     Mark Rutland <mark.rutland@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        Davidlohr Bueso <dbueso@...e.de>,
        Hugh Dickins <hughd@...gle.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] futex: Remove unnecessary warning from get_futex_key

On Wed, Aug 09, 2017 at 03:43:09PM +0100, Mel Gorman wrote:
> On Wed, Aug 09, 2017 at 03:05:19PM +0100, Mark Rutland wrote:
> > > diff --git a/kernel/futex.c b/kernel/futex.c
> > > index 16dbe4c93895..f50b434756c1 100644
> > > --- a/kernel/futex.c
> > > +++ b/kernel/futex.c
> > > @@ -670,13 +670,14 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
> > >  		 * this reference was taken by ihold under the page lock
> > >  		 * pinning the inode in place so i_lock was unnecessary. The
> > >  		 * only way for this check to fail is if the inode was
> > > -		 * truncated in parallel so warn for now if this happens.
> > > +		 * truncated in parallel which is almost certainly an
> > > +		 * application bug. In such a case, just retry.
> > >  		 *
> > >  		 * We are not calling into get_futex_key_refs() in file-backed
> > >  		 * cases, therefore a successful atomic_inc return below will
> > >  		 * guarantee that get_futex_key() will still imply smp_mb(); (B).
> > >  		 */
> > > -		if (WARN_ON_ONCE(!atomic_inc_not_zero(&inode->i_count))) {
> > > +		if (!atomic_inc_not_zero(&inode->i_count)) {
> > 
> > I applied the same diff yesterday, and haven't seen anything go wrong
> > with my test case and/or with Syzkaller running, so FWIW:
> > 
> > Tested-by: Mark Rutland <mark.rutland@....com>
> > 
> > Thanks for putting this together!
> > 
> 
> No problem. FWIW, I had the test case running for 12 hours in a loop as
> well and other than having to adjust the number of threads doing futex()
> to trigger the warning without the patch, I observed no other problems.
> If Thomas is happy, I hope this can be merged for 4.13 (or picked up
> directly by Linus if he feels like it). Even if it's delayed, I'll resubmit
> to -stable manually if the "Cc: stable" gets stripped along the way.

Probably best if Linus picks this up directly as Thomas is on holidays.

In any case,

Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>