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@xxxxxx>
