From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752905AbdHIOGb (ORCPT ); Wed, 9 Aug 2017 10:06:31 -0400 Received: from foss.arm.com ([217.140.101.70]:49338 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751904AbdHIOG3 (ORCPT ); Wed, 9 Aug 2017 10:06:29 -0400 Date: Wed, 9 Aug 2017 15:05:19 +0100 From: Mark Rutland To: Mel Gorman Cc: Thomas Gleixner , Linus Torvalds , Ingo Molnar , Davidlohr Bueso , Hugh Dickins , Peter Zijlstra , Sebastian Andrzej Siewior , linux-kernel@vger.kernel.org Subject: Re: [PATCH] futex: Remove unnecessary warning from get_futex_key Message-ID: <20170809140519.GE29494@leverpostej> References: <20170809072711.np2ayrasrogph5tm@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170809072711.np2ayrasrogph5tm@suse.de> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Aug 09, 2017 at 08:27:11AM +0100, Mel Gorman wrote: > Commit 65d8fc777f6d ("futex: Remove requirement for lock_page() in > get_futex_key()") removed an unnecessary lock_page() with the side-effect > that page->mapping needed to be treated very carefully. Two defensive > warnings were added in case any assumption was missed and the first warning > assumed a correct application would not alter a mapping backing a futex key. > Since merging, it has not triggered for any unexpected case but Mark > Rutland reported the following bug triggering due to the first warning. [...] > Reported-by: Mark Rutland > Signed-off-by: Mel Gorman > Cc: stable@vger.kernel.org # 4.7+ > --- > kernel/futex.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > 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 Thanks for putting this together! Mark.