Message ID | 159950114896.567790.10646736292763230158.stgit@magnolia (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | xfs_repair: more fuzzer fixes | expand |
On Mon, Sep 07, 2020 at 10:52:28AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > 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> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/repair/dinode.c b/repair/dinode.c index 1fe68bd41117..7577b50ffb2b 100644 --- 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