From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C04FCC25B08 for ; Tue, 9 Aug 2022 19:45:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344014AbiHITpE (ORCPT ); Tue, 9 Aug 2022 15:45:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45094 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345036AbiHITot (ORCPT ); Tue, 9 Aug 2022 15:44:49 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DCCD826553; Tue, 9 Aug 2022 12:44:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=X6ZKr7fgxCrOJ9tv9cKzUL964WC7Pz5KIRmzI2W7soA=; b=O7rcApswccVM2TyEVknqdGGO8v hl1R92hKtdyVfwS5opk3FiajnAwrUrdev/h941H+VQ+LsR4yCjsgEbwpZJbQImEGH6YghH2ToRv5z o/Pn1jy4xsjDYLdHmVBd+HQ7UCw/bMue4cXeoNS5QcpsZgMd0sZWgrCVMyDqQwbxcUKx60VR9o7FD cuY7b1b4Rin/nWyZYgOcaGumrYqPERJSnnizVPVjw2uPB/6SZgeXsfkB6jtPoMpzN/pQ1VH5k0tNO UEBpX4m+FaXkSY10KQ/7rgFkkzGiUbgKeZFOKRq+PN27QNvMCfMk2LRwbjx6plW7cz0OluexognRQ 7kprafMg==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1oLV9T-00Fn8c-Ka; Tue, 09 Aug 2022 19:44:31 +0000 Date: Tue, 9 Aug 2022 20:44:31 +0100 From: Matthew Wilcox To: Mikulas Patocka Cc: Linus Torvalds , Will Deacon , "Paul E. McKenney" , Ard Biesheuvel , Alexander Viro , Alan Stern , Andrea Parri , Peter Zijlstra , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , Akira Yokosawa , Daniel Lustig , Joel Fernandes , Linux Kernel Mailing List , linux-arch , linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v6] add barriers to buffer_uptodate and set_buffer_uptodate Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Tue, Aug 09, 2022 at 02:32:13PM -0400, Mikulas Patocka wrote: > From: Mikulas Patocka > > Let's have a look at this piece of code in __bread_slow: > get_bh(bh); > bh->b_end_io = end_buffer_read_sync; > submit_bh(REQ_OP_READ, 0, bh); > wait_on_buffer(bh); > if (buffer_uptodate(bh)) > return bh; > Neither wait_on_buffer nor buffer_uptodate contain any memory barrier. > Consequently, if someone calls sb_bread and then reads the buffer data, > the read of buffer data may be executed before wait_on_buffer(bh) on > architectures with weak memory ordering and it may return invalid data. > > Fix this bug by adding a memory barrier to set_buffer_uptodate and an > acquire barrier to buffer_uptodate (in a similar way as > folio_test_uptodate and folio_mark_uptodate). > > Signed-off-by: Mikulas Patocka Reviewed-by: Matthew Wilcox (Oracle) > Cc: stable@vger.kernel.org > > Index: linux-2.6/include/linux/buffer_head.h > =================================================================== > --- linux-2.6.orig/include/linux/buffer_head.h > +++ linux-2.6/include/linux/buffer_head.h > @@ -117,7 +117,6 @@ static __always_inline int test_clear_bu > * of the form "mark_buffer_foo()". These are higher-level functions which > * do something in addition to setting a b_state bit. > */ > -BUFFER_FNS(Uptodate, uptodate) > BUFFER_FNS(Dirty, dirty) > TAS_BUFFER_FNS(Dirty, dirty) > BUFFER_FNS(Lock, locked) > @@ -135,6 +134,30 @@ BUFFER_FNS(Meta, meta) > BUFFER_FNS(Prio, prio) > BUFFER_FNS(Defer_Completion, defer_completion) > > +static __always_inline void set_buffer_uptodate(struct buffer_head *bh) > +{ > + /* > + * make it consistent with folio_mark_uptodate > + * pairs with smp_load_acquire in buffer_uptodate > + */ > + smp_mb__before_atomic(); > + set_bit(BH_Uptodate, &bh->b_state); > +} > + > +static __always_inline void clear_buffer_uptodate(struct buffer_head *bh) > +{ > + clear_bit(BH_Uptodate, &bh->b_state); > +} > + > +static __always_inline int buffer_uptodate(const struct buffer_head *bh) > +{ > + /* > + * make it consistent with folio_test_uptodate > + * pairs with smp_mb__before_atomic in set_buffer_uptodate > + */ > + return (smp_load_acquire(&bh->b_state) & (1UL << BH_Uptodate)) != 0; > +} > + > #define bh_offset(bh) ((unsigned long)(bh)->b_data & ~PAGE_MASK) > > /* If we *know* page->private refers to buffer_heads */ >