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

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

[Darrick J. Wong]

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.

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);

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

[Brian Foster]

On Tue, Feb 02, 2021 at 11:39:45AM -0800, 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.
> 
> Fixes: afeda6000b0c ("xfs: validate ondisk/incore dquot flags")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  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);
> 

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

[Chandan Babu R]

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