btrfs-progs: check: Fix heap use after free
authorSu Yue <suy.fnst@cn.fujitsu.com>
Wed, 3 May 2017 01:50:14 +0000 (09:50 +0800)
committerDavid Sterba <dsterba@suse.com>
Mon, 15 May 2017 14:13:17 +0000 (16:13 +0200)
fsck/004-no-dir-index makes valgrinds complaining about Invalid read.
==31890== Invalid read of size 1
==31890==    at 0x453D09: repair_inode_backrefs (cmds-check.c:2690)
==31890==    by 0x453D09: check_inode_recs (cmds-check.c:3330)
==31890==    by 0x453D09: check_fs_root (cmds-check.c:4012)
==31890==    by 0x45E788: check_fs_roots (cmds-check.c:4098)
==31890==    by 0x45E788: cmd_check (cmds-check.c:13031)
==31890==    by 0x40A88A: main (btrfs.c:246)
==31890==  Address 0x5cb7b90 is 16 bytes inside a block of size 50 free'd
==31890==    at 0x4C2C14B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31890==    by 0x453D08: repair_inode_backrefs (cmds-check.c:2684)
==31890==    by 0x453D08: check_inode_recs (cmds-check.c:3330)
==31890==    by 0x453D08: check_fs_root (cmds-check.c:4012)
==31890==    by 0x45E788: check_fs_roots (cmds-check.c:4098)
==31890==    by 0x45E788: cmd_check (cmds-check.c:13031)
==31890==    by 0x40A88A: main (btrfs.c:246)
==31890==  Block was alloc'd at
==31890==    at 0x4C2AF1F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31890==    by 0x45055C: get_inode_backref (cmds-check.c:1075)
==31890==    by 0x45055C: add_inode_backref (cmds-check.c:1097)
==31890==    by 0x45180C: process_dir_item (cmds-check.c:1525)
==31890==    by 0x45180C: process_one_leaf (cmds-check.c:1838)
==31890==    by 0x45180C: walk_down_tree (cmds-check.c:2134)
==31890==    by 0x45180C: check_fs_root (cmds-check.c:3957)
==31890==    by 0x45E788: check_fs_roots (cmds-check.c:4098)
==31890==    by 0x45E788: cmd_check (cmds-check.c:13031)
==31890==    by 0x40A88A: main (btrfs.c:246)
==31890==
==31890== Invalid read of size 8
==31890==    at 0x452D66: repair_inode_backrefs (cmds-check.c:2731)
==31890==    by 0x452D66: check_inode_recs (cmds-check.c:3330)
==31890==    by 0x452D66: check_fs_root (cmds-check.c:4012)
==31890==    by 0x45E788: check_fs_roots (cmds-check.c:4098)
==31890==    by 0x45E788: cmd_check (cmds-check.c:13031)
==31890==    by 0x40A88A: main (btrfs.c:246)
==31890==  Address 0x5cb7b90 is 16 bytes inside a block of size 50 free'd
==31890==    at 0x4C2C14B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31890==    by 0x453D08: repair_inode_backrefs (cmds-check.c:2684)
==31890==    by 0x453D08: check_inode_recs (cmds-check.c:3330)
==31890==    by 0x453D08: check_fs_root (cmds-check.c:4012)
==31890==    by 0x45E788: check_fs_roots (cmds-check.c:4098)
==31890==    by 0x45E788: cmd_check (cmds-check.c:13031)
==31890==    by 0x40A88A: main (btrfs.c:246)
==31890==  Block was alloc'd at
==31890==    at 0x4C2AF1F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31890==    by 0x45055C: get_inode_backref (cmds-check.c:1075)
==31890==    by 0x45055C: add_inode_backref (cmds-check.c:1097)
==31890==    by 0x45180C: process_dir_item (cmds-check.c:1525)
==31890==    by 0x45180C: process_one_leaf (cmds-check.c:1838)
==31890==    by 0x45180C: walk_down_tree (cmds-check.c:2134)
==31890==    by 0x45180C: check_fs_root (cmds-check.c:3957)
==31890==    by 0x45E788: check_fs_roots (cmds-check.c:4098)
==31890==    by 0x45E788: cmd_check (cmds-check.c:13031)
==31890==    by 0x40A88A: main (btrfs.c:246)
==31890==

While iterating over backrefs in repair_inode_backrefs, there are
several situations to repair one backref according
backref->found_dir_item and backref->found_dir_index.  Two of these
branches may free the backref, but next checks will still access the
freed memory.

Because these branches are independent, let repair_inode_backrefs skip
to handle next backref after free can fix it.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
cmds-check.c

index afa7823..6b00201 100644 (file)
@@ -2667,6 +2667,7 @@ static int repair_inode_backrefs(struct btrfs_root *root,
                        repaired++;
                        list_del(&backref->list);
                        free(backref);
+                       continue;
                }
 
                if (!delete && !backref->found_dir_index &&
@@ -2677,12 +2678,12 @@ static int repair_inode_backrefs(struct btrfs_root *root,
                                break;
                        repaired++;
                        if (backref->found_dir_item &&
-                           backref->found_dir_index &&
                            backref->found_dir_index) {
                                if (!backref->errors &&
                                    backref->found_inode_ref) {
                                        list_del(&backref->list);
                                        free(backref);
+                                       continue;
                                }
                        }
                }