xfs: AGI length should be bounds checked

Similar to the recent patch strengthening the AGF agf_length
verification, the AGI verifier does not check that the AGI length field
is within known good bounds.  This isn't currently checked by runtime
kernel code, yet we assume in many places that it is correct and verify
other metadata against it.

Add length verification to the AGI verifier.  Just like the AGF length
checking, the length of the AGI must be equal to the size of the AG
specified in the superblock, unless it is the last AG in the filesystem.
In that case, it must be less than or equal to sb->sb_agblocks and
greater than XFS_MIN_AG_BLOCKS, which is the smallest AG a growfs
operation will allow to exist.

There's only one place in the filesystem that actually uses agi_length,
but let's not leave it vulnerable to the same weird nonsense that
generates syzbot bugs, eh?

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
This commit is contained in:
Darrick J. Wong
2023-06-29 10:15:45 -07:00
parent 5cf32f63b0
commit 2d7d1e7ea3
3 changed files with 60 additions and 39 deletions

View File

@@ -2486,11 +2486,14 @@ xfs_ialloc_log_agi(
static xfs_failaddr_t
xfs_agi_verify(
struct xfs_buf *bp)
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_mount;
struct xfs_agi *agi = bp->b_addr;
int i;
struct xfs_mount *mp = bp->b_mount;
struct xfs_agi *agi = bp->b_addr;
xfs_failaddr_t fa;
uint32_t agi_seqno = be32_to_cpu(agi->agi_seqno);
uint32_t agi_length = be32_to_cpu(agi->agi_length);
int i;
if (xfs_has_crc(mp)) {
if (!uuid_equal(&agi->agi_uuid, &mp->m_sb.sb_meta_uuid))
@@ -2507,6 +2510,10 @@ xfs_agi_verify(
if (!XFS_AGI_GOOD_VERSION(be32_to_cpu(agi->agi_versionnum)))
return __this_address;
fa = xfs_validate_ag_length(bp, agi_seqno, agi_length);
if (fa)
return fa;
if (be32_to_cpu(agi->agi_level) < 1 ||
be32_to_cpu(agi->agi_level) > M_IGEO(mp)->inobt_maxlevels)
return __this_address;
@@ -2516,15 +2523,6 @@ xfs_agi_verify(
be32_to_cpu(agi->agi_free_level) > M_IGEO(mp)->inobt_maxlevels))
return __this_address;
/*
* during growfs operations, the perag is not fully initialised,
* so we can't use it for any useful checking. growfs ensures we can't
* use it by using uncached buffers that don't have the perag attached
* so we can detect and avoid this problem.
*/
if (bp->b_pag && be32_to_cpu(agi->agi_seqno) != bp->b_pag->pag_agno)
return __this_address;
for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++) {
if (agi->agi_unlinked[i] == cpu_to_be32(NULLAGINO))
continue;