Skip to contentSkip to conversation

xfs_repair: catch bad depth in traverse_int_dir2block

PATCH In review Eric Sandeen 3 participants 6 messages Jun 8, 2009 – Jun 11, 2009

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfs_repair: catch bad depth in traverse_int_dir2block

A bad on-disk tree depth in traverse_int_dir2block() can later cause a segfault when it's used as an array index in this function; if we get something beyond the max depth, just error out and the dir will get rebuilt.

---

repair/dir2.c +10 -2
index 9575fb1..2723e3b 100644
@@ -339,9 +339,17 @@ traverse_int_dir2block(xfs_mount_t	*mp,
 		/*
 		 * maintain level counter
 		 */
-		if (i == -1)
+		if (i == -1) {
 			i = da_cursor->active = be16_to_cpu(node->hdr.level);
-		else  {
+			if (i >= XFS_DA_NODE_MAXDEPTH) {
+				do_warn(_("bad header depth for directory "
+					  "inode %llu\n"),
+					da_cursor->ino);
+				da_brelse(bp);
+				i = -1;
+				goto error_out;
+			}
+		} else {
 			if (be16_to_cpu(node->hdr.level) == i - 1)  {
 				i--;
 			} else  {

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v1 1/1
#1 lore raw
Quoted 31 lines · Eric Sandeen

A bad on-disk tree depth in traverse_int_dir2block() can later cause a segfault when it's used as an array index in this function; if we get something beyond the max depth, just error out and the dir will get rebuilt.

---

repair/dir2.c +10 -2
index 9575fb1..2723e3b 100644
@@ -339,9 +339,17 @@ traverse_int_dir2block(xfs_mount_t	*mp,
 		/*
 		 * maintain level counter
 		 */
-		if (i == -1)
+		if (i == -1) {
 			i = da_cursor->active = be16_to_cpu(node->hdr.level);
-		else  {
+			if (i >= XFS_DA_NODE_MAXDEPTH) {
+				do_warn(_("bad header depth for directory "
+					  "inode %llu\n"),
+					da_cursor->ino);
+				da_brelse(bp);
+				i = -1;
+				goto error_out;
+			}
+		} else {
 
Quoted 1 line

From reading the surrounding code this means the dir is now flagged

as corrupted and we attemp to rebuild it. Which is the only thing we can do here for now.

It would be very good to have an xfsqa testcase with a forcibly corrupted directoty (checked in image or using xfs_db) to verify this behaviour.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#2 lore raw

Guys,

After applying the patch, we get much further. xfs_repair then fails in phase 7:

xfs_repair: phase7.c:47: set_nlinks: Assertion `fs_inode_nlink' failed.

Aborted

Any ideas on that one? Thanks!

Signature · 8 lines
Richard Kolkovich
IntraMeta Corporation
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#3 lore raw
Quoted 11 lines · Richard Kolkovich

Guys,

After applying the patch, we get much further. xfs_repair then fails in phase 7:

xfs_repair: phase7.c:47: set_nlinks: Assertion `fs_inode_nlink' failed.

Aborted

Any ideas on that one? Thanks!

Yep, I meant to reply, I hit that one too. Haven't yet looked into that one ....

-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#4 lore raw
Quoted 11 lines · Eric Sandeen
Quoted 10 lines · Richard Kolkovich

Guys,

After applying the patch, we get much further. xfs_repair then fails in phase 7:

xfs_repair: phase7.c:47: set_nlinks: Assertion `fs_inode_nlink' failed.

Aborted

Quoted 6 lines
Quoted 1 line

Any ideas on that one? Thanks!

Yep, I meant to reply, I hit that one too. Haven't yet looked into that one ....

-Eric

Any ideas on a workaround if not a fix? We really just want to get through the repair and see what (if anything) is still usable...

Thanks!

Signature · 8 lines
Richard Kolkovich
IntraMeta Corporation
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#5 lore raw
Quoted 12 lines · Richard Kolkovich
Quoted 11 lines · Eric Sandeen
Quoted 10 lines · Richard Kolkovich

Guys,

After applying the patch, we get much further. xfs_repair then fails in phase 7:

xfs_repair: phase7.c:47: set_nlinks: Assertion `fs_inode_nlink' failed.

Aborted

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

Any ideas on that one? Thanks!

Yep, I meant to reply, I hit that one too. Haven't yet looked into that one ....

-Eric

Any ideas on a workaround if not a fix? We really just want to get through the repair and see what (if anything) is still usable...

Try setting the nlink feature on the superblock, which apparently got lost... i'm not sure why repair doesn't cope but you can fix that:

xfs_db -x /device

xfs_db> sb 0
xfs_db> write versionnum 0x30A4

but then you'll hit something else I'm looking at, an inode moved to lost+found which is still not referenced, grr.

-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs