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 <wangyugui@e16-tech.com>
Issue: #412
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This commit is contained in:
Qu Wenruo 2021-11-01 19:30:17 +08:00 committed by David Sterba
parent cc8a100c90
commit 8f81113021
2 changed files with 13 additions and 4 deletions

View file

@ -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 */

View file

@ -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;
}