On 03 Feb 2021 at 01:09, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Fixes: afeda6000b0c ("xfs: validate ondisk/incore dquot flags")
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> 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
