diff mbox

btrfs-progs: check: Fix heap use after free.

Message ID 20170503015014.2221-1-suy.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Su Yue May 3, 2017, 1:50 a.m. UTC
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 judgments 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>
---
 cmds-check.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

David Sterba May 3, 2017, 2:51 p.m. UTC | #1
On Wed, May 03, 2017 at 09:50:14AM +0800, Su Yue wrote:
> 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 judgments 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>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/cmds-check.c b/cmds-check.c
index 897b1587..e52fbb84 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -2666,6 +2666,7 @@  static int repair_inode_backrefs(struct btrfs_root *root,
 			repaired++;
 			list_del(&backref->list);
 			free(backref);
+			continue;
 		}
 
 		if (!delete && !backref->found_dir_index &&
@@ -2676,12 +2677,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;
 				}
 			}
 		}