diff mbox series

[v2] btrfs: fix overflow when copying corrupt csums for a message

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

Commit Message

Johannes Thumshirn Sept. 21, 2020, 7:57 a.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
  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
  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:

[   35.726623] BTRFS: device fsid 815caf9a-dc43-4d2a-ac54-764b8333d765 devid 1 transid 5 /dev/loop0 scanned by syz-repro (215)
[   35.738994] BTRFS info (device loop0): disk space caching is enabled
[   35.738998] BTRFS info (device loop0): has skinny extents
[   35.743337] BTRFS warning (device loop0): loop0 checksum verify failed on 1052672 wanted 0xf9c035fc8d239a54 found 0x67a25c14b7eabcf9 level 0
[   35.743420] BTRFS error (device loop0): failed to read chunk root
[   35.745899] BTRFS error (device loop0): open_ctree failed

Reported-by: syzbot+e864a35d361e1d4e29a5@syzkaller.appspotmail.com
Fixes: d5178578bcd4 ("btrfs: directly call into crypto framework for checksumming")
CC: stable@vger.kernel.org # 5.4+
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

---
Changes to v1:
- Use CSUM_FMT (David)
- Fix the 2nd possible overflow (David)
- Remove some unnecessary local variables and memcpy (David)
- Update commit log to have the now correct new log entries
---
 fs/btrfs/disk-io.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

David Sterba Sept. 21, 2020, 9:42 a.m. UTC | #1
On Mon, Sep 21, 2020 at 04:57:14PM +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
>   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
>   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:
> 
> [   35.726623] BTRFS: device fsid 815caf9a-dc43-4d2a-ac54-764b8333d765 devid 1 transid 5 /dev/loop0 scanned by syz-repro (215)
> [   35.738994] BTRFS info (device loop0): disk space caching is enabled
> [   35.738998] BTRFS info (device loop0): has skinny extents
> [   35.743337] BTRFS warning (device loop0): loop0 checksum verify failed on 1052672 wanted 0xf9c035fc8d239a54 found 0x67a25c14b7eabcf9 level 0
> [   35.743420] BTRFS error (device loop0): failed to read chunk root
> [   35.745899] BTRFS error (device loop0): open_ctree failed
> 
> Reported-by: syzbot+e864a35d361e1d4e29a5@syzkaller.appspotmail.com
> Fixes: d5178578bcd4 ("btrfs: directly call into crypto framework for checksumming")
> CC: stable@vger.kernel.org # 5.4+
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 
> ---
> Changes to v1:
> - Use CSUM_FMT (David)
> - Fix the 2nd possible overflow (David)
> - Remove some unnecessary local variables and memcpy (David)
> - Update commit log to have the now correct new log entries

Thanks, I'll take this as-is so it's the obvious minimal fix, there are
a few more cleanups possible.
diff mbox series

Patch

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 160b485d2cc0..53b588a7e150 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -586,16 +586,15 @@  static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
 	csum_tree_block(eb, result);
 
 	if (memcmp_extent_buffer(eb, result, 0, csum_size)) {
-		u32 val;
-		u32 found = 0;
-
-		memcpy(&found, result, csum_size);
+		u8 val[BTRFS_CSUM_SIZE] = { 0 };
 
 		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 " CSUM_FMT " found " CSUM_FMT " level %d",
 			      fs_info->sb->s_id, eb->start,
-			      val, found, btrfs_header_level(eb));
+			      CSUM_FMT_VALUE(csum_size, val),
+			      CSUM_FMT_VALUE(csum_size, result),
+			      btrfs_header_level(eb));
 		ret = -EUCLEAN;
 		goto err;
 	}