[PATCH v2] xfs_repair: fix process_bmbt_reclist_int

[PATCH v2] xfs_repair: fix process_bmbt_reclist_int

[Ben Myers]

There is a set checks for corruption in block map btrees in
process_bmbt_reclist_int that we identify but currently do not fix.  It
appears that the author's intent in this function was to set error = 1,
and then only clear it when all of the checks were completed
successfully.  Unfortunately error can be cleared when it is used for
the return value of blkmap_set_ext.  Some kinds of corruption are not
being fixed, including duplicate extents, claiming free blocks, claiming
metadata blocks, and multiply used blocks.

Fix this by using error2 for the return code from blkmap_set_ext.

Signed-off-by: Ben Myers <bpm@sgi.com>

---

v2: Use error2 instead of shadowing error, as per Dave's suggestion.

 repair/dinode.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Index: b/repair/dinode.c
===================================================================
--- a/repair/dinode.c	2013-12-10 14:42:57.905267321 -0600
+++ b/repair/dinode.c	2013-12-10 14:45:43.435246026 -0600
@@ -663,9 +663,10 @@ _("inode %" PRIu64 " - extent offset too
 		}
 
 		if (blkmapp && *blkmapp) {
-			error = blkmap_set_ext(blkmapp, irec.br_startoff,
+			int	error2;
+			error2 = blkmap_set_ext(blkmapp, irec.br_startoff,
 					irec.br_startblock, irec.br_blockcount);
-			if (error) {
+			if (error2) {
 				/*
 				 * we don't want to clear the inode due to an
 				 * internal bmap tracking error, but if we've
@@ -677,7 +678,7 @@ _("inode %" PRIu64 " - extent offset too
 				do_abort(
 _("Fatal error: inode %" PRIu64 " - blkmap_set_ext(): %s\n"
   "\t%s fork, off - %" PRIu64 ", start - %" PRIu64 ", cnt %" PRIu64 "\n"),
-					ino, strerror(error), forkname,
+					ino, strerror(error2), forkname,
 					irec.br_startoff, irec.br_startblock,
 					irec.br_blockcount);
 			}

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

Re: [PATCH v2] xfs_repair: fix process_bmbt_reclist_int

[Christoph Hellwig]

On Tue, Dec 10, 2013 at 02:53:52PM -0600, Ben Myers wrote:
> There is a set checks for corruption in block map btrees in
> process_bmbt_reclist_int that we identify but currently do not fix.  It
> appears that the author's intent in this function was to set error = 1,
> and then only clear it when all of the checks were completed
> successfully.  Unfortunately error can be cleared when it is used for
> the return value of blkmap_set_ext.  Some kinds of corruption are not
> being fixed, including duplicate extents, claiming free blocks, claiming
> metadata blocks, and multiply used blocks.
> 
> Fix this by using error2 for the return code from blkmap_set_ext.
> 
> Signed-off-by: Ben Myers <bpm@sgi.com>

Looks good,

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

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

Re: [PATCH v2] xfs_repair: fix process_bmbt_reclist_int

[Rich Johnston]

This has been committed.

Thanks
--Rich

commit ea4a8de1e135905e240a54f192daf23275a7e8a8
Author: Ben Myers <bpm@sgi.com>
Date:   Tue Dec 10 20:53:52 2013 +0000

     xfs_repair: fix process_bmbt_reclist_int

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