Re: [PATCH] Revert "bcachefs: Add asserts to bch2_dev_btree_bitmap_marked_sectors()"

[Manas]

On 22.10.2024 09:43, Hongbo Li wrote:
> On 2024/10/22 0:48, Manas via B4 Relay wrote:
> > From: Manas <manas18244@xxxxxxxxxxx>
> >
> > This reverts commit 60f2b1bcf519416dbffee219132aa949d0c39d0e.
> >
> > This syzbot bug[1] is triggered due to the BUG_ON assertions added in
> > __bch2_dev_btree_bitmap_mark. During runtime, m->btree_bitmap_shift is
> > 63 '?'. This triggers both the assertions.
> >
> > Reverting the commit does not reproduce the said bug.
> >
> > [1] https://syzkaller.appspot.com/bug?extid=e8eff054face85d7ea41
> >
> > Signed-off-by: Manas <manas18244@xxxxxxxxxxx>
> > Reported-by: syzbot+e8eff054face85d7ea41@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://syzkaller.appspot.com/bug?extid=e8eff054face85d7ea41
> > ---
> > This syzbot bug[1] is triggered due to the BUG_ON assertions added in
> > __bch2_dev_btree_bitmap_mark. During runtime, m->btree_bitmap_shift is
> > 63 '?'. This triggers both the assertions.
> >
> > I am unfamiliar with the codebase, and there wasn't a lore discussion
> > about the assertions in the commit, so I am unsure about the relevance
> > of these assertions.
> >
> > Reverting the commit does not reproduce the said bug.
> >
> > [1] https://syzkaller.appspot.com/bug?extid=e8eff054face85d7ea41
> > ---
> >  fs/bcachefs/sb-members.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/fs/bcachefs/sb-members.c b/fs/bcachefs/sb-members.c
> > index fb08dd680dacf82bca414f424024e4a00bf432de..9790fd47338c46d2af30547e1f41a1e578b71aa4 100644
> > --- a/fs/bcachefs/sb-members.c
> > +++ b/fs/bcachefs/sb-members.c
> > @@ -450,9 +450,6 @@ static void __bch2_dev_btree_bitmap_mark(struct bch_sb_field_members_v2 *mi, uns
> >  		m->btree_bitmap_shift += resize;
> >  	}
> > -	BUG_ON(m->btree_bitmap_shift > 57);
> > -	BUG_ON(end > 64ULL << m->btree_bitmap_shift);
> > -
> May be this is not good way by just removing the BUG_ON. In my humble 
> opinion, the former code have checked m->btree_bitmap_shift in 
> bch2_dev_btree_bitmap_marked_sectors. May be add the similar condition 
> in this helper will be better.
Hi Hongbo, thanks for reviewing this. I was unsure about the fix so I decided to
initiate the conversation by reverting. Yes, that makes sense. I am sending a
fresh patch adding that condition.

--
Manas