[syzbot] [btrfs?] general protection fault in btrfs_lookup_csums_bitmap

[syzbot] [btrfs?] general protection fault in btrfs_lookup_csums_bitmap

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    b04ae0f45168 Merge tag 'v6.12-rc3-smb3-client-fixes' of gi..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11478430580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=cfbd94c114a3d407
dashboard link: https://syzkaller.appspot.com/bug?extid=5d2b33d7835870519b5f
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=1162d240580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=15478430580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7feb34a89c2a/non_bootable_disk-b04ae0f4.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/3e40a4ec7885/vmlinux-b04ae0f4.xz
kernel image: https://storage.googleapis.com/syzbot-assets/9312d8ec05d3/bzImage-b04ae0f4.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/d4d1e4e89afc/mount_0.gz

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

workqueue: max_active 32767 requested for btrfs-compressed-write is out of range, clamping between 1 and 512
workqueue: max_active 32767 requested for btrfs-scrub is out of range, clamping between 1 and 512
BTRFS info (device loop0 state CS): scrub: started on devid 1
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000041: 0000 [#1] PREEMPT SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000208-0x000000000000020f]
CPU: 0 UID: 0 PID: 5110 Comm: syz-executor381 Not tainted 6.12.0-rc3-syzkaller-00319-gb04ae0f45168 #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:btrfs_lookup_csums_bitmap+0xc4/0x1600 fs/btrfs/file-item.c:615
Code: 8c 24 a8 00 00 00 42 c7 44 31 08 f3 f3 f3 f3 e8 d2 83 e1 fd 48 89 9c 24 88 00 00 00 48 81 c3 08 02 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 30 00 74 08 48 89 df e8 9d 39 4b fe 4c 8b 2b ba 11 00 00
RSP: 0018:ffffc9000af5f100 EFLAGS: 00010206
RAX: 0000000000000041 RBX: 0000000000000208 RCX: ffff888000cf2440
RDX: 0000000000000000 RSI: ffff888047132080 RDI: 0000000000000000
RBP: ffffc9000af5f290 R08: ffff88801fb3c800 R09: ffffc9000af5f420
R10: dffffc0000000000 R11: ffffed1008e2402e R12: 0000000000500000
R13: ffffc9000af5f420 R14: dffffc0000000000 R15: 0000000000500000
FS:  00005555764d5480(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055572fc64400 CR3: 0000000040a06000 CR4: 0000000000352ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 scrub_find_fill_first_stripe+0xe96/0x1200 fs/btrfs/scrub.c:1618
 queue_scrub_stripe fs/btrfs/scrub.c:1912 [inline]
 scrub_simple_mirror+0x5c6/0x960 fs/btrfs/scrub.c:2144
 scrub_stripe+0xa7a/0x2a60 fs/btrfs/scrub.c:2310
 scrub_chunk+0x2e3/0x470 fs/btrfs/scrub.c:2442
 scrub_enumerate_chunks+0xc4f/0x16a0 fs/btrfs/scrub.c:2706
 btrfs_scrub_dev+0x774/0xde0 fs/btrfs/scrub.c:3028
 btrfs_ioctl_scrub+0x236/0x370 fs/btrfs/ioctl.c:3251
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:907 [inline]
 __se_sys_ioctl+0xf9/0x170 fs/ioctl.c:893
 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:0x7f4e99a28f19
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 17 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 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:00007ffcb799b9b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f4e99a28f19
RDX: 0000000020000000 RSI: 00000000c400941b RDI: 0000000000000004
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffcb799ba00
R13: 00007ffcb799bc88 R14: 431bde82d7b634db R15: 00007f4e99a7103b
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:btrfs_lookup_csums_bitmap+0xc4/0x1600 fs/btrfs/file-item.c:615
Code: 8c 24 a8 00 00 00 42 c7 44 31 08 f3 f3 f3 f3 e8 d2 83 e1 fd 48 89 9c 24 88 00 00 00 48 81 c3 08 02 00 00 48 89 d8 48 c1 e8 03 <42> 80 3c 30 00 74 08 48 89 df e8 9d 39 4b fe 4c 8b 2b ba 11 00 00
RSP: 0018:ffffc9000af5f100 EFLAGS: 00010206
RAX: 0000000000000041 RBX: 0000000000000208 RCX: ffff888000cf2440
RDX: 0000000000000000 RSI: ffff888047132080 RDI: 0000000000000000
RBP: ffffc9000af5f290 R08: ffff88801fb3c800 R09: ffffc9000af5f420
R10: dffffc0000000000 R11: ffffed1008e2402e R12: 0000000000500000
R13: ffffc9000af5f420 R14: dffffc0000000000 R15: 0000000000500000
FS:  00005555764d5480(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055572fc64400 CR3: 0000000040a06000 CR4: 0000000000352ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
   0:	8c 24 a8             	mov    %fs,(%rax,%rbp,4)
   3:	00 00                	add    %al,(%rax)
   5:	00 42 c7             	add    %al,-0x39(%rdx)
   8:	44 31 08             	xor    %r9d,(%rax)
   b:	f3 f3 f3 f3 e8 d2 83 	repz repz repz repz call 0xfde183e6
  12:	e1 fd
  14:	48 89 9c 24 88 00 00 	mov    %rbx,0x88(%rsp)
  1b:	00
  1c:	48 81 c3 08 02 00 00 	add    $0x208,%rbx
  23:	48 89 d8             	mov    %rbx,%rax
  26:	48 c1 e8 03          	shr    $0x3,%rax
* 2a:	42 80 3c 30 00       	cmpb   $0x0,(%rax,%r14,1) <-- trapping instruction
  2f:	74 08                	je     0x39
  31:	48 89 df             	mov    %rbx,%rdi
  34:	e8 9d 39 4b fe       	call   0xfe4b39d6
  39:	4c 8b 2b             	mov    (%rbx),%r13
  3c:	ba                   	.byte 0xba
  3d:	11 00                	adc    %eax,(%rax)


---
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] [btrfs?] general protection fault in btrfs_lookup_csums_bitmap

[Edward Adam Davis]

Did we load the csum root?

#syz test

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 3a3427428074..1ba4d8ba902b 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -1602,7 +1602,8 @@ static int scrub_find_fill_first_stripe(struct btrfs_block_group *bg,
 	}
 
 	/* Now fill the data csum. */
-	if (bg->flags & BTRFS_BLOCK_GROUP_DATA) {
+	if (!test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
+	    bg->flags & BTRFS_BLOCK_GROUP_DATA) {
 		int sector_nr;
 		unsigned long csum_bitmap = 0;
 

Re: [syzbot] [btrfs?] general protection fault in btrfs_lookup_csums_bitmap

[syzbot]

Hello,

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

Reported-by: syzbot+5d2b33d7835870519b5f@syzkaller.appspotmail.com
Tested-by: syzbot+5d2b33d7835870519b5f@syzkaller.appspotmail.com

Tested on:

commit:         c2ee9f59 KVM: selftests: Fix build on on non-x86 archi..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1278c287980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=fc6f8ce8c5369043
dashboard link: https://syzkaller.appspot.com/bug?extid=5d2b33d7835870519b5f
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=12356640580000

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

[PATCH] btrfs: add a sanity check for csum root before fill the data csum

[Edward Adam Davis]

Syzbot reported a null-ptr-deref in btrfs_lookup_csums_bitmap.
The btrfs info contains IGNOREDATACSUMS, which prevents the csum root from
being loaded.
Before filling in the csum data, check the flag BTRFS_FS_STATE_NO_DATA_CSUMS
to confirm that the csum root has been loaded.

Reported-and-tested-by: syzbot+5d2b33d7835870519b5f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5d2b33d7835870519b5f
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 fs/btrfs/scrub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 3a3427428074..1ba4d8ba902b 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -1602,7 +1602,8 @@ static int scrub_find_fill_first_stripe(struct btrfs_block_group *bg,
 	}
 
 	/* Now fill the data csum. */
-	if (bg->flags & BTRFS_BLOCK_GROUP_DATA) {
+	if (!test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
+	    bg->flags & BTRFS_BLOCK_GROUP_DATA) {
 		int sector_nr;
 		unsigned long csum_bitmap = 0;
 
-- 
2.43.0

Re: [PATCH] btrfs: add a sanity check for csum root before fill the data csum

[Qu Wenruo]

在 2024/10/23 21:34, Edward Adam Davis 写道:
> Syzbot reported a null-ptr-deref in btrfs_lookup_csums_bitmap.
> The btrfs info contains IGNOREDATACSUMS, which prevents the csum root from
> being loaded.
> Before filling in the csum data, check the flag BTRFS_FS_STATE_NO_DATA_CSUMS
> to confirm that the csum root has been loaded.
>
> Reported-and-tested-by: syzbot+5d2b33d7835870519b5f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5d2b33d7835870519b5f
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/scrub.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 3a3427428074..1ba4d8ba902b 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -1602,7 +1602,8 @@ static int scrub_find_fill_first_stripe(struct btrfs_block_group *bg,
>   	}
>
>   	/* Now fill the data csum. */
> -	if (bg->flags & BTRFS_BLOCK_GROUP_DATA) {
> +	if (!test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
> +	    bg->flags & BTRFS_BLOCK_GROUP_DATA) {
>   		int sector_nr;
>   		unsigned long csum_bitmap = 0;
>

Re: [PATCH] btrfs: add a sanity check for csum root before fill the data csum

[David Sterba]

On Wed, Oct 23, 2024 at 07:04:40PM +0800, Edward Adam Davis wrote:
> Syzbot reported a null-ptr-deref in btrfs_lookup_csums_bitmap.
> The btrfs info contains IGNOREDATACSUMS, which prevents the csum root from
> being loaded.
> Before filling in the csum data, check the flag BTRFS_FS_STATE_NO_DATA_CSUMS
> to confirm that the csum root has been loaded.
> 
> Reported-and-tested-by: syzbot+5d2b33d7835870519b5f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5d2b33d7835870519b5f
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>

Added to for-next, thanks.

> ---
>  fs/btrfs/scrub.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 3a3427428074..1ba4d8ba902b 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -1602,7 +1602,8 @@ static int scrub_find_fill_first_stripe(struct btrfs_block_group *bg,
>  	}
>  
>  	/* Now fill the data csum. */
> -	if (bg->flags & BTRFS_BLOCK_GROUP_DATA) {
> +	if (!test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&

I've updatd the coment as this is double negation that could be
confusing on a quick read.

> +	    bg->flags & BTRFS_BLOCK_GROUP_DATA) {
>  		int sector_nr;
>  		unsigned long csum_bitmap = 0;
>  
> -- 
> 2.43.0
> 

Re: [PATCH] btrfs: add a sanity check for csum root before fill the data csum

[Qu Wenruo]

在 2024/10/26 05:14, David Sterba 写道:
> On Wed, Oct 23, 2024 at 07:04:40PM +0800, Edward Adam Davis wrote:
>> Syzbot reported a null-ptr-deref in btrfs_lookup_csums_bitmap.
>> The btrfs info contains IGNOREDATACSUMS, which prevents the csum root from
>> being loaded.
>> Before filling in the csum data, check the flag BTRFS_FS_STATE_NO_DATA_CSUMS
>> to confirm that the csum root has been loaded.
>>
>> Reported-and-tested-by: syzbot+5d2b33d7835870519b5f@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=5d2b33d7835870519b5f
>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>
> Added to for-next, thanks.

Wait for a second, I believe LiZhi Xu's solution is better.

And sorry I didn't notice that until his patch is submitted.

The problem for this fix is, although it fixes the crash, it also gives
a false feel of safety that scrub is finding nothing wrong.

But the truth is, there is no csum root, and everything can go wrong.

Thus I'd prefer LiZhi's solution which error out and terminate the scrub
immediately.

Thanks,
Qu
>
>> ---
>>   fs/btrfs/scrub.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
>> index 3a3427428074..1ba4d8ba902b 100644
>> --- a/fs/btrfs/scrub.c
>> +++ b/fs/btrfs/scrub.c
>> @@ -1602,7 +1602,8 @@ static int scrub_find_fill_first_stripe(struct btrfs_block_group *bg,
>>   	}
>>
>>   	/* Now fill the data csum. */
>> -	if (bg->flags & BTRFS_BLOCK_GROUP_DATA) {
>> +	if (!test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
>
> I've updatd the coment as this is double negation that could be
> confusing on a quick read.
>
>> +	    bg->flags & BTRFS_BLOCK_GROUP_DATA) {
>>   		int sector_nr;
>>   		unsigned long csum_bitmap = 0;
>>
>> --
>> 2.43.0
>>
>

Re: [PATCH] btrfs: add a sanity check for csum root before fill the data csum

[David Sterba]

On Sat, Oct 26, 2024 at 07:45:18AM +1030, Qu Wenruo wrote:
> 
> 
> 在 2024/10/26 05:14, David Sterba 写道:
> > On Wed, Oct 23, 2024 at 07:04:40PM +0800, Edward Adam Davis wrote:
> >> Syzbot reported a null-ptr-deref in btrfs_lookup_csums_bitmap.
> >> The btrfs info contains IGNOREDATACSUMS, which prevents the csum root from
> >> being loaded.
> >> Before filling in the csum data, check the flag BTRFS_FS_STATE_NO_DATA_CSUMS
> >> to confirm that the csum root has been loaded.
> >>
> >> Reported-and-tested-by: syzbot+5d2b33d7835870519b5f@syzkaller.appspotmail.com
> >> Closes: https://syzkaller.appspot.com/bug?extid=5d2b33d7835870519b5f
> >> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> >
> > Added to for-next, thanks.
> 
> Wait for a second, I believe LiZhi Xu's solution is better.
> 
> And sorry I didn't notice that until his patch is submitted.
> 
> The problem for this fix is, although it fixes the crash, it also gives
> a false feel of safety that scrub is finding nothing wrong.
> 
> But the truth is, there is no csum root, and everything can go wrong.
> 
> Thus I'd prefer LiZhi's solution which error out and terminate the scrub
> immediately.

Ok, I've dropped this patch from for-next. Please add "btrfs: add a
sanity check for btrfs root".