linux-kernel - Re: [PATCH] Revert "bcachefs: Add asserts to bch2_dev_btree_bitmap_marked

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d56wz2z6qne7lym3nidzb36yal25cmnaebjw6mkrz4lykw7ntz@jw6xsbveq227>
Date: Tue, 22 Oct 2024 19:29:02 +0530
From: Manas <manas18244@...td.ac.in>
To: Hongbo Li <lihongbo22@...wei.com>
Cc: Kent Overstreet <kent.overstreet@...ux.dev>, 
	linux-bcachefs@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Anup Sharma <anupnewsmail@...il.com>, Shuah Khan <skhan@...uxfoundation.org>, 
	syzbot+e8eff054face85d7ea41@...kaller.appspotmail.com
Subject: Re: [PATCH] Revert "bcachefs: Add asserts to
 bch2_dev_btree_bitmap_marked_sectors()"

On 22.10.2024 09:43, Hongbo Li wrote:
>On 2024/10/22 0:48, Manas via B4 Relay wrote:
>>From: Manas <manas18244@...td.ac.in>
>>
>>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@...td.ac.in>
>>Reported-by: syzbot+e8eff054face85d7ea41@...kaller.appspotmail.com
>>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