[PATCH] xfs_repair: catch bad depth in traverse_int_dir2block

[PATCH] xfs_repair: catch bad depth in traverse_int_dir2block

[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.

Reported-by: Richard Kolkovich <richard@intrameta.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---

diff --git a/repair/dir2.c b/repair/dir2.c
index 9575fb1..2723e3b 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_repair: catch bad depth in traverse_int_dir2block

[Christoph Hellwig]

On Sun, Jun 07, 2009 at 10:51:52PM -0500, Eric Sandeen wrote:
> 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.
> 
> Reported-by: Richard Kolkovich <richard@intrameta.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
> 
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 9575fb1..2723e3b 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -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 {

>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.


Reviewed-by: Christoph Hellwig <hch@lst.de>

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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_repair: catch bad depth in traverse_int_dir2block

[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!


-- 

Richard Kolkovich
IntraMeta Corporation
richard@intrameta.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_repair: catch bad depth in traverse_int_dir2block

[Eric Sandeen]

Richard Kolkovich wrote:
> 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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_repair: catch bad depth in traverse_int_dir2block

[Richard Kolkovich]

On Tue, Jun 09, 2009 at 12:38:35PM -0400, Eric Sandeen wrote:
> Richard Kolkovich wrote:
> > 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

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!

-- 

Richard Kolkovich
IntraMeta Corporation
richard@intrameta.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_repair: catch bad depth in traverse_int_dir2block

[Eric Sandeen]

Richard Kolkovich wrote:
> On Tue, Jun 09, 2009 at 12:38:35PM -0400, Eric Sandeen wrote:
>> Richard Kolkovich wrote:
>>> 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
> 
> 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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs