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 <[email protected]> > To: Brian Foster <[email protected]> > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Reported-by: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=e74fea078710bbca6f4b > Tested-by: [email protected] > Signed-off-by: Pei Li <[email protected]> > --- > 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 <[email protected]> Date: Tue Jul 9 16:43:01 2024 -0400 bcachefs: Fix RCU splat Reported-by: [email protected] Signed-off-by: Kent Overstreet <[email protected]> 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) {
