[syzbot] [bcachefs?] UBSAN: shift-out-of-bounds in member_to_text

[syzbot] [bcachefs?] UBSAN: shift-out-of-bounds in member_to_text

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    431c1646e1f8 Linux 6.11-rc6
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1433084d980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=660f6eb11f9c7dc5
dashboard link: https://syzkaller.appspot.com/bug?extid=064ce437a1ad63d3f6ef
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=160ccf2b980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=13dbab2b980000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-431c1646.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/e5b61d9e315e/vmlinux-431c1646.xz
kernel image: https://storage.googleapis.com/syzbot-assets/149431616278/bzImage-431c1646.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/814c5affc830/mount_0.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+064ce437a1ad63d3f6ef@syzkaller.appspotmail.com

loop0: detected capacity change from 0 to 32768
------------[ cut here ]------------
UBSAN: shift-out-of-bounds in fs/bcachefs/sb-members.c:249:2
shift exponent 69 is too large for 64-bit type 'unsigned long long'
CPU: 0 UID: 0 PID: 5099 Comm: syz-executor454 Not tainted 6.11.0-rc6-syzkaller #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:93 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:119
 ubsan_epilogue lib/ubsan.c:231 [inline]
 __ubsan_handle_shift_out_of_bounds+0x3c8/0x420 lib/ubsan.c:468
 member_to_text+0x9a2/0x9b0 fs/bcachefs/sb-members.c:249
 bch2_sb_members_v2_to_text+0x267/0x3b0 fs/bcachefs/sb-members.c:310
 bch2_sb_field_validate+0x201/0x2e0 fs/bcachefs/super-io.c:1233
 bch2_sb_validate+0xb4b/0xef0 fs/bcachefs/super-io.c:472
 __bch2_read_super+0xc1b/0x1370 fs/bcachefs/super-io.c:827
 bch2_fs_open+0x246/0xdf0 fs/bcachefs/super.c:2083
 bch2_fs_get_tree+0x731/0x1700 fs/bcachefs/fs.c:1933
 vfs_get_tree+0x90/0x2b0 fs/super.c:1800
 do_new_mount+0x2be/0xb40 fs/namespace.c:3472
 do_mount fs/namespace.c:3812 [inline]
 __do_sys_mount fs/namespace.c:4020 [inline]
 __se_sys_mount+0x2d6/0x3c0 fs/namespace.c:3997
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fe3ee4d7faa
Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb a6 e8 5e 04 00 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe262fa7b8 EFLAGS: 00000282 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007ffe262fa7d0 RCX: 00007fe3ee4d7faa
RDX: 00000000200058c0 RSI: 0000000020000100 RDI: 00007ffe262fa7d0
RBP: 0000000000000004 R08: 00007ffe262fa810 R09: 0000000000005903
R10: 0000000000000000 R11: 0000000000000282 R12: 0000000000000000
R13: 00007ffe262fa810 R14: 0000000000000003 R15: 0000000001000000
 </TASK>
---[ end trace ]---


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [syzbot] [bcachefs?] UBSAN: shift-out-of-bounds in member_to_text

[Edward Adam Davis]

Because the value of m.btree_bitmap_shift is 69 in this case, it cause 
shift-out-of-bounds in member_to_text.

Add a check for btree_bitmap_shift in validate_member, when it bigger than 64
return -BCH_ERR_invalid_sb_members.
Simultaneously adjust the output mode of btree_bitmap_shift in member_to_text. 

#syz test

diff --git a/fs/bcachefs/sb-members.c b/fs/bcachefs/sb-members.c
index 39196f2a4197..85a4245ea6a3 100644
--- a/fs/bcachefs/sb-members.c
+++ b/fs/bcachefs/sb-members.c
@@ -162,6 +162,12 @@ static int validate_member(struct printbuf *err,
 		return -BCH_ERR_invalid_sb_members;
 	}
 
+	if (m.btree_bitmap_shift > 64) {
+		prt_printf(err, "device %u: too many big bitmap shift (got %u, max 64)",
+			   i, m.btree_bitmap_shift);
+		return -BCH_ERR_invalid_sb_members;
+	}
+
 	return 0;
 }
 
@@ -245,8 +251,7 @@ static void member_to_text(struct printbuf *out,
 		prt_printf(out, "(none)");
 	prt_newline(out);
 
-	prt_printf(out, "Btree allocated bitmap blocksize:\t");
-	prt_units_u64(out, 1ULL << m.btree_bitmap_shift);
+	prt_printf(out, "Btree allocated bitmap shift: %d\t", m.btree_bitmap_shift);
 	prt_newline(out);
 
 	prt_printf(out, "Btree allocated bitmap:\t");

Re: [syzbot] [bcachefs?] UBSAN: shift-out-of-bounds in member_to_text

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+064ce437a1ad63d3f6ef@syzkaller.appspotmail.com
Tested-by: syzbot+064ce437a1ad63d3f6ef@syzkaller.appspotmail.com

Tested on:

commit:         67784a74 Merge tag 'ata-6.11-rc7' of git://git.kernel...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=135f0f33980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=660f6eb11f9c7dc5
dashboard link: https://syzkaller.appspot.com/bug?extid=064ce437a1ad63d3f6ef
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=140dcb1f980000

Note: testing is done by a robot and is best-effort only.

Re: [syzbot] [bcachefs?] UBSAN: shift-out-of-bounds in member_to_text

[Kent Overstreet]

On Tue, Sep 03, 2024 at 10:13:02AM GMT, Edward Adam Davis wrote:
> Because the value of m.btree_bitmap_shift is 69 in this case, it cause 
> shift-out-of-bounds in member_to_text.
> 
> Add a check for btree_bitmap_shift in validate_member, when it bigger than 64
> return -BCH_ERR_invalid_sb_members.
> Simultaneously adjust the output mode of btree_bitmap_shift in member_to_text. 
> 
> #syz test
> 
> diff --git a/fs/bcachefs/sb-members.c b/fs/bcachefs/sb-members.c
> index 39196f2a4197..85a4245ea6a3 100644
> --- a/fs/bcachefs/sb-members.c
> +++ b/fs/bcachefs/sb-members.c
> @@ -162,6 +162,12 @@ static int validate_member(struct printbuf *err,
>  		return -BCH_ERR_invalid_sb_members;
>  	}
>  
> +	if (m.btree_bitmap_shift > 64) {

should be >=, no?

> +		prt_printf(err, "device %u: too many big bitmap shift (got %u, max 64)",
> +			   i, m.btree_bitmap_shift);
> +		return -BCH_ERR_invalid_sb_members;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -245,8 +251,7 @@ static void member_to_text(struct printbuf *out,
>  		prt_printf(out, "(none)");
>  	prt_newline(out);
>  
> -	prt_printf(out, "Btree allocated bitmap blocksize:\t");
> -	prt_units_u64(out, 1ULL << m.btree_bitmap_shift);
> +	prt_printf(out, "Btree allocated bitmap shift: %d\t", m.btree_bitmap_shift);
>  	prt_newline(out);
>  
>  	prt_printf(out, "Btree allocated bitmap:\t");
> 

Re: [syzbot] [bcachefs?] UBSAN: shift-out-of-bounds in member_to_text

[Edward Adam Davis]

On Wed, 4 Sep 2024 15:39:37 -0400, Kent Overstreet wrote: 
> On Tue, Sep 03, 2024 at 10:13:02AM GMT, Edward Adam Davis wrote:
> > Because the value of m.btree_bitmap_shift is 69 in this case, it cause
> > shift-out-of-bounds in member_to_text.
> >
> > Add a check for btree_bitmap_shift in validate_member, when it bigger than 64
> > return -BCH_ERR_invalid_sb_members.
> > Simultaneously adjust the output mode of btree_bitmap_shift in member_to_text.
> >
> > #syz test
> >
> > diff --git a/fs/bcachefs/sb-members.c b/fs/bcachefs/sb-members.c
> > index 39196f2a4197..85a4245ea6a3 100644
> > --- a/fs/bcachefs/sb-members.c
> > +++ b/fs/bcachefs/sb-members.c
> > @@ -162,6 +162,12 @@ static int validate_member(struct printbuf *err,
> >  		return -BCH_ERR_invalid_sb_members;
> >  	}
> >
> > +	if (m.btree_bitmap_shift > 64) {
> 
> should be >=, no?
Yes, you are right, here should be "if (m.btree_bitmap_shift >= 64)"
> 
> > +		prt_printf(err, "device %u: too many big bitmap shift (got %u, max 64)",
> > +			   i, m.btree_bitmap_shift);
> > +		return -BCH_ERR_invalid_sb_members;
> > +	}
> > +
> >  	return 0;
> >  }
> >
> > @@ -245,8 +251,7 @@ static void member_to_text(struct printbuf *out,
> >  		prt_printf(out, "(none)");
> >  	prt_newline(out);
> >
> > -	prt_printf(out, "Btree allocated bitmap blocksize:\t");
> > -	prt_units_u64(out, 1ULL << m.btree_bitmap_shift);
> > +	prt_printf(out, "Btree allocated bitmap shift: %d\t", m.btree_bitmap_shift);
> >  	prt_newline(out);
> >
> >  	prt_printf(out, "Btree allocated bitmap:\t");

BR,
Edward

[PATCH] bcachefs: Fix oob in member_to_text

[Edward Adam Davis]

Because the value of m.btree_bitmap_shift is 69 in this case, it cause
shift-out-of-bounds in member_to_text.

Add a check for btree_bitmap_shift in validate_member, when it is greater
than or equal to 64 return -BCH_ERR_invalid_sb_members.
Simultaneously adjust the output mode of btree_bitmap_shift in member_to_text.

Reported-and-tested-by: syzbot+064ce437a1ad63d3f6ef@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=064ce437a1ad63d3f6ef
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 fs/bcachefs/sb-members.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/bcachefs/sb-members.c b/fs/bcachefs/sb-members.c
index 39196f2a4197..c9daab47f886 100644
--- a/fs/bcachefs/sb-members.c
+++ b/fs/bcachefs/sb-members.c
@@ -162,6 +162,12 @@ static int validate_member(struct printbuf *err,
 		return -BCH_ERR_invalid_sb_members;
 	}
 
+	if (m.btree_bitmap_shift >= 64) {
+		prt_printf(err, "device %u: too many big bitmap shift (got %u, max 64)",
+			   i, m.btree_bitmap_shift);
+		return -BCH_ERR_invalid_sb_members;
+	}
+
 	return 0;
 }
 
@@ -245,8 +251,7 @@ static void member_to_text(struct printbuf *out,
 		prt_printf(out, "(none)");
 	prt_newline(out);
 
-	prt_printf(out, "Btree allocated bitmap blocksize:\t");
-	prt_units_u64(out, 1ULL << m.btree_bitmap_shift);
+	prt_printf(out, "Btree allocated bitmap shift: %d\t", m.btree_bitmap_shift);
 	prt_newline(out);
 
 	prt_printf(out, "Btree allocated bitmap:\t");
-- 
2.43.0

Re: [syzbot] [bcachefs?] UBSAN: shift-out-of-bounds in member_to_text

[Kent Overstreet]

On Sun, Sep 08, 2024 at 08:59:37PM GMT, Edward Adam Davis wrote:
> On Wed, 4 Sep 2024 15:39:37 -0400, Kent Overstreet wrote: 
> > On Tue, Sep 03, 2024 at 10:13:02AM GMT, Edward Adam Davis wrote:
> > > Because the value of m.btree_bitmap_shift is 69 in this case, it cause
> > > shift-out-of-bounds in member_to_text.
> > >
> > > Add a check for btree_bitmap_shift in validate_member, when it bigger than 64
> > > return -BCH_ERR_invalid_sb_members.
> > > Simultaneously adjust the output mode of btree_bitmap_shift in member_to_text.
> > >
> > > #syz test
> > >
> > > diff --git a/fs/bcachefs/sb-members.c b/fs/bcachefs/sb-members.c
> > > index 39196f2a4197..85a4245ea6a3 100644
> > > --- a/fs/bcachefs/sb-members.c
> > > +++ b/fs/bcachefs/sb-members.c
> > > @@ -162,6 +162,12 @@ static int validate_member(struct printbuf *err,
> > >  		return -BCH_ERR_invalid_sb_members;
> > >  	}
> > >
> > > +	if (m.btree_bitmap_shift > 64) {
> > 
> > should be >=, no?
> Yes, you are right, here should be "if (m.btree_bitmap_shift >= 64)"

Send me a proper patch and I'll apply...