1. Patchew
  2. linux
  3. View series

[PATCH] Fix WARNING: suspicious RCU usage in bch2_bucket_ref_update

Pei Li posted 1 patch 1 year, 5 months ago 
fs/bcachefs/buckets.c | 6 ++++++
1 file changed, 6 insertions(+)
[PATCH] Fix WARNING: suspicious RCU usage in bch2_bucket_ref_update
Posted by Pei Li 1 year, 5 months ago 
When dereference ca->bucket_gens, we are expecting one of the three
locks to be hold if ca->fs is not NULL.

This patch acquires mark_lock before entering __mark_pointer()

Note: testing is done by a robot and is best-effort only.

To: Kent Overstreet <kent.overstreet@linux.dev>
To: Brian Foster <bfoster@redhat.com>
Cc: linux-bcachefs@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: skhan@linuxfoundation.org
Cc: syzkaller-bugs@googlegroups.com
Cc: linux-kernel-mentees@lists.linuxfoundation.org
Reported-by: syzbot+e74fea078710bbca6f4b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e74fea078710bbca6f4b
Tested-by: syzbot+e74fea078710bbca6f4b@syzkaller.appspotmail.com
Signed-off-by: Pei Li <peili.dev@gmail.com>
---
Syzbot reported the following warning:

fs/bcachefs/buckets.h:107 suspicious rcu_dereference_check() usage!

When dereference ca->bucket_gens, we are expecting one of the three
locks to be hold if ca->fs is not NULL.

This patch acquires mark_lock before entering __mark_pointer()

Tested on:

commit:         34afb82a Merge tag '6.10-rc6-smb3-server-fixes' of git..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1492e87e980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=42a432cfd0e579e0
dashboard link: https://syzkaller.appspot.com/bug?extid=e74fea078710bbca6f4b
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=15e2e87e980000

Note: testing is done by a robot and is best-effort only.
---
 fs/bcachefs/buckets.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/bcachefs/buckets.c b/fs/bcachefs/buckets.c
index 743d57eba760..03147e46dc47 100644
--- a/fs/bcachefs/buckets.c
+++ b/fs/bcachefs/buckets.c
@@ -1033,8 +1033,14 @@ static int bch2_trigger_pointer(struct btree_trans *trans,
 
 	if (flags & BTREE_TRIGGER_transactional) {
 		struct bkey_i_alloc_v4 *a = bch2_trans_start_alloc_update(trans, bucket);
+
+		percpu_down_read(&c->mark_lock);
+
 		ret = PTR_ERR_OR_ZERO(a) ?:
 			__mark_pointer(trans, ca, k, &p.ptr, *sectors, bp.data_type, &a->v);
+
+		percpu_up_read(&c->mark_lock);
+
 		if (ret)
 			goto err;
 

---
base-commit: 256abd8e550ce977b728be79a74e1729438b4948
change-id: 20240709-bug10-6b354d193f20

Best regards,
-- 
Pei Li <peili.dev@gmail.com>
Re: [PATCH] Fix WARNING: suspicious RCU usage in bch2_bucket_ref_update
Posted by Kent Overstreet 1 year, 5 months ago 
On Wed, Jul 10, 2024 at 08:33:24AM GMT, Pei Li wrote:
> When dereference ca->bucket_gens, we are expecting one of the three
> locks to be hold if ca->fs is not NULL.
> 
> This patch acquires mark_lock before entering __mark_pointer()
> 
> Note: testing is done by a robot and is best-effort only.
> 
> To: Kent Overstreet <kent.overstreet@linux.dev>
> To: Brian Foster <bfoster@redhat.com>
> Cc: linux-bcachefs@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: skhan@linuxfoundation.org
> Cc: syzkaller-bugs@googlegroups.com
> Cc: linux-kernel-mentees@lists.linuxfoundation.org
> Reported-by: syzbot+e74fea078710bbca6f4b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e74fea078710bbca6f4b
> Tested-by: syzbot+e74fea078710bbca6f4b@syzkaller.appspotmail.com
> Signed-off-by: Pei Li <peili.dev@gmail.com>
> ---
> Syzbot reported the following warning:
> 
> fs/bcachefs/buckets.h:107 suspicious rcu_dereference_check() usage!
> 
> When dereference ca->bucket_gens, we are expecting one of the three
> locks to be hold if ca->fs is not NULL.
> 
> This patch acquires mark_lock before entering __mark_pointer()
> 
> Tested on:
> 
> commit:         34afb82a Merge tag '6.10-rc6-smb3-server-fixes' of git..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=1492e87e980000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=42a432cfd0e579e0
> dashboard link: https://syzkaller.appspot.com/bug?extid=e74fea078710bbca6f4b
> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> patch:          https://syzkaller.appspot.com/x/patch.diff?x=15e2e87e980000
> 
> Note: testing is done by a robot and is best-effort only.
> ---
>  fs/bcachefs/buckets.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/bcachefs/buckets.c b/fs/bcachefs/buckets.c
> index 743d57eba760..03147e46dc47 100644
> --- a/fs/bcachefs/buckets.c
> +++ b/fs/bcachefs/buckets.c
> @@ -1033,8 +1033,14 @@ static int bch2_trigger_pointer(struct btree_trans *trans,
>  
>  	if (flags & BTREE_TRIGGER_transactional) {
>  		struct bkey_i_alloc_v4 *a = bch2_trans_start_alloc_update(trans, bucket);
> +
> +		percpu_down_read(&c->mark_lock);
> +
>  		ret = PTR_ERR_OR_ZERO(a) ?:
>  			__mark_pointer(trans, ca, k, &p.ptr, *sectors, bp.data_type, &a->v);
> +
> +		percpu_up_read(&c->mark_lock);

This is much heavier than we need, a more localized rcu_read_lock() is
sufficient. This is in my tree (neglected to include it with today's
pull request, whoops).

commit 5df2f4ffe8bb9bead346f4768df603fa575678df
Author: Kent Overstreet <kent.overstreet@linux.dev>
Date:   Tue Jul 9 16:43:01 2024 -0400

    bcachefs: Fix RCU splat
    
    Reported-by: syzbot+e74fea078710bbca6f4b@syzkaller.appspotmail.com
    Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/buckets.c b/fs/bcachefs/buckets.c
index 743d57eba760..314ee3e0187f 100644
--- a/fs/bcachefs/buckets.c
+++ b/fs/bcachefs/buckets.c
@@ -805,7 +805,7 @@ int bch2_bucket_ref_update(struct btree_trans *trans, struct bch_dev *ca,
 			"bucket %u:%zu gen %u (mem gen %u) data type %s: stale dirty ptr (gen %u)\n"
 			"while marking %s",
 			ptr->dev, bucket_nr, b_gen,
-			*bucket_gen(ca, bucket_nr),
+			bucket_gen_get(ca, bucket_nr),
 			bch2_data_type_str(bucket_data_type ?: ptr_data_type),
 			ptr->gen,
 			(printbuf_reset(&buf),
diff --git a/fs/bcachefs/buckets.h b/fs/bcachefs/buckets.h
index 80ee0be9793e..8ad4be73860c 100644
--- a/fs/bcachefs/buckets.h
+++ b/fs/bcachefs/buckets.h
@@ -116,6 +116,14 @@ static inline u8 *bucket_gen(struct bch_dev *ca, size_t b)
 	return gens->b + b;
 }
 
+static inline u8 bucket_gen_get(struct bch_dev *ca, size_t b)
+{
+	rcu_read_lock();
+	u8 gen = *bucket_gen(ca, b);
+	rcu_read_unlock();
+	return gen;
+}
+
 static inline size_t PTR_BUCKET_NR(const struct bch_dev *ca,
 				   const struct bch_extent_ptr *ptr)
 {

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.