xfs_repair: fix handling of data blocks colliding with existing metadata

Prior to commit a406779bc8d8, any blocks in a data fork extent that
collided with existing blocks would cause the entire data fork extent to
be rejected.  Unfortunately, the patch to add data block sharing support
suppressed checking for any collision, including metadata.  What we
really wanted to do here during a check_dups==1 scan is to is check for
specific collisions and without updating the block mapping data.

So, move the check_dups test after the for-switch construction.  This
re-enables detecting collisions between data fork blocks and a
previously scanned chunk of metadata, and improves the specificity of
the error message that results.

This was found by fuzzing recs[2].free=zeroes in xfs/364, though this
patch alone does not solve all the problems that scenario presents.

Fixes: a406779bc8d8 ("xfs_repair: handle multiple owners of data blocks")
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

diff --git a/repair/dinode.c b/repair/dinode.c
index f25ff18596cd35e74775feb220775f96767c8852..fd9b95f93048b502d0b4b9ba80f051ca8047ba09 100644 (file)
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -476,28 +476,6 @@ _("Fatal error: inode %" PRIu64 " - blkmap_set_ext(): %s\n"
                        locked_agno = agno;
                }
 
-               if (check_dups) {
-                       /*
-                        * if we're just checking the bmap for dups,
-                        * return if we find one, otherwise, continue
-                        * checking each entry without setting the
-                        * block bitmap
-                        */
-                       if (!(type == XR_INO_DATA &&
-                           xfs_sb_version_hasreflink(&mp->m_sb)) &&
-                           search_dup_extent(agno, agbno, ebno)) {
-                               do_warn(
-_("%s fork in ino %" PRIu64 " claims dup extent, "
-  "off - %" PRIu64 ", start - %" PRIu64 ", cnt %" PRIu64 "\n"),
-                                       forkname, ino, irec.br_startoff,
-                                       irec.br_startblock,
-                                       irec.br_blockcount);
-                               goto done;
-                       }
-                       *tot += irec.br_blockcount;
-                       continue;
-               }
-
                for (b = irec.br_startblock;
                     agbno < ebno;
                     b += blen, agbno += blen) {
@@ -554,6 +532,17 @@ _("illegal state %d in block map %" PRIu64 "\n"),
                        }
                }
 
+               if (check_dups) {
+                       /*
+                        * If we're just checking the bmap for dups and we
+                        * didn't find any non-reflink collisions, update our
+                        * inode's block count and move on to the next extent.
+                        * We're not yet updating the block usage information.
+                        */
+                       *tot += irec.br_blockcount;
+                       continue;
+               }
+
                /*
                 * Update the internal extent map only after we've checked
                 * every block in this extent.  The first time we reject this