From: Manas <manas18244@iiitd.ac.in>

The syzkaller bug triggers BUG_ON assertions in
__bch2_dev_btree_bitmap_mark. btree_bitmap_shift is checked in
bch2_dev_btree_bitmap_marked_sectors prior to this. But only one out of
two assertions is checked. This patch adds the other as conditional.

Reported-by: syzbot+e8eff054face85d7ea41@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e8eff054face85d7ea41
Signed-off-by: Manas <manas18244@iiitd.ac.in>
---
The syzkaller bug[1] triggers BUG_ON assertions in
__bch2_dev_btree_bitmap_mark.

btree_bitmap_shift is checked in bch2_dev_btree_bitmap_marked_sectors
prior to this. But only one out of two assertions is checked. This patch
adds the other as conditional.

A previous discussion about adding conditional in
bch2_dev_btree_bitmap_marked_sectors is here[2].

[1] https://syzkaller.appspot.com/bug?extid=e8eff054face85d7ea41
[2] https://lore.kernel.org/all/9ec25394-3d89-41b3-b62e-2d522cdc7319@huawei.com/
---
 fs/bcachefs/sb-members.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/bcachefs/sb-members.h b/fs/bcachefs/sb-members.h
index 762083b564ee5909071eb4235c3d5cd6e72ee519..3141791491928930a707a6a16579c6d2e11e3700 100644
--- a/fs/bcachefs/sb-members.h
+++ b/fs/bcachefs/sb-members.h
@@ -347,12 +347,16 @@ void bch2_dev_errors_reset(struct bch_dev *);
 static inline bool bch2_dev_btree_bitmap_marked_sectors(struct bch_dev *ca, u64 start, unsigned sectors)
 {
 	u64 end = start + sectors;
+	u8 bitmap_shift = ca->mi.btree_bitmap_shift;
 
-	if (end > 64ULL << ca->mi.btree_bitmap_shift)
+	if (bitmap_shift > 57)
+		return true;
+
+	if (end > 64ULL << bitmap_shift)
 		return false;
 
-	for (unsigned bit = start >> ca->mi.btree_bitmap_shift;
-	     (u64) bit << ca->mi.btree_bitmap_shift < end;
+	for (unsigned bit = start >> bitmap_shift;
+	     (u64) bit << bitmap_shift < end;
 	     bit++)
 		if (!(ca->mi.btree_allocated_bitmap & BIT_ULL(bit)))
 			return false;

---
base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
change-id: 20241022-fix-missing-cond-bcachefs-a889f51e8aa2

Best regards,
-- 
Manas <manas18244@iiitd.ac.in>

On Tue, Oct 22, 2024 at 08:55:32PM +0530, Manas via B4 Relay wrote:
> From: Manas <manas18244@iiitd.ac.in>
> 
> The syzkaller bug triggers BUG_ON assertions in
> __bch2_dev_btree_bitmap_mark. btree_bitmap_shift is checked in
> bch2_dev_btree_bitmap_marked_sectors prior to this. But only one out of
> two assertions is checked. This patch adds the other as conditional.
> 
> Reported-by: syzbot+e8eff054face85d7ea41@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e8eff054face85d7ea41
> Signed-off-by: Manas <manas18244@iiitd.ac.in>
> ---
> The syzkaller bug[1] triggers BUG_ON assertions in
> __bch2_dev_btree_bitmap_mark.

The validation in validate_member() just needs to be fixed (and why 57?
that should be a constant, not a magic number).

> 
> btree_bitmap_shift is checked in bch2_dev_btree_bitmap_marked_sectors
> prior to this. But only one out of two assertions is checked. This patch
> adds the other as conditional.
> 
> A previous discussion about adding conditional in
> bch2_dev_btree_bitmap_marked_sectors is here[2].
> 
> [1] https://syzkaller.appspot.com/bug?extid=e8eff054face85d7ea41
> [2] https://lore.kernel.org/all/9ec25394-3d89-41b3-b62e-2d522cdc7319@huawei.com/
> ---
>  fs/bcachefs/sb-members.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/bcachefs/sb-members.h b/fs/bcachefs/sb-members.h
> index 762083b564ee5909071eb4235c3d5cd6e72ee519..3141791491928930a707a6a16579c6d2e11e3700 100644
> --- a/fs/bcachefs/sb-members.h
> +++ b/fs/bcachefs/sb-members.h
> @@ -347,12 +347,16 @@ void bch2_dev_errors_reset(struct bch_dev *);
>  static inline bool bch2_dev_btree_bitmap_marked_sectors(struct bch_dev *ca, u64 start, unsigned sectors)
>  {
>  	u64 end = start + sectors;
> +	u8 bitmap_shift = ca->mi.btree_bitmap_shift;
>  
> -	if (end > 64ULL << ca->mi.btree_bitmap_shift)
> +	if (bitmap_shift > 57)
> +		return true;
> +
> +	if (end > 64ULL << bitmap_shift)
>  		return false;
>  
> -	for (unsigned bit = start >> ca->mi.btree_bitmap_shift;
> -	     (u64) bit << ca->mi.btree_bitmap_shift < end;
> +	for (unsigned bit = start >> bitmap_shift;
> +	     (u64) bit << bitmap_shift < end;
>  	     bit++)
>  		if (!(ca->mi.btree_allocated_bitmap & BIT_ULL(bit)))
>  			return false;
> 
> ---
> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> change-id: 20241022-fix-missing-cond-bcachefs-a889f51e8aa2
> 
> Best regards,
> -- 
> Manas <manas18244@iiitd.ac.in>
> 
>