diff mbox series

btrfs: fix overflow when copying corrupt csums

Message ID 20200915144931.24555-1-johannes.thumshirn@wdc.com (mailing list archive)
State New, archived
Headers show
Series btrfs: fix overflow when copying corrupt csums | expand

Commit Message

Johannes Thumshirn Sept. 15, 2020, 2:49 p.m. UTC
Syzkaller reported a buffer overflow in btree_readpage_end_io_hook() when
loop mounting a crafted image:

detected buffer overflow in memcpy
------------[ cut here ]------------
kernel BUG at lib/string.c:1129!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 26 Comm: kworker/u4:2 Not tainted 5.9.0-rc4-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: btrfs-endio-meta btrfs_work_helper
RIP: 0010:fortify_panic+0xf/0x20 lib/string.c:1129
Code: 89 c7 48 89 74 24 08 48 89 04 24 e8 ab 39 00 fe 48 8b 74 24 08 48 8b 04 24 eb d5 48 89 fe 48 c7 c7 40 22 97 88 e8 b0 8c a9 fd <0f> 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 41 57 41 56 41
RSP: 0018:ffffc90000e27980 EFLAGS: 00010286
RAX: 0000000000000022 RBX: ffff8880a80dca64 RCX: 0000000000000000
RDX: ffff8880a90860c0 RSI: ffffffff815dba07 RDI: fffff520001c4f22
RBP: ffff8880a80dca00 R08: 0000000000000022 R09: ffff8880ae7318e7
R10: 0000000000000000 R11: 0000000000077578 R12: 00000000ffffff6e
R13: 0000000000000008 R14: ffffc90000e27a40 R15: 1ffff920001c4f3c
FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000557335f440d0 CR3: 000000009647d000 CR4: 00000000001506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 memcpy include/linux/string.h:405 [inline]
 btree_readpage_end_io_hook.cold+0x206/0x221 fs/btrfs/disk-io.c:642
 end_bio_extent_readpage+0x4de/0x10c0 fs/btrfs/extent_io.c:2854
 bio_endio+0x3cf/0x7f0 block/bio.c:1449
 end_workqueue_fn+0x114/0x170 fs/btrfs/disk-io.c:1695
 btrfs_work_helper+0x221/0xe20 fs/btrfs/async-thread.c:318
 process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
 worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
 kthread+0x3b5/0x4a0 kernel/kthread.c:292
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
Modules linked in:
---[ end trace b68924293169feef ]---
RIP: 0010:fortify_panic+0xf/0x20 lib/string.c:1129
Code: 89 c7 48 89 74 24 08 48 89 04 24 e8 ab 39 00 fe 48 8b 74 24 08 48 8b 04 24 eb d5 48 89 fe 48 c7 c7 40 22 97 88 e8 b0 8c a9 fd <0f> 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 41 57 41 56 41
RSP: 0018:ffffc90000e27980 EFLAGS: 00010286
RAX: 0000000000000022 RBX: ffff8880a80dca64 RCX: 0000000000000000
RDX: ffff8880a90860c0 RSI: ffffffff815dba07 RDI: fffff520001c4f22
RBP: ffff8880a80dca00 R08: 0000000000000022 R09: ffff8880ae7318e7
R10: 0000000000000000 R11: 0000000000077578 R12: 00000000ffffff6e
R13: 0000000000000008 R14: ffffc90000e27a40 R15: 1ffff920001c4f3c
FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f95b7c4d008 CR3: 000000009647d000 CR4: 00000000001506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400

The overflow happens, because in btree_readpage_end_io_hook() we assume that
we have found a 4 byte checksum instead of the real possible 32 bytes we
have for the checksums.

With the fix applied:

BTRFS: device fsid 815caf9a-dc43-4d2a-ac54-764b8333d765 devid 1 transid 5 /dev/loop0 scanned by syz-repro (214)
BTRFS info (device loop0): disk space caching is enabled
BTRFS info (device loop0): has skinny extents
BTRFS warning (device loop0): loop0 checksum verify failed on 1052672 wanted fc35c0f9 found 4b0bbc71 level 0
BTRFS error (device loop0): failed to read chunk root
BTRFS error (device loop0): open_ctree failed

Fixes: d5178578bcd4 ("btrfs: directly call into crypto framework for checksumming")
Reported-by: syzbot+e864a35d361e1d4e29a5@syzkaller.appspotmail.com
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/disk-io.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Josef Bacik Sept. 15, 2020, 5:13 p.m. UTC | #1
On 9/15/20 10:49 AM, Johannes Thumshirn wrote:
> Syzkaller reported a buffer overflow in btree_readpage_end_io_hook() when
> loop mounting a crafted image:
> 
> detected buffer overflow in memcpy
> ------------[ cut here ]------------
> kernel BUG at lib/string.c:1129!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 26 Comm: kworker/u4:2 Not tainted 5.9.0-rc4-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Workqueue: btrfs-endio-meta btrfs_work_helper
> RIP: 0010:fortify_panic+0xf/0x20 lib/string.c:1129
> Code: 89 c7 48 89 74 24 08 48 89 04 24 e8 ab 39 00 fe 48 8b 74 24 08 48 8b 04 24 eb d5 48 89 fe 48 c7 c7 40 22 97 88 e8 b0 8c a9 fd <0f> 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 41 57 41 56 41
> RSP: 0018:ffffc90000e27980 EFLAGS: 00010286
> RAX: 0000000000000022 RBX: ffff8880a80dca64 RCX: 0000000000000000
> RDX: ffff8880a90860c0 RSI: ffffffff815dba07 RDI: fffff520001c4f22
> RBP: ffff8880a80dca00 R08: 0000000000000022 R09: ffff8880ae7318e7
> R10: 0000000000000000 R11: 0000000000077578 R12: 00000000ffffff6e
> R13: 0000000000000008 R14: ffffc90000e27a40 R15: 1ffff920001c4f3c
> FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000557335f440d0 CR3: 000000009647d000 CR4: 00000000001506e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>   memcpy include/linux/string.h:405 [inline]
>   btree_readpage_end_io_hook.cold+0x206/0x221 fs/btrfs/disk-io.c:642
>   end_bio_extent_readpage+0x4de/0x10c0 fs/btrfs/extent_io.c:2854
>   bio_endio+0x3cf/0x7f0 block/bio.c:1449
>   end_workqueue_fn+0x114/0x170 fs/btrfs/disk-io.c:1695
>   btrfs_work_helper+0x221/0xe20 fs/btrfs/async-thread.c:318
>   process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
>   worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
>   kthread+0x3b5/0x4a0 kernel/kthread.c:292
>   ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
> Modules linked in:
> ---[ end trace b68924293169feef ]---
> RIP: 0010:fortify_panic+0xf/0x20 lib/string.c:1129
> Code: 89 c7 48 89 74 24 08 48 89 04 24 e8 ab 39 00 fe 48 8b 74 24 08 48 8b 04 24 eb d5 48 89 fe 48 c7 c7 40 22 97 88 e8 b0 8c a9 fd <0f> 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 41 57 41 56 41
> RSP: 0018:ffffc90000e27980 EFLAGS: 00010286
> RAX: 0000000000000022 RBX: ffff8880a80dca64 RCX: 0000000000000000
> RDX: ffff8880a90860c0 RSI: ffffffff815dba07 RDI: fffff520001c4f22
> RBP: ffff8880a80dca00 R08: 0000000000000022 R09: ffff8880ae7318e7
> R10: 0000000000000000 R11: 0000000000077578 R12: 00000000ffffff6e
> R13: 0000000000000008 R14: ffffc90000e27a40 R15: 1ffff920001c4f3c
> FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f95b7c4d008 CR3: 000000009647d000 CR4: 00000000001506e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> 
> The overflow happens, because in btree_readpage_end_io_hook() we assume that
> we have found a 4 byte checksum instead of the real possible 32 bytes we
> have for the checksums.
> 
> With the fix applied:
> 
> BTRFS: device fsid 815caf9a-dc43-4d2a-ac54-764b8333d765 devid 1 transid 5 /dev/loop0 scanned by syz-repro (214)
> BTRFS info (device loop0): disk space caching is enabled
> BTRFS info (device loop0): has skinny extents
> BTRFS warning (device loop0): loop0 checksum verify failed on 1052672 wanted fc35c0f9 found 4b0bbc71 level 0
> BTRFS error (device loop0): failed to read chunk root
> BTRFS error (device loop0): open_ctree failed
> 
> Fixes: d5178578bcd4 ("btrfs: directly call into crypto framework for checksumming")
> Reported-by: syzbot+e864a35d361e1d4e29a5@syzkaller.appspotmail.com
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
Anand Jain Sept. 16, 2020, 1:08 a.m. UTC | #2
On 15/9/20 10:49 pm, Johannes Thumshirn wrote:
> Syzkaller reported a buffer overflow in btree_readpage_end_io_hook() when
> loop mounting a crafted image:
> 
> detected buffer overflow in memcpy
> ------------[ cut here ]------------
> kernel BUG at lib/string.c:1129!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 26 Comm: kworker/u4:2 Not tainted 5.9.0-rc4-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Workqueue: btrfs-endio-meta btrfs_work_helper
> RIP: 0010:fortify_panic+0xf/0x20 lib/string.c:1129
> Code: 89 c7 48 89 74 24 08 48 89 04 24 e8 ab 39 00 fe 48 8b 74 24 08 48 8b 04 24 eb d5 48 89 fe 48 c7 c7 40 22 97 88 e8 b0 8c a9 fd <0f> 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 41 57 41 56 41
> RSP: 0018:ffffc90000e27980 EFLAGS: 00010286
> RAX: 0000000000000022 RBX: ffff8880a80dca64 RCX: 0000000000000000
> RDX: ffff8880a90860c0 RSI: ffffffff815dba07 RDI: fffff520001c4f22
> RBP: ffff8880a80dca00 R08: 0000000000000022 R09: ffff8880ae7318e7
> R10: 0000000000000000 R11: 0000000000077578 R12: 00000000ffffff6e
> R13: 0000000000000008 R14: ffffc90000e27a40 R15: 1ffff920001c4f3c
> FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000557335f440d0 CR3: 000000009647d000 CR4: 00000000001506e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>   memcpy include/linux/string.h:405 [inline]
>   btree_readpage_end_io_hook.cold+0x206/0x221 fs/btrfs/disk-io.c:642
>   end_bio_extent_readpage+0x4de/0x10c0 fs/btrfs/extent_io.c:2854
>   bio_endio+0x3cf/0x7f0 block/bio.c:1449
>   end_workqueue_fn+0x114/0x170 fs/btrfs/disk-io.c:1695
>   btrfs_work_helper+0x221/0xe20 fs/btrfs/async-thread.c:318
>   process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
>   worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
>   kthread+0x3b5/0x4a0 kernel/kthread.c:292
>   ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
> Modules linked in:
> ---[ end trace b68924293169feef ]---
> RIP: 0010:fortify_panic+0xf/0x20 lib/string.c:1129
> Code: 89 c7 48 89 74 24 08 48 89 04 24 e8 ab 39 00 fe 48 8b 74 24 08 48 8b 04 24 eb d5 48 89 fe 48 c7 c7 40 22 97 88 e8 b0 8c a9 fd <0f> 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 41 57 41 56 41
> RSP: 0018:ffffc90000e27980 EFLAGS: 00010286
> RAX: 0000000000000022 RBX: ffff8880a80dca64 RCX: 0000000000000000
> RDX: ffff8880a90860c0 RSI: ffffffff815dba07 RDI: fffff520001c4f22
> RBP: ffff8880a80dca00 R08: 0000000000000022 R09: ffff8880ae7318e7
> R10: 0000000000000000 R11: 0000000000077578 R12: 00000000ffffff6e
> R13: 0000000000000008 R14: ffffc90000e27a40 R15: 1ffff920001c4f3c
> FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f95b7c4d008 CR3: 000000009647d000 CR4: 00000000001506e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> 
> The overflow happens, because in btree_readpage_end_io_hook() we assume that
> we have found a 4 byte checksum instead of the real possible 32 bytes we
> have for the checksums.
> 
> With the fix applied:
> 
> BTRFS: device fsid 815caf9a-dc43-4d2a-ac54-764b8333d765 devid 1 transid 5 /dev/loop0 scanned by syz-repro (214)
> BTRFS info (device loop0): disk space caching is enabled
> BTRFS info (device loop0): has skinny extents
> BTRFS warning (device loop0): loop0 checksum verify failed on 1052672 wanted fc35c0f9 found 4b0bbc71 level 0
> BTRFS error (device loop0): failed to read chunk root
> BTRFS error (device loop0): open_ctree failed
> 
> Fixes: d5178578bcd4 ("btrfs: directly call into crypto framework for checksumming")
> Reported-by: syzbot+e864a35d361e1d4e29a5@syzkaller.appspotmail.com
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

The size is fixed.
Reviewed-by: Anand Jain <anand.jain@oracle.com>

nit below.

> ---
>   fs/btrfs/disk-io.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 160b485d2cc0..28b962840972 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -587,15 +587,15 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
>   
>   	if (memcmp_extent_buffer(eb, result, 0, csum_size)) {
>   		u32 val;
> -		u32 found = 0;
> +		u8 found[BTRFS_CSUM_SIZE] = { };


u8 found[BTRFS_CSUM_SIZE] = {0}; not required?

Thanks, Anand

>   
>   		memcpy(&found, result, csum_size);
>   
>   		read_extent_buffer(eb, &val, 0, csum_size);
>   		btrfs_warn_rl(fs_info,
> -		"%s checksum verify failed on %llu wanted %x found %x level %d",
> +		"%s checksum verify failed on %llu wanted %x found %*pH level %d",
>   			      fs_info->sb->s_id, eb->start,
> -			      val, found, btrfs_header_level(eb));
> +			      val, csum_size, found, btrfs_header_level(eb));
>   		ret = -EUCLEAN;
>   		goto err;
>   	}
>
David Sterba Sept. 16, 2020, 9:52 a.m. UTC | #3
On Wed, Sep 16, 2020 at 09:08:56AM +0800, Anand Jain wrote:
> On 15/9/20 10:49 pm, Johannes Thumshirn wrote:
> > index 160b485d2cc0..28b962840972 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -587,15 +587,15 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
> >   
> >   	if (memcmp_extent_buffer(eb, result, 0, csum_size)) {
> >   		u32 val;
> > -		u32 found = 0;
> > +		u8 found[BTRFS_CSUM_SIZE] = { };
> 
> 
> u8 found[BTRFS_CSUM_SIZE] = {0}; not required?

Yes, { 0 } is the initializer we use, I fixed that.
> 
> >   
> >   		memcpy(&found, result, csum_size);
> >   
> >   		read_extent_buffer(eb, &val, 0, csum_size);
> >   		btrfs_warn_rl(fs_info,
> > -		"%s checksum verify failed on %llu wanted %x found %x level %d",
> > +		"%s checksum verify failed on %llu wanted %x found %*pH level %d",
> >   			      fs_info->sb->s_id, eb->start,
> > -			      val, found, btrfs_header_level(eb));
> > +			      val, csum_size, found, btrfs_header_level(eb));

And the format of csum as well.
David Sterba Sept. 16, 2020, 9:52 a.m. UTC | #4
On Tue, Sep 15, 2020 at 11:49:31PM +0900, Johannes Thumshirn wrote:
> Syzkaller reported a buffer overflow in btree_readpage_end_io_hook() when
> loop mounting a crafted image:
> 
> detected buffer overflow in memcpy
> ------------[ cut here ]------------
> kernel BUG at lib/string.c:1129!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 26 Comm: kworker/u4:2 Not tainted 5.9.0-rc4-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Workqueue: btrfs-endio-meta btrfs_work_helper
> RIP: 0010:fortify_panic+0xf/0x20 lib/string.c:1129
> Code: 89 c7 48 89 74 24 08 48 89 04 24 e8 ab 39 00 fe 48 8b 74 24 08 48 8b 04 24 eb d5 48 89 fe 48 c7 c7 40 22 97 88 e8 b0 8c a9 fd <0f> 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 41 57 41 56 41
> RSP: 0018:ffffc90000e27980 EFLAGS: 00010286
> RAX: 0000000000000022 RBX: ffff8880a80dca64 RCX: 0000000000000000
> RDX: ffff8880a90860c0 RSI: ffffffff815dba07 RDI: fffff520001c4f22
> RBP: ffff8880a80dca00 R08: 0000000000000022 R09: ffff8880ae7318e7
> R10: 0000000000000000 R11: 0000000000077578 R12: 00000000ffffff6e
> R13: 0000000000000008 R14: ffffc90000e27a40 R15: 1ffff920001c4f3c
> FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000557335f440d0 CR3: 000000009647d000 CR4: 00000000001506e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  memcpy include/linux/string.h:405 [inline]
>  btree_readpage_end_io_hook.cold+0x206/0x221 fs/btrfs/disk-io.c:642
>  end_bio_extent_readpage+0x4de/0x10c0 fs/btrfs/extent_io.c:2854
>  bio_endio+0x3cf/0x7f0 block/bio.c:1449
>  end_workqueue_fn+0x114/0x170 fs/btrfs/disk-io.c:1695
>  btrfs_work_helper+0x221/0xe20 fs/btrfs/async-thread.c:318
>  process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
>  worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
>  kthread+0x3b5/0x4a0 kernel/kthread.c:292
>  ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
> Modules linked in:
> ---[ end trace b68924293169feef ]---
> RIP: 0010:fortify_panic+0xf/0x20 lib/string.c:1129
> Code: 89 c7 48 89 74 24 08 48 89 04 24 e8 ab 39 00 fe 48 8b 74 24 08 48 8b 04 24 eb d5 48 89 fe 48 c7 c7 40 22 97 88 e8 b0 8c a9 fd <0f> 0b cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 41 57 41 56 41
> RSP: 0018:ffffc90000e27980 EFLAGS: 00010286
> RAX: 0000000000000022 RBX: ffff8880a80dca64 RCX: 0000000000000000
> RDX: ffff8880a90860c0 RSI: ffffffff815dba07 RDI: fffff520001c4f22
> RBP: ffff8880a80dca00 R08: 0000000000000022 R09: ffff8880ae7318e7
> R10: 0000000000000000 R11: 0000000000077578 R12: 00000000ffffff6e
> R13: 0000000000000008 R14: ffffc90000e27a40 R15: 1ffff920001c4f3c
> FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f95b7c4d008 CR3: 000000009647d000 CR4: 00000000001506e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> 
> The overflow happens, because in btree_readpage_end_io_hook() we assume that
> we have found a 4 byte checksum instead of the real possible 32 bytes we
> have for the checksums.
> 
> With the fix applied:
> 
> BTRFS: device fsid 815caf9a-dc43-4d2a-ac54-764b8333d765 devid 1 transid 5 /dev/loop0 scanned by syz-repro (214)
> BTRFS info (device loop0): disk space caching is enabled
> BTRFS info (device loop0): has skinny extents
> BTRFS warning (device loop0): loop0 checksum verify failed on 1052672 wanted fc35c0f9 found 4b0bbc71 level 0
> BTRFS error (device loop0): failed to read chunk root
> BTRFS error (device loop0): open_ctree failed
> 
> Fixes: d5178578bcd4 ("btrfs: directly call into crypto framework for checksumming")
> Reported-by: syzbot+e864a35d361e1d4e29a5@syzkaller.appspotmail.com
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---

Added to misc-next, thanks.
David Sterba Sept. 18, 2020, 2:51 p.m. UTC | #5
On Tue, Sep 15, 2020 at 11:49:31PM +0900, Johannes Thumshirn wrote:
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -587,15 +587,15 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
>  
>  	if (memcmp_extent_buffer(eb, result, 0, csum_size)) {
>  		u32 val;
> -		u32 found = 0;
> +		u8 found[BTRFS_CSUM_SIZE] = { };
>  
>  		memcpy(&found, result, csum_size);

3x shame on us all giving it a reviewed-by: there are two buffer
overflows but the patch fixes just one.

'found' is filled from from the result

>  		read_extent_buffer(eb, &val, 0, csum_size);

and what about 'val' that's read from eb as well?

Funny is that no copying needs to be done at all, we can use 'result'
directly and also use the fact that checksum is at the beginning of the
first eb page and we can just set the pointer for the message.
diff mbox series

Patch

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 160b485d2cc0..28b962840972 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -587,15 +587,15 @@  static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
 
 	if (memcmp_extent_buffer(eb, result, 0, csum_size)) {
 		u32 val;
-		u32 found = 0;
+		u8 found[BTRFS_CSUM_SIZE] = { };
 
 		memcpy(&found, result, csum_size);
 
 		read_extent_buffer(eb, &val, 0, csum_size);
 		btrfs_warn_rl(fs_info,
-		"%s checksum verify failed on %llu wanted %x found %x level %d",
+		"%s checksum verify failed on %llu wanted %x found %*pH level %d",
 			      fs_info->sb->s_id, eb->start,
-			      val, found, btrfs_header_level(eb));
+			      val, csum_size, found, btrfs_header_level(eb));
 		ret = -EUCLEAN;
 		goto err;
 	}