From 8f81113021ecf36db4694f03aed1c1c98e2b3f70 Mon Sep 17 00:00:00 2001 From: Qu Wenruo Date: Mon, 1 Nov 2021 19:30:17 +0800 Subject: [PATCH] btrfs-progs: check: fix a lowmem mode crash where fatal error is not properly handled [BUG] When a special image (diverted from fsck/012) has its unused slots (slot number >= nritems) with garbage, lowmem mode btrfs check can crash: (gdb) run check --mode=lowmem ~/downloads/good.img.restored Starting program: /home/adam/btrfs/btrfs-progs/btrfs check --mode=lowmem ~/downloads/good.img.restored ... ERROR: root 5 INODE[5044031582654955520] nlink(257228800) not equal to inode_refs(0) ERROR: root 5 INODE[5044031582654955520] nbytes 474624 not equal to extent_size 0 Program received signal SIGSEGV, Segmentation fault. 0x0000555555639b11 in btrfs_inode_size (eb=0x5555558a7540, s=0x642e6cd1) at ./kernel-shared/ctree.h:1703 1703 BTRFS_SETGET_FUNCS(inode_size, struct btrfs_inode_item, size, 64); (gdb) bt #0 0x0000555555639b11 in btrfs_inode_size (eb=0x5555558a7540, s=0x642e6cd1) at ./kernel-shared/ctree.h:1703 #1 0x0000555555641544 in check_inode_item (root=0x5555556c2290, path=0x7fffffffd960) at check/mode-lowmem.c:2628 [CAUSE] At check_inode_item() we have path->slot[0] at 29, while the tree block only has 26 items. This happens because two reasons: - btrfs_next_item() never reverts its slots Even if we failed to read next leaf. - check_inode_item() doesn't inform the caller that a fatal error happened In check_inode_item(), if btrfs_next_item() failed, it goes to out label, which doesn't really set @err properly. This means, when check_inode_item() fails at btrfs_next_item(), it will increase path->slots[0], while it's already beyond current tree block nritems. When the slot increases furthermore, and if the unused item slots have some garbage, we will get invalid btrfs_item_ptr() result, and causing above segfault. [FIX] Fix the problems by two ways: - Make btrfs_next_item() to revert its path->slots[0] on failure - Properly detect fatal error from check_inode_item() By this, we will no longer crash on the crafted image. Reported-by: Wang Yugui Issue: #412 Signed-off-by: Qu Wenruo Signed-off-by: David Sterba --- check/mode-lowmem.c | 4 ++-- kernel-shared/ctree.h | 13 +++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c index b88b2b19..696ad215 100644 --- a/check/mode-lowmem.c +++ b/check/mode-lowmem.c @@ -2686,7 +2686,7 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path) ret = -EIO; if (ret < 0) { - /* out will fill 'err' rusing current statistics */ + err |= FATAL_ERROR; goto out; } else if (ret > 0) { err |= LAST_ITEM; @@ -2898,7 +2898,7 @@ again: /* modify cur since check_inode_item may change path */ cur = path->nodes[0]; - if (err & LAST_ITEM) + if (err & LAST_ITEM || err & FATAL_ERROR) goto out; /* still have inode items in this leaf */ diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h index 563ea50b..8af09623 100644 --- a/kernel-shared/ctree.h +++ b/kernel-shared/ctree.h @@ -2729,8 +2729,17 @@ static inline int btrfs_next_item(struct btrfs_root *root, struct btrfs_path *p) { ++p->slots[0]; - if (p->slots[0] >= btrfs_header_nritems(p->nodes[0])) - return btrfs_next_leaf(root, p); + if (p->slots[0] >= btrfs_header_nritems(p->nodes[0])) { + int ret; + ret = btrfs_next_leaf(root, p); + /* + * Revert the increased slot, or the path may point to + * an invalid item. + */ + if (ret) + p->slots[0]--; + return ret; + } return 0; }