On Fri, Nov 10, 2023 at 03:33:13PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Discovered when trying to track down a weird recovery corruption
> issue that wasn't detected at recovery time.
>
> The specific corruption was a zero extent count field when big
> extent counts are in use, and it turns out the dinode verifier
> doesn't detect that specific corruption case, either. So fix it too.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_inode_buf.c | 3 +++
> fs/xfs/xfs_inode_item_recover.c | 14 +++++++++++++-
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index a35781577cad..0f970a0b3382 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -508,6 +508,9 @@ xfs_dinode_verify(
> if (mode && nextents + naextents > nblocks)
> return __this_address;
>
> + if (nextents + naextents == 0 && nblocks != 0)
> + return __this_address;
> +
> if (S_ISDIR(mode) && nextents > mp->m_dir_geo->max_extents)
> return __this_address;
>
> diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
> index 6b09e2bf2d74..f4c31c2b60d5 100644
> --- a/fs/xfs/xfs_inode_item_recover.c
> +++ b/fs/xfs/xfs_inode_item_recover.c
> @@ -286,6 +286,7 @@ xlog_recover_inode_commit_pass2(
> struct xfs_log_dinode *ldip;
> uint isize;
> int need_free = 0;
> + xfs_failaddr_t fa;
>
> if (item->ri_buf[0].i_len == sizeof(struct xfs_inode_log_format)) {
> in_f = item->ri_buf[0].i_addr;
> @@ -529,8 +530,19 @@ xlog_recover_inode_commit_pass2(
> (dip->di_mode != 0))
> error = xfs_recover_inode_owner_change(mp, dip, in_f,
> buffer_list);
> - /* re-generate the checksum. */
> + /* re-generate the checksum and validate the recovered inode. */
> xfs_dinode_calc_crc(log->l_mp, dip);
> + fa = xfs_dinode_verify(log->l_mp, in_f->ilf_ino, dip);
> + if (fa) {
Does xlog_recover_dquot_commit_pass2 need to call xfs_dquot_verify as
well?
This patch looks good though,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> + XFS_CORRUPTION_ERROR(
> + "Bad dinode after recovery",
> + XFS_ERRLEVEL_LOW, mp, dip, sizeof(*dip));
> + xfs_alert(mp,
> + "Metadata corruption detected at %pS, inode 0x%llx",
> + fa, in_f->ilf_ino);
> + error = -EFSCORRUPTED;
> + goto out_release;
> + }
>
> ASSERT(bp->b_mount == mp);
> bp->b_flags |= _XBF_LOGRECOVERY;
> --
> 2.42.0
>
