* [PATCH] bcachefs: Fix shift-out-of-bounds in bch2_blacklist_entries_gc
@ 2024-06-25 18:41 Pei Li
2024-06-25 21:53 ` Kent Overstreet
0 siblings, 1 reply; 2+ messages in thread
From: Pei Li @ 2024-06-25 18:41 UTC (permalink / raw)
To: Kent Overstreet, Brian Foster
Cc: linux-bcachefs, linux-kernel, syzkaller-bugs, skhan, peili.dev,
syzbot+835d255ad6bc7f29ee12
This series fix the shift-out-of-bounds issue in
bch2_blacklist_entries_gc().
Instead of passing 0 to eytzinger0_first() when iterating the entries,
we explicitly check 0 and initialize i to be 0.
syzbot has tested the proposed patch and the reproducer did not trigger
any issue:
Reported-and-tested-by: syzbot+835d255ad6bc7f29ee12@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=835d255ad6bc7f29ee12
Signed-off-by: Pei Li <peili.dev@gmail.com>
---
Syzbot reported the following issue:
UBSAN: shift-out-of-bounds in ./include/linux/log2.h:67:13
shift exponent 4294967295 is too large for 64-bit type 'long unsigned int'
This is because 0 is passed to __rounddown_pow_of_two(), and -1 is
returned as an unsigned integer. In 32 bit system, it will become
4294967295.
This patch fixes the issue by adding check in
bch2_blacklist_entries_gc() to avoid passing 0 into eytzinger0_first().
If we found out t->nr equals to 0, we directly use 0 to access the root
of the list.
Tested on:
commit: 55027e68 Merge tag 'input-for-v6.10-rc5' of git://git...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1191153e980000
kernel config: https://syzkaller.appspot.com/x/.config?x=d6b9ee98d841760c
dashboard link: https://syzkaller.appspot.com/bug?extid=835d255ad6bc7f29ee12
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=15ca4301980000
---
fs/bcachefs/journal_seq_blacklist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/bcachefs/journal_seq_blacklist.c b/fs/bcachefs/journal_seq_blacklist.c
index ed4846709611..1f25c111c54c 100644
--- a/fs/bcachefs/journal_seq_blacklist.c
+++ b/fs/bcachefs/journal_seq_blacklist.c
@@ -232,7 +232,7 @@ bool bch2_blacklist_entries_gc(struct bch_fs *c)
BUG_ON(nr != t->nr);
unsigned i;
- for (src = bl->start, i = eytzinger0_first(t->nr);
+ for (src = bl->start, i = t->nr == 0 ? 0 : eytzinger0_first(t->nr);
src < bl->start + nr;
src++, i = eytzinger0_next(i, nr)) {
BUG_ON(t->entries[i].start != le64_to_cpu(src->start));
---
base-commit: 563a50672d8a86ec4b114a4a2f44d6e7ff855f5b
change-id: 20240625-bug1-42684cf9d7ed
Best regards,
--
Pei Li <peili.dev@gmail.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] bcachefs: Fix shift-out-of-bounds in bch2_blacklist_entries_gc
2024-06-25 18:41 [PATCH] bcachefs: Fix shift-out-of-bounds in bch2_blacklist_entries_gc Pei Li
@ 2024-06-25 21:53 ` Kent Overstreet
0 siblings, 0 replies; 2+ messages in thread
From: Kent Overstreet @ 2024-06-25 21:53 UTC (permalink / raw)
To: Pei Li
Cc: Brian Foster, linux-bcachefs, linux-kernel, syzkaller-bugs,
skhan, syzbot+835d255ad6bc7f29ee12
On Tue, Jun 25, 2024 at 11:41:29AM -0700, Pei Li wrote:
> This series fix the shift-out-of-bounds issue in
> bch2_blacklist_entries_gc().
>
> Instead of passing 0 to eytzinger0_first() when iterating the entries,
> we explicitly check 0 and initialize i to be 0.
>
> syzbot has tested the proposed patch and the reproducer did not trigger
> any issue:
>
> Reported-and-tested-by: syzbot+835d255ad6bc7f29ee12@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=835d255ad6bc7f29ee12
> Signed-off-by: Pei Li <peili.dev@gmail.com>
> ---
> Syzbot reported the following issue:
> UBSAN: shift-out-of-bounds in ./include/linux/log2.h:67:13
> shift exponent 4294967295 is too large for 64-bit type 'long unsigned int'
>
> This is because 0 is passed to __rounddown_pow_of_two(), and -1 is
> returned as an unsigned integer. In 32 bit system, it will become
> 4294967295.
>
> This patch fixes the issue by adding check in
> bch2_blacklist_entries_gc() to avoid passing 0 into eytzinger0_first().
> If we found out t->nr equals to 0, we directly use 0 to access the root
> of the list.
Thanks! Applied
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-06-25 21:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-25 18:41 [PATCH] bcachefs: Fix shift-out-of-bounds in bch2_blacklist_entries_gc Pei Li
2024-06-25 21:53 ` Kent Overstreet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®