diff mbox series

[3/3] xfs: scrub should mark dir corrupt if entry points to unallocated inode

Message ID 158388770010.939608.11408184090778445318.stgit@magnolia (mailing list archive)
State New, archived
Headers show
Series xfs: fix errors in attr/directory scrubbers | expand

Commit Message

Darrick J. Wong March 11, 2020, 12:48 a.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

In xchk_dir_check_ftype, we should mark the directory corrupt if we try
to _iget a directory entry's inode pointer and the inode btree says the
inode is not allocated.  This involves changing the IGET call to force
the inobt lookup to return EINVAL if the inode isn't allocated; and
rearranging the code so that we always perform the iget.

We can also remove the !hasftype code from the function, because any
DT_ flags we encounter on those filesystems were synthesized in core,
not read in from disk.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/scrub/dir.c |   39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

Comments

Dave Chinner March 11, 2020, 5:55 a.m. UTC | #1 
On Tue, Mar 10, 2020 at 05:48:20PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> In xchk_dir_check_ftype, we should mark the directory corrupt if we try
> to _iget a directory entry's inode pointer and the inode btree says the
> inode is not allocated.  This involves changing the IGET call to force
> the inobt lookup to return EINVAL if the inode isn't allocated; and
> rearranging the code so that we always perform the iget.
> 
> We can also remove the !hasftype code from the function, because any
> DT_ flags we encounter on those filesystems were synthesized in core,
> not read in from disk.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/scrub/dir.c |   39 ++++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 17 deletions(-)

That's much neater.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
diff mbox series

Patch

diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index ef7cc8e101ab..c186c83544ac 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -39,9 +39,12 @@  struct xchk_dir_ctx {
 	struct xfs_scrub	*sc;
 };
 
-/* Check that an inode's mode matches a given DT_ type. */
+/*
+ * Check that a directory entry's inode pointer directs us to an allocated
+ * inode and (if applicable) the inode mode matches the entry's DT_ type.
+ */
 STATIC int
-xchk_dir_check_ftype(
+xchk_dir_check_iptr(
 	struct xchk_dir_ctx	*sdc,
 	xfs_fileoff_t		offset,
 	xfs_ino_t		inum,
@@ -52,13 +55,6 @@  xchk_dir_check_ftype(
 	int			ino_dtype;
 	int			error = 0;
 
-	if (!xfs_sb_version_hasftype(&mp->m_sb)) {
-		if (dtype != DT_UNKNOWN && dtype != DT_DIR)
-			xchk_fblock_set_corrupt(sdc->sc, XFS_DATA_FORK,
-					offset);
-		goto out;
-	}
-
 	/*
 	 * Grab the inode pointed to by the dirent.  We release the
 	 * inode before we cancel the scrub transaction.  Since we're
@@ -66,17 +62,26 @@  xchk_dir_check_ftype(
 	 * eofblocks cleanup (which allocates what would be a nested
 	 * transaction), we can't use DONTCACHE here because DONTCACHE
 	 * inodes can trigger immediate inactive cleanup of the inode.
+	 *
+	 * We use UNTRUSTED here to force validation of the inode number (using
+	 * the inode btree) before we look up the inode record.  If this fails
+	 * validation for any reason, we will receive EINVAL, which indicates a
+	 * corrupt directory entry.
 	 */
-	error = xfs_iget(mp, sdc->sc->tp, inum, 0, 0, &ip);
+	error = xfs_iget(mp, sdc->sc->tp, inum, XFS_IGET_UNTRUSTED, 0, &ip);
+	if (error == -EINVAL)
+		error = -EFSCORRUPTED;
 	if (!xchk_fblock_xref_process_error(sdc->sc, XFS_DATA_FORK, offset,
 			&error))
 		goto out;
 
-	/* Convert mode to the DT_* values that dir_emit uses. */
-	ino_dtype = xfs_dir3_get_dtype(mp,
-			xfs_mode_to_ftype(VFS_I(ip)->i_mode));
-	if (ino_dtype != dtype)
-		xchk_fblock_set_corrupt(sdc->sc, XFS_DATA_FORK, offset);
+	if (xfs_sb_version_hasftype(&mp->m_sb)) {
+		/* Convert mode to the DT_* values that dir_emit uses. */
+		ino_dtype = xfs_dir3_get_dtype(mp,
+				xfs_mode_to_ftype(VFS_I(ip)->i_mode));
+		if (ino_dtype != dtype)
+			xchk_fblock_set_corrupt(sdc->sc, XFS_DATA_FORK, offset);
+	}
 	xfs_irele(ip);
 out:
 	return error;
@@ -166,8 +171,8 @@  xchk_dir_actor(
 		goto out;
 	}
 
-	/* Verify the file type.  This function absorbs error codes. */
-	error = xchk_dir_check_ftype(sdc, offset, lookup_ino, type);
+	/* Verify the inode pointer.  This function absorbs error codes. */
+	error = xchk_dir_check_iptr(sdc, offset, lookup_ino, type);
 	if (error)
 		goto out;
 out: