[PATCH] wait_on_bit: add an acquire memory barrier

From: Mikulas Patocka
Date: Mon Aug 22 2022 - 05:39:03 EST


Hi

I'd like to ask what do you think about this patch? Do you want to commit
it - or do you think that the barrier should be added to the callers of
wait_on_bit?

Mikulas



From: Mikulas Patocka <mpatocka@xxxxxxxxxx>

There are several places in the kernel where wait_on_bit is not followed
by a memory barrier (for example, in drivers/md/dm-bufio.c:new_read). On
architectures with weak memory ordering, it may happen that memory
accesses that follow wait_on_bit are reordered before wait_on_bit and they
may return invalid data.

Fix this class of bugs by adding an acquire memory barrier to wait_on_bit,
wait_on_bit_io, wait_on_bit_timeout and wait_on_bit_action. The code that
uses these functions should clear the bit using the function
clear_bit_unlock.

Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx

---
include/linux/wait_bit.h | 16 ++++++++++++----
kernel/sched/wait_bit.c | 2 ++
2 files changed, 14 insertions(+), 4 deletions(-)

Index: linux-2.6/include/linux/wait_bit.h
===================================================================
--- linux-2.6.orig/include/linux/wait_bit.h 2022-08-20 14:33:44.000000000 +0200
+++ linux-2.6/include/linux/wait_bit.h 2022-08-20 15:41:43.000000000 +0200
@@ -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_acquire__after_ctrl_dep(); /* should pair with clear_bit_unlock */
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_acquire__after_ctrl_dep(); /* should pair with clear_bit_unlock */
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_acquire__after_ctrl_dep(); /* should pair with clear_bit_unlock */
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_acquire__after_ctrl_dep(); /* should pair with clear_bit_unlock */
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 2022-08-20 14:33:44.000000000 +0200
+++ linux-2.6/kernel/sched/wait_bit.c 2022-08-20 15:41:39.000000000 +0200
@@ -49,6 +49,8 @@ __wait_on_bit(struct wait_queue_head *wq
ret = (*action)(&wbq_entry->key, mode);
} while (test_bit(wbq_entry->key.bit_nr, wbq_entry->key.flags) && !ret);

+ smp_acquire__after_ctrl_dep(); /* should pair with clear_bit_unlock */
+
finish_wait(wq_head, &wbq_entry->wq_entry);

return ret;