Re: [PATCH 09/16] xfs_repair: convert to libxfs_verify_agbno

[Christoph Hellwig <hch@infradead.org>]

On Mon, May 11, 2020 at 09:22:03AM -0700, Darrick J. Wong wrote:
> Yes.  Each of those "bno != 0" checks occurs in the context of checking
> an AG header's pointer to a btree root.  The roots should never be zero
> if the corresponding feature is enabled, and we're careful to check the
> feature bits first.
> 
> AFAICT that bno != 0 check is actually there to cover a deficiency in
> the verify_agbno function, which is that it only checked that the
> supplied argument didn't go past the end of the AG and did not check
> that the pointer didn't point into the AG header block(s).
> 
> Checking for a nonzero value is also insufficient, since on a
> blocksize < sectorsize * 4 filesystem, the AGFL can end up in a nonzero
> agbno.  libxfs_verify_agbno covers all of these cases.
> 
> > Either way this should probably be documented in the changelog.
> 
> Ok, how about this for a commit message:
> 
> "Convert the homegrown verify_agbno callers to use the libxfs function,
> as needed.  In some places we drop the "bno != 0" checks because those
> conditionals are checking btree roots; btree roots should never be
> zero if the corresponding feature bit is set; and repair skips the if
> clause entirely if the feature bit is disabled.
> 
> "In effect, this strengthens repair to validate that AG btree pointers
> neither point to the AG headers nor point past the end of the AG."

With the additional explanation in the commit log:

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