All of lore.kernel.org
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sasha Levin <sashal@kernel.org>,
	Dave Chinner <david@fromorbit.com>,
	"Darrick J . Wong" <djwong@kernel.org>,
	Christoph Hellwig <hch@lst.de>, Brian Foster <bfoster@redhat.com>,
	Christian Brauner <brauner@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Leah Rumancik <leah.rumancik@gmail.com>,
	Adam Manzanares <a.manzanares@samsung.com>,
	linux-xfs@vger.kernel.org, stable@vger.kernel.org,
	Chandan Babu R <chandanrlinux@gmail.com>
Subject: [PATCH 5.10 v2 4/8] xfs: fix incorrect root dquot corruption error when switching group/project quota types
Date: Mon,  6 Jun 2022 17:32:51 +0300	[thread overview]
Message-ID: <20220606143255.685988-5-amir73il@gmail.com> (raw)
In-Reply-To: <20220606143255.685988-1-amir73il@gmail.com>

From: "Darrick J. Wong" <djwong@kernel.org>

commit 45068063efb7dd0a8d115c106aa05d9ab0946257 upstream.

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>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.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 1d95ed387d66..80c4579d6835 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -500,6 +500,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(
@@ -512,8 +548,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);
-- 
2.25.1


  parent reply	other threads:[~2022-06-06 14:33 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-06 14:32 [PATCH 5.10 v2 0/8] xfs fixes for 5.10.y (part 2) Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 1/8] xfs: set inode size after creating symlink Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 2/8] xfs: sync lazy sb accounting on quiesce of read-only mounts Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 3/8] xfs: fix chown leaking delalloc quota blocks when fssetxattr fails Amir Goldstein
2022-06-06 14:32 ` Amir Goldstein [this message]
2022-06-06 14:32 ` [PATCH 5.10 v2 5/8] xfs: restore shutdown check in mapped write fault path Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 6/8] xfs: force log and push AIL to clear pinned inodes when aborting mount Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 7/8] xfs: consider shutdown in bmapbt cursor delete assert Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 8/8] xfs: assert in xfs_btree_del_cursor should take into account error Amir Goldstein
2022-06-06 21:30   ` Dave Chinner
2022-06-06 22:33     ` Amir Goldstein
2022-06-07  3:01       ` Dave Chinner
2022-06-07  4:47         ` Greg Kroah-Hartman
2022-06-07  4:56           ` Amir Goldstein
2022-06-07  6:09         ` Amir Goldstein
2022-06-07 18:35           ` Darrick J. Wong
2022-06-07 19:12             ` Luis Chamberlain
2022-06-08  4:37             ` XFS stable maintenance (Was: Re: [PATCH 5.10 v2 8/8] xfs: assert in xfs_btree_del_cursor should take into account error) Amir Goldstein
2022-06-06 17:01 ` [PATCH 5.10 v2 0/8] xfs fixes for 5.10.y (part 2) Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220606143255.685988-5-amir73il@gmail.com \
    --to=amir73il@gmail.com \
    --cc=a.manzanares@samsung.com \
    --cc=bfoster@redhat.com \
    --cc=brauner@kernel.org \
    --cc=chandanrlinux@gmail.com \
    --cc=david@fromorbit.com \
    --cc=djwong@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=leah.rumancik@gmail.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.