From 2e9e6481e2a78de3a85083beccfbf6eda2689922 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Thu, 19 Jul 2018 12:29:13 -0700 Subject: xfs: detect and fix bad summary counts at mount Filippo Giunchedi complained that xfs doesn't even perform basic sanity checks of the fs summary counters at mount time. Therefore, recalculate the summary counters from the AGFs after log recovery if the counts were bad (or we had to recover the fs). Enhance the recalculation routine to fail the mount entirely if the new values are also obviously incorrect. We use a mount state flag to record the "bad summary count" state so that the (subsequent) online fsck patches can detect subtlely incorrect counts and set the flag; clear it userspace asks for a repair; or force a recalculation at the next mount if nobody fixes it by unmount time. Reported-by: Filippo Giunchedi Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- fs/xfs/libxfs/xfs_sb.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) (limited to 'fs/xfs/libxfs/xfs_sb.c') diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c index 350119eeaecb..b3ad15956366 100644 --- a/fs/xfs/libxfs/xfs_sb.c +++ b/fs/xfs/libxfs/xfs_sb.c @@ -804,6 +804,7 @@ xfs_initialize_perag_data( uint64_t bfree = 0; uint64_t bfreelst = 0; uint64_t btree = 0; + uint64_t fdblocks; int error; for (index = 0; index < agcount; index++) { @@ -827,17 +828,31 @@ xfs_initialize_perag_data( btree += pag->pagf_btreeblks; xfs_perag_put(pag); } + fdblocks = bfree + bfreelst + btree; + + /* + * If the new summary counts are obviously incorrect, fail the + * mount operation because that implies the AGFs are also corrupt. + * Clear BAD_SUMMARY so that we don't unmount with a dirty log, which + * will prevent xfs_repair from fixing anything. + */ + if (fdblocks > sbp->sb_dblocks || ifree > ialloc) { + xfs_alert(mp, "AGF corruption. Please run xfs_repair."); + error = -EFSCORRUPTED; + goto out; + } /* Overwrite incore superblock counters with just-read data */ spin_lock(&mp->m_sb_lock); sbp->sb_ifree = ifree; sbp->sb_icount = ialloc; - sbp->sb_fdblocks = bfree + bfreelst + btree; + sbp->sb_fdblocks = fdblocks; spin_unlock(&mp->m_sb_lock); xfs_reinit_percpu_counters(mp); - - return 0; +out: + mp->m_flags &= ~XFS_MOUNT_BAD_SUMMARY; + return error; } /* -- cgit v1.2.3 From eca383fcd63b452cf533505154135da2a1f70227 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Sun, 29 Jul 2018 13:10:44 -0700 Subject: xfs: refactor superblock verifiers Split the superblock verifier into the common checks, the read-time checks, and the write-time check functions. No functional changes, but we're setting up to add more write-only checks. Signed-off-by: Darrick J. Wong Reviewed-by: Eric Sandeen --- fs/xfs/libxfs/xfs_sb.c | 205 ++++++++++++++++++++++++++----------------------- 1 file changed, 111 insertions(+), 94 deletions(-) (limited to 'fs/xfs/libxfs/xfs_sb.c') diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c index b3ad15956366..f3835e923893 100644 --- a/fs/xfs/libxfs/xfs_sb.c +++ b/fs/xfs/libxfs/xfs_sb.c @@ -96,80 +96,94 @@ xfs_perag_put( trace_xfs_perag_put(pag->pag_mount, pag->pag_agno, ref, _RET_IP_); } -/* - * Check the validity of the SB found. - */ +/* Check all the superblock fields we care about when reading one in. */ STATIC int -xfs_mount_validate_sb( - xfs_mount_t *mp, - xfs_sb_t *sbp, - bool check_inprogress, - bool check_version) +xfs_validate_sb_read( + struct xfs_mount *mp, + struct xfs_sb *sbp) { - uint32_t agcount = 0; - uint32_t rem; - - if (sbp->sb_magicnum != XFS_SB_MAGIC) { - xfs_warn(mp, "bad magic number"); - return -EWRONGFS; - } - - - if (!xfs_sb_good_version(sbp)) { - xfs_warn(mp, "bad version"); - return -EWRONGFS; - } + if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_5) + return 0; /* - * Version 5 superblock feature mask validation. Reject combinations the - * kernel cannot support up front before checking anything else. For - * write validation, we don't need to check feature masks. + * Version 5 superblock feature mask validation. Reject combinations + * the kernel cannot support up front before checking anything else. */ - if (check_version && XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) { - if (xfs_sb_has_compat_feature(sbp, - XFS_SB_FEAT_COMPAT_UNKNOWN)) { - xfs_warn(mp, + if (xfs_sb_has_compat_feature(sbp, XFS_SB_FEAT_COMPAT_UNKNOWN)) { + xfs_warn(mp, "Superblock has unknown compatible features (0x%x) enabled.", - (sbp->sb_features_compat & - XFS_SB_FEAT_COMPAT_UNKNOWN)); - xfs_warn(mp, + (sbp->sb_features_compat & XFS_SB_FEAT_COMPAT_UNKNOWN)); + xfs_warn(mp, "Using a more recent kernel is recommended."); - } + } - if (xfs_sb_has_ro_compat_feature(sbp, - XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) { - xfs_alert(mp, + if (xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) { + xfs_alert(mp, "Superblock has unknown read-only compatible features (0x%x) enabled.", - (sbp->sb_features_ro_compat & - XFS_SB_FEAT_RO_COMPAT_UNKNOWN)); - if (!(mp->m_flags & XFS_MOUNT_RDONLY)) { - xfs_warn(mp, + (sbp->sb_features_ro_compat & + XFS_SB_FEAT_RO_COMPAT_UNKNOWN)); + if (!(mp->m_flags & XFS_MOUNT_RDONLY)) { + xfs_warn(mp, "Attempted to mount read-only compatible filesystem read-write."); - xfs_warn(mp, + xfs_warn(mp, "Filesystem can only be safely mounted read only."); - return -EINVAL; - } - } - if (xfs_sb_has_incompat_feature(sbp, - XFS_SB_FEAT_INCOMPAT_UNKNOWN)) { - xfs_warn(mp, -"Superblock has unknown incompatible features (0x%x) enabled.", - (sbp->sb_features_incompat & - XFS_SB_FEAT_INCOMPAT_UNKNOWN)); - xfs_warn(mp, -"Filesystem can not be safely mounted by this kernel."); return -EINVAL; } - } else if (xfs_sb_version_hascrc(sbp)) { - /* - * We can't read verify the sb LSN because the read verifier is - * called before the log is allocated and processed. We know the - * log is set up before write verifier (!check_version) calls, - * so just check it here. - */ - if (!xfs_log_check_lsn(mp, sbp->sb_lsn)) - return -EFSCORRUPTED; + } + if (xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_UNKNOWN)) { + xfs_warn(mp, +"Superblock has unknown incompatible features (0x%x) enabled.", + (sbp->sb_features_incompat & + XFS_SB_FEAT_INCOMPAT_UNKNOWN)); + xfs_warn(mp, +"Filesystem cannot be safely mounted by this kernel."); + return -EINVAL; + } + + return 0; +} + +/* Check all the superblock fields we care about when writing one out. */ +STATIC int +xfs_validate_sb_write( + struct xfs_mount *mp, + struct xfs_sb *sbp) +{ + if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_5) + return 0; + + /* XXX: For write validation, we don't need to check feature masks?? */ + + /* + * We can't read verify the sb LSN because the read verifier is called + * before the log is allocated and processed. We know the log is set up + * before write verifier calls, so check it here. + */ + if (!xfs_log_check_lsn(mp, sbp->sb_lsn)) + return -EFSCORRUPTED; + + return 0; +} + +/* Check the validity of the SB. */ +STATIC int +xfs_validate_sb_common( + struct xfs_mount *mp, + struct xfs_buf *bp, + struct xfs_sb *sbp) +{ + uint32_t agcount = 0; + uint32_t rem; + + if (sbp->sb_magicnum != XFS_SB_MAGIC) { + xfs_warn(mp, "bad magic number"); + return -EWRONGFS; + } + + if (!xfs_sb_good_version(sbp)) { + xfs_warn(mp, "bad version"); + return -EWRONGFS; } if (xfs_sb_version_has_pquotino(sbp)) { @@ -321,7 +335,12 @@ xfs_mount_validate_sb( return -EFBIG; } - if (check_inprogress && sbp->sb_inprogress) { + /* + * Don't touch the filesystem if a user tool thinks it owns the primary + * superblock. mkfs doesn't clear the flag from secondary supers, so + * we don't check them at all. + */ + if (XFS_BUF_ADDR(bp) == XFS_SB_DADDR && sbp->sb_inprogress) { xfs_warn(mp, "Offline file system operation in progress!"); return -EFSCORRUPTED; } @@ -596,29 +615,6 @@ xfs_sb_to_disk( } } -static int -xfs_sb_verify( - struct xfs_buf *bp, - bool check_version) -{ - struct xfs_mount *mp = bp->b_target->bt_mount; - struct xfs_sb sb; - - /* - * Use call variant which doesn't convert quota flags from disk - * format, because xfs_mount_validate_sb checks the on-disk flags. - */ - __xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false); - - /* - * Only check the in progress field for the primary superblock as - * mkfs.xfs doesn't clear it from secondary superblocks. - */ - return xfs_mount_validate_sb(mp, &sb, - bp->b_maps[0].bm_bn == XFS_SB_DADDR, - check_version); -} - /* * If the superblock has the CRC feature bit set or the CRC field is non-null, * check that the CRC is valid. We check the CRC field is non-null because a @@ -633,11 +629,12 @@ xfs_sb_verify( */ static void xfs_sb_read_verify( - struct xfs_buf *bp) + struct xfs_buf *bp) { - struct xfs_mount *mp = bp->b_target->bt_mount; - struct xfs_dsb *dsb = XFS_BUF_TO_SBP(bp); - int error; + struct xfs_sb sb; + struct xfs_mount *mp = bp->b_target->bt_mount; + struct xfs_dsb *dsb = XFS_BUF_TO_SBP(bp); + int error; /* * open code the version check to avoid needing to convert the entire @@ -657,7 +654,16 @@ xfs_sb_read_verify( } } } - error = xfs_sb_verify(bp, true); + + /* + * Check all the superblock fields. Don't byteswap the xquota flags + * because _verify_common checks the on-disk values. + */ + __xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false); + error = xfs_validate_sb_common(mp, bp, &sb); + if (error) + goto out_error; + error = xfs_validate_sb_read(mp, &sb); out_error: if (error == -EFSCORRUPTED || error == -EFSBADCRC) @@ -691,15 +697,22 @@ static void xfs_sb_write_verify( struct xfs_buf *bp) { + struct xfs_sb sb; struct xfs_mount *mp = bp->b_target->bt_mount; struct xfs_buf_log_item *bip = bp->b_log_item; int error; - error = xfs_sb_verify(bp, false); - if (error) { - xfs_verifier_error(bp, error, __this_address); - return; - } + /* + * Check all the superblock fields. Don't byteswap the xquota flags + * because _verify_common checks the on-disk values. + */ + __xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false); + error = xfs_validate_sb_common(mp, bp, &sb); + if (error) + goto out_error; + error = xfs_validate_sb_write(mp, &sb); + if (error) + goto out_error; if (!xfs_sb_version_hascrc(&mp->m_sb)) return; @@ -708,6 +721,10 @@ xfs_sb_write_verify( XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn); xfs_buf_update_cksum(bp, XFS_SB_CRC_OFF); + return; + +out_error: + xfs_verifier_error(bp, error, __this_address); } const struct xfs_buf_ops xfs_sb_buf_ops = { -- cgit v1.2.3 From 8756a5af18191a471e670cc577aea60b652fea4c Mon Sep 17 00:00:00 2001 From: Bill O'Donnell Date: Thu, 26 Jul 2018 10:10:34 -0700 Subject: libxfs: add more bounds checking to sb sanity checks Current sb verifier doesn't check bounds on sb_fdblocks and sb_ifree. Add sanity checks for these parameters. Signed-off-by: Bill O'Donnell [darrick: port to refactored sb validation predicates] Signed-off-by: Darrick J. Wong Reviewed-by: Eric Sandeen --- fs/xfs/libxfs/xfs_sb.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'fs/xfs/libxfs/xfs_sb.c') diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c index f3835e923893..3d29f4a5242f 100644 --- a/fs/xfs/libxfs/xfs_sb.c +++ b/fs/xfs/libxfs/xfs_sb.c @@ -150,6 +150,18 @@ xfs_validate_sb_write( struct xfs_mount *mp, struct xfs_sb *sbp) { + /* + * Carry out additional sb summary counter sanity checks when we write + * the superblock. We skip this in the read validator because there + * could be newer superblocks in the log and if the values are garbage + * we'll recalculate them at the end of log mount. + */ + if (sbp->sb_fdblocks > sbp->sb_dblocks || + sbp->sb_ifree > sbp->sb_icount) { + xfs_warn(mp, "SB summary counter sanity check failed"); + return -EFSCORRUPTED; + } + if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_5) return 0; -- cgit v1.2.3 From 69775fd15dc78e0547af45fb3e375d5423cb21b1 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Thu, 26 Jul 2018 10:10:42 -0700 Subject: xfs: verify icount in superblock write Add a helper predicate to check the inode count for sanity, then use it in the superblock write verifier to inspect sb_icount. Signed-off-by: Darrick J. Wong Reviewed-by: Bill O'Donnell Reviewed-by: Eric Sandeen --- fs/xfs/libxfs/xfs_sb.c | 3 ++- fs/xfs/libxfs/xfs_types.c | 34 ++++++++++++++++++++++++++++++++++ fs/xfs/libxfs/xfs_types.h | 1 + 3 files changed, 37 insertions(+), 1 deletion(-) (limited to 'fs/xfs/libxfs/xfs_sb.c') diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c index 3d29f4a5242f..05e7ed1b8022 100644 --- a/fs/xfs/libxfs/xfs_sb.c +++ b/fs/xfs/libxfs/xfs_sb.c @@ -154,9 +154,10 @@ xfs_validate_sb_write( * Carry out additional sb summary counter sanity checks when we write * the superblock. We skip this in the read validator because there * could be newer superblocks in the log and if the values are garbage - * we'll recalculate them at the end of log mount. + * even after replay we'll recalculate them at the end of log mount. */ if (sbp->sb_fdblocks > sbp->sb_dblocks || + !xfs_verify_icount(mp, sbp->sb_icount) || sbp->sb_ifree > sbp->sb_icount) { xfs_warn(mp, "SB summary counter sanity check failed"); return -EFSCORRUPTED; diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c index 2e2a243cef2e..33a5ca346baf 100644 --- a/fs/xfs/libxfs/xfs_types.c +++ b/fs/xfs/libxfs/xfs_types.c @@ -171,3 +171,37 @@ xfs_verify_rtbno( { return rtbno < mp->m_sb.sb_rblocks; } + +/* Calculate the range of valid icount values. */ +static void +xfs_icount_range( + struct xfs_mount *mp, + unsigned long long *min, + unsigned long long *max) +{ + unsigned long long nr_inos = 0; + xfs_agnumber_t agno; + + /* root, rtbitmap, rtsum all live in the first chunk */ + *min = XFS_INODES_PER_CHUNK; + + for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) { + xfs_agino_t first, last; + + xfs_agino_range(mp, agno, &first, &last); + nr_inos += last - first + 1; + } + *max = nr_inos; +} + +/* Sanity-checking of inode counts. */ +bool +xfs_verify_icount( + struct xfs_mount *mp, + unsigned long long icount) +{ + unsigned long long min, max; + + xfs_icount_range(mp, &min, &max); + return icount >= min && icount <= max; +} diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h index 4055d62f690c..b9e6c89284c3 100644 --- a/fs/xfs/libxfs/xfs_types.h +++ b/fs/xfs/libxfs/xfs_types.h @@ -165,5 +165,6 @@ bool xfs_verify_ino(struct xfs_mount *mp, xfs_ino_t ino); bool xfs_internal_inum(struct xfs_mount *mp, xfs_ino_t ino); bool xfs_verify_dir_ino(struct xfs_mount *mp, xfs_ino_t ino); bool xfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno); +bool xfs_verify_icount(struct xfs_mount *mp, unsigned long long icount); #endif /* __XFS_TYPES_H__ */ -- cgit v1.2.3 From 9e037cb7972fab5a9f55bca4ebe6e4dbf7e160bc Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Mon, 30 Jul 2018 16:45:35 -0700 Subject: xfs: check for unknown v5 feature bits in superblock write verifier Make sure we never try to write the superblock with unknown feature bits set. We checked those at mount time, so if they're set now then memory is corrupt. Signed-off-by: Darrick J. Wong Reviewed-by: Eric Sandeen --- fs/xfs/libxfs/xfs_sb.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) (limited to 'fs/xfs/libxfs/xfs_sb.c') diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c index 05e7ed1b8022..ca1b3a7a9171 100644 --- a/fs/xfs/libxfs/xfs_sb.c +++ b/fs/xfs/libxfs/xfs_sb.c @@ -166,7 +166,40 @@ xfs_validate_sb_write( if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_5) return 0; - /* XXX: For write validation, we don't need to check feature masks?? */ + /* + * Version 5 superblock feature mask validation. Reject combinations + * the kernel cannot support since we checked for unsupported bits in + * the read verifier, which means that memory is corrupt. + */ + if (xfs_sb_has_compat_feature(sbp, XFS_SB_FEAT_COMPAT_UNKNOWN)) { + xfs_warn(mp, +"Corruption detected in superblock compatible features (0x%x)!", + (sbp->sb_features_compat & XFS_SB_FEAT_COMPAT_UNKNOWN)); + return -EFSCORRUPTED; + } + + if (xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) { + xfs_alert(mp, +"Corruption detected in superblock read-only compatible features (0x%x)!", + (sbp->sb_features_ro_compat & + XFS_SB_FEAT_RO_COMPAT_UNKNOWN)); + return -EFSCORRUPTED; + } + if (xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_UNKNOWN)) { + xfs_warn(mp, +"Corruption detected in superblock incompatible features (0x%x)!", + (sbp->sb_features_incompat & + XFS_SB_FEAT_INCOMPAT_UNKNOWN)); + return -EFSCORRUPTED; + } + if (xfs_sb_has_incompat_log_feature(sbp, + XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN)) { + xfs_warn(mp, +"Corruption detected in superblock incompatible log features (0x%x)!", + (sbp->sb_features_log_incompat & + XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN)); + return -EFSCORRUPTED; + } /* * We can't read verify the sb LSN because the read verifier is called -- cgit v1.2.3 From 1f31c98d650ca342e2f54cb17c4554ad110c5a11 Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Wed, 1 Aug 2018 15:50:27 -0700 Subject: xfs: only validate summary counts on primary superblock Skip the summary counter checks for secondary superblocks and inprogress primary superblocks because mkfs has always written those out with zeroed summary counters. Signed-off-by: Darrick J. Wong Reviewed-by: Carlos Maiolino --- fs/xfs/libxfs/xfs_sb.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'fs/xfs/libxfs/xfs_sb.c') diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c index ca1b3a7a9171..081f46e30556 100644 --- a/fs/xfs/libxfs/xfs_sb.c +++ b/fs/xfs/libxfs/xfs_sb.c @@ -148,6 +148,7 @@ xfs_validate_sb_read( STATIC int xfs_validate_sb_write( struct xfs_mount *mp, + struct xfs_buf *bp, struct xfs_sb *sbp) { /* @@ -155,10 +156,15 @@ xfs_validate_sb_write( * the superblock. We skip this in the read validator because there * could be newer superblocks in the log and if the values are garbage * even after replay we'll recalculate them at the end of log mount. + * + * mkfs has traditionally written zeroed counters to inprogress and + * secondary superblocks, so allow this usage to continue because + * we never read counters from such superblocks. */ - if (sbp->sb_fdblocks > sbp->sb_dblocks || - !xfs_verify_icount(mp, sbp->sb_icount) || - sbp->sb_ifree > sbp->sb_icount) { + if (XFS_BUF_ADDR(bp) == XFS_SB_DADDR && !sbp->sb_inprogress && + (sbp->sb_fdblocks > sbp->sb_dblocks || + !xfs_verify_icount(mp, sbp->sb_icount) || + sbp->sb_ifree > sbp->sb_icount)) { xfs_warn(mp, "SB summary counter sanity check failed"); return -EFSCORRUPTED; } @@ -756,7 +762,7 @@ xfs_sb_write_verify( error = xfs_validate_sb_common(mp, bp, &sb); if (error) goto out_error; - error = xfs_validate_sb_write(mp, &sb); + error = xfs_validate_sb_write(mp, bp, &sb); if (error) goto out_error; -- cgit v1.2.3