Re: [PATCH] xfs: fix incorrect root dquot corruption error when switching group/project quota types

[Chandan Babu R <chandanrlinux@gmail.com>]

On 03 Feb 2021 at 01:09, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> While writing up a regression test for broken behavior when a chprojid
> request fails, I noticed that we were logging corruption notices about
> the root dquot of the group/project quota file at mount time when
> testing V4 filesystems.
>
> In commit afeda6000b0c, I was trying to improve ondisk dquot validation
> by making sure that when we load an ondisk dquot into memory on behalf
> of an incore dquot, the dquot id and type matches.  Unfortunately, I
> forgot that V4 filesystems only have two quota files, and can switch
> that file between group and project quota types at mount time.  When we
> perform that switch, we'll try to load the default quota limits from the
> root dquot prior to running quotacheck and log a corruption error when
> the types don't match.
>
> This is inconsequential because quotacheck will reset the second quota
> file as part of doing the switch, but we shouldn't leave scary messages
> in the kernel log.
>

Looks good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> Fixes: afeda6000b0c ("xfs: validate ondisk/incore dquot flags")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/xfs_dquot.c |   39 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 175f544f7c45..bd8379b98374 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -506,6 +506,42 @@ xfs_dquot_alloc(
>  	return dqp;
>  }
>  
> +/* Check the ondisk dquot's id and type match what the incore dquot expects. */
> +static bool
> +xfs_dquot_check_type(
> +	struct xfs_dquot	*dqp,
> +	struct xfs_disk_dquot	*ddqp)
> +{
> +	uint8_t			ddqp_type;
> +	uint8_t			dqp_type;
> +
> +	ddqp_type = ddqp->d_type & XFS_DQTYPE_REC_MASK;
> +	dqp_type = xfs_dquot_type(dqp);
> +
> +	if (be32_to_cpu(ddqp->d_id) != dqp->q_id)
> +		return false;
> +
> +	/*
> +	 * V5 filesystems always expect an exact type match.  V4 filesystems
> +	 * expect an exact match for user dquots and for non-root group and
> +	 * project dquots.
> +	 */
> +	if (xfs_sb_version_hascrc(&dqp->q_mount->m_sb) ||
> +	    dqp_type == XFS_DQTYPE_USER || dqp->q_id != 0)
> +		return ddqp_type == dqp_type;
> +
> +	/*
> +	 * V4 filesystems support either group or project quotas, but not both
> +	 * at the same time.  The non-user quota file can be switched between
> +	 * group and project quota uses depending on the mount options, which
> +	 * means that we can encounter the other type when we try to load quota
> +	 * defaults.  Quotacheck will soon reset the the entire quota file
> +	 * (including the root dquot) anyway, but don't log scary corruption
> +	 * reports to dmesg.
> +	 */
> +	return ddqp_type == XFS_DQTYPE_GROUP || ddqp_type == XFS_DQTYPE_PROJ;
> +}
> +
>  /* Copy the in-core quota fields in from the on-disk buffer. */
>  STATIC int
>  xfs_dquot_from_disk(
> @@ -518,8 +554,7 @@ xfs_dquot_from_disk(
>  	 * Ensure that we got the type and ID we were looking for.
>  	 * Everything else was checked by the dquot buffer verifier.
>  	 */
> -	if ((ddqp->d_type & XFS_DQTYPE_REC_MASK) != xfs_dquot_type(dqp) ||
> -	    be32_to_cpu(ddqp->d_id) != dqp->q_id) {
> +	if (!xfs_dquot_check_type(dqp, ddqp)) {
>  		xfs_alert_tag(bp->b_mount, XFS_PTAG_VERIFIER_ERROR,
>  			  "Metadata corruption detected at %pS, quota %u",
>  			  __this_address, dqp->q_id);


-- 
chandan