All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: djwong@kernel.org
Cc: linux-xfs@vger.kernel.org
Subject: [PATCH 8/9] xfs: skip the rmapbt search on an empty attr fork unless we know it was zapped
Date: Wed, 06 Dec 2023 18:43:47 -0800	[thread overview]
Message-ID: <170191666238.1182270.18118442139749127193.stgit@frogsfrogsfrogs> (raw)
In-Reply-To: <170191666087.1182270.4104947285831369542.stgit@frogsfrogsfrogs>

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

The attribute fork scrubber can optionally scan the reverse mapping
records of the filesystem to determine if the fork is missing mappings
that it should have.  However, this is a very expensive operation, so we
only want to do this if we suspect that the fork is missing records.
For attribute forks the criteria for suspicion is that the attr fork is
in EXTENTS format and has zero extents.

However, there are several ways that a file can end up in this state
through regular filesystem usage.  For example, an LSM can set a
s_security hook but then decide not to set an ACL; or an attr set can
create the attr fork but then the actual set operation fails with
ENOSPC; or we can delete all the attrs on a file whose data fork is in
btree format, in which case we do not delete the attr fork.  We don't
want to run the expensive check for any case that can be arrived at
through regular operations.

However.

When online inode repair decides to zap an attribute fork, it cannot
determine if it is zapping ACL information.  As a precaution it removes
all the discretionary access control permissions and sets the user and
group ids to zero.  Check these three additional conditions to decide if
we want to scan the rmap records.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/scrub/bmap.c |   44 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 7 deletions(-)


diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index 0ff1f631a9594..a632885825b27 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -664,16 +664,46 @@ xchk_bmap_want_check_rmaps(
 	 * The inode repair code zaps broken inode forks by resetting them back
 	 * to EXTENTS format and zero extent records.  If we encounter a fork
 	 * in this state along with evidence that the fork isn't supposed to be
-	 * empty, we need to scan the reverse mappings to decide if we're going
-	 * to rebuild the fork.  Data forks with nonzero file size are scanned.
-	 * xattr forks are never empty of content, so they are always scanned.
+	 * empty, we might want scan the reverse mappings to decide if we're
+	 * going to rebuild the fork.
 	 */
 	ifp = xfs_ifork_ptr(sc->ip, info->whichfork);
 	if (ifp->if_format == XFS_DINODE_FMT_EXTENTS && ifp->if_nextents == 0) {
-		if (info->whichfork == XFS_DATA_FORK &&
-		    i_size_read(VFS_I(sc->ip)) == 0)
-			return false;
-
+		switch (info->whichfork) {
+		case XFS_DATA_FORK:
+			/*
+			 * Data forks with zero file size are presumed not to
+			 * have any written data blocks.  Skip the scan.
+			 */
+			if (i_size_read(VFS_I(sc->ip)) == 0)
+				return false;
+			break;
+		case XFS_ATTR_FORK:
+			/*
+			 * Files can have an attr fork in EXTENTS format with
+			 * zero records for several reasons:
+			 *
+			 * a) an attr set created a fork but ran out of space
+			 * b) attr replace deleted an old attr but failed
+			 *    during the set step
+			 * c) the data fork was in btree format when all attrs
+			 *    were deleted, so the fork was left in place
+			 * d) the inode repair code zapped the fork
+			 *
+			 * Only in case (d) do we want to scan the rmapbt to
+			 * see if we need to rebuild the attr fork.  The fork
+			 * zap code clears all DAC permission bits and zeroes
+			 * the uid and gid, so avoid the scan if any of those
+			 * three conditions are not met.
+			 */
+			if ((VFS_I(sc->ip)->i_mode & 0777) != 0)
+				return false;
+			if (!uid_eq(VFS_I(sc->ip)->i_uid, GLOBAL_ROOT_UID))
+				return false;
+			if (!gid_eq(VFS_I(sc->ip)->i_gid, GLOBAL_ROOT_GID))
+				return false;
+			break;
+		}
 		return true;
 	}
 


  parent reply	other threads:[~2023-12-07  2:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-07  2:38 [PATCHSET v28.1 0/9] xfs: online repair of inodes and forks Darrick J. Wong
2023-12-07  2:41 ` [PATCH 1/9] xfs: disable online repair quota helpers when quota not enabled Darrick J. Wong
2023-12-07  2:42 ` [PATCH 2/9] xfs: try to attach dquots to files before repairing them Darrick J. Wong
2023-12-07  2:42 ` [PATCH 3/9] xfs: add missing nrext64 inode flag check to scrub Darrick J. Wong
2023-12-07  5:31   ` Christoph Hellwig
2023-12-07  2:42 ` [PATCH 4/9] xfs: repair inode records Darrick J. Wong
2023-12-07  5:41   ` Christoph Hellwig
2023-12-11 20:04     ` Darrick J. Wong
2023-12-12  5:36       ` Christoph Hellwig
2023-12-13  1:36         ` Darrick J. Wong
2023-12-07  2:43 ` [PATCH 5/9] xfs: zap broken inode forks Darrick J. Wong
2023-12-07  6:00   ` Christoph Hellwig
2023-12-07  6:01     ` Christoph Hellwig
2023-12-07  2:43 ` [PATCH 6/9] xfs: set inode sick state flags when we zap either ondisk fork Darrick J. Wong
2023-12-07  5:58   ` Christoph Hellwig
2023-12-11 22:48     ` Darrick J. Wong
2023-12-07  2:43 ` [PATCH 7/9] xfs: abort directory parent scrub scans if we encounter a zapped directory Darrick J. Wong
2023-12-07  6:03   ` Christoph Hellwig
2023-12-11 19:19     ` Darrick J. Wong
2023-12-07  2:43 ` Darrick J. Wong [this message]
2023-12-07  6:07   ` [PATCH 8/9] xfs: skip the rmapbt search on an empty attr fork unless we know it was zapped Christoph Hellwig
2023-12-11 22:50     ` Darrick J. Wong
2023-12-07  2:44 ` [PATCH 9/9] xfs: repair obviously broken inode modes Darrick J. Wong
2023-12-07  6:10   ` Christoph Hellwig
2023-12-11 22:19     ` Darrick J. Wong
2023-12-12  5:35       ` Christoph Hellwig
2023-12-13  1:04         ` Darrick J. Wong

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=170191666238.1182270.18118442139749127193.stgit@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=linux-xfs@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.