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 28816C19F2C for ; Mon, 1 Aug 2022 00:27:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229710AbiHAA14 (ORCPT ); Sun, 31 Jul 2022 20:27:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48512 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233233AbiHAA1y (ORCPT ); Sun, 31 Jul 2022 20:27:54 -0400 Received: from netrider.rowland.org (netrider.rowland.org [192.131.102.5]) by lindbergh.monkeyblade.net (Postfix) with SMTP id CDA02D124 for ; Sun, 31 Jul 2022 17:27:52 -0700 (PDT) Received: (qmail 554189 invoked by uid 1000); 31 Jul 2022 20:27:51 -0400 Date: Sun, 31 Jul 2022 20:27:51 -0400 From: Alan Stern To: Mikulas Patocka Cc: Linus Torvalds , Will Deacon , "Paul E. McKenney" , Ard Biesheuvel , Alexander Viro , 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 v3 1/2] wait_bit: do read barrier after testing a bit 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 Sun, Jul 31, 2022 at 04:40:59PM -0400, Mikulas Patocka wrote: > wait_on_bit tests the bit without any memory barriers, consequently the > code that follows wait_on_bit may be moved before testing the bit on > architectures with weak memory ordering. When the code tests for some > event using wait_on_bit and then performs a load operation, the load may > be unexpectedly moved before wait_on_bit and it may return data that > existed before the event occurred. > > Such bugs exist in fs/buffer.c:__wait_on_buffer, > drivers/md/dm-bufio.c:new_read, > drivers/media/usb/dvb-usb-v2/dvb_usb_core.c:dvb_usb_start_feed, > drivers/bluetooth/btusb.c:btusb_mtk_hci_wmt_sync > and perhaps in other places. > > We fix this class of bugs by adding a read barrier after test_bit(). > > Signed-off-by: Mikulas Patocka > Cc: stable@vger.kernel.org > > Index: linux-2.6/include/linux/wait_bit.h > =================================================================== > --- linux-2.6.orig/include/linux/wait_bit.h > +++ linux-2.6/include/linux/wait_bit.h > @@ -71,8 +71,10 @@ static inline int > wait_on_bit(unsigned long *word, int bit, unsigned mode) > { > might_sleep(); > - if (!test_bit(bit, word)) > + if (!test_bit(bit, word)) { > + smp_rmb(); Any new code using smp_rmb or an acquire access should always include a comment that explains where the matching smp_wmb or release access is. Alan Stern > return 0; > + } > return out_of_line_wait_on_bit(word, bit, > bit_wait, > mode); > @@ -96,8 +98,10 @@ static inline int > wait_on_bit_io(unsigned long *word, int bit, unsigned mode) > { > might_sleep(); > - if (!test_bit(bit, word)) > + if (!test_bit(bit, word)) { > + smp_rmb(); > return 0; > + } > return out_of_line_wait_on_bit(word, bit, > bit_wait_io, > mode); > @@ -123,8 +127,10 @@ wait_on_bit_timeout(unsigned long *word, > unsigned long timeout) > { > might_sleep(); > - if (!test_bit(bit, word)) > + if (!test_bit(bit, word)) { > + smp_rmb(); > return 0; > + } > return out_of_line_wait_on_bit_timeout(word, bit, > bit_wait_timeout, > mode, timeout); > @@ -151,8 +157,10 @@ wait_on_bit_action(unsigned long *word, > unsigned mode) > { > might_sleep(); > - if (!test_bit(bit, word)) > + if (!test_bit(bit, word)) { > + smp_rmb(); > return 0; > + } > return out_of_line_wait_on_bit(word, bit, action, mode); > } > > Index: linux-2.6/kernel/sched/wait_bit.c > =================================================================== > --- linux-2.6.orig/kernel/sched/wait_bit.c > +++ linux-2.6/kernel/sched/wait_bit.c > @@ -51,6 +51,8 @@ __wait_on_bit(struct wait_queue_head *wq > > finish_wait(wq_head, &wbq_entry->wq_entry); > > + smp_rmb(); > + > return ret; > } > EXPORT_SYMBOL(__wait_on_bit); >