All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: sandeen@sandeen.net, darrick.wong@oracle.com
Cc: linux-xfs@vger.kernel.org, alex@zadara.com
Subject: [PATCH 1/7] xfs_repair: replace verify_inum with libxfs inode validators
Date: Tue, 04 Feb 2020 16:46:44 -0800	[thread overview]
Message-ID: <158086360402.2079685.8627541630086580270.stgit@magnolia> (raw)
In-Reply-To: <158086359783.2079685.9581209719946834913.stgit@magnolia>

From: Darrick J. Wong <darrick.wong@oracle.com>

Repair uses the verify_inum function to validate inode numbers that it
finds in the superblock and in directories.  libxfs now has validator
functions to cover that kind of thing, so remove verify_inum().  As a
side bonus, this means that we will flag directories that point to the
quota/realtime metadata inodes.

This fixes a regression found by fuzzing u3.sfdir3.hdr.parent.i4 to
lastbit (aka making a directory's .. point to the user quota inode) in
xfs/384.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/libxfs_api_defs.h |    1 +
 repair/dino_chunks.c     |    2 +-
 repair/dinode.c          |   29 -----------------------------
 repair/dinode.h          |    4 ----
 repair/dir2.c            |    7 +++----
 repair/phase4.c          |   12 ++++++------
 repair/phase6.c          |    8 ++++----
 7 files changed, 15 insertions(+), 48 deletions(-)


diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 6e09685b..9daf2635 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -176,6 +176,7 @@
 #define xfs_trans_roll			libxfs_trans_roll
 
 #define xfs_verify_cksum		libxfs_verify_cksum
+#define xfs_verify_dir_ino		libxfs_verify_dir_ino
 #define xfs_verify_ino			libxfs_verify_ino
 #define xfs_verify_rtbno		libxfs_verify_rtbno
 #define xfs_zero_extent			libxfs_zero_extent
diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
index 00b67468..dbf3d37a 100644
--- a/repair/dino_chunks.c
+++ b/repair/dino_chunks.c
@@ -65,7 +65,7 @@ check_aginode_block(xfs_mount_t	*mp,
  * inode chunk.  returns number of new inodes if things are good
  * and 0 if bad.  start is the start of the discovered inode chunk.
  * routine assumes that ino is a legal inode number
- * (verified by verify_inum()).  If the inode chunk turns out
+ * (verified by libxfs_verify_ino()).  If the inode chunk turns out
  * to be good, this routine will put the inode chunk into
  * the good inode chunk tree if required.
  *
diff --git a/repair/dinode.c b/repair/dinode.c
index 8af2cb25..0d9c96be 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -171,35 +171,6 @@ verify_ag_bno(xfs_sb_t *sbp,
 	return 1;
 }
 
-/*
- * returns 0 if inode number is valid, 1 if bogus
- */
-int
-verify_inum(xfs_mount_t		*mp,
-		xfs_ino_t	ino)
-{
-	xfs_agnumber_t	agno;
-	xfs_agino_t	agino;
-	xfs_agblock_t	agbno;
-	xfs_sb_t	*sbp = &mp->m_sb;;
-
-	/* range check ag #, ag block.  range-checking offset is pointless */
-
-	agno = XFS_INO_TO_AGNO(mp, ino);
-	agino = XFS_INO_TO_AGINO(mp, ino);
-	agbno = XFS_AGINO_TO_AGBNO(mp, agino);
-	if (agbno == 0)
-		return 1;
-
-	if (ino == 0 || ino == NULLFSINO)
-		return(1);
-
-	if (ino != XFS_AGINO_TO_INO(mp, agno, agino))
-		return(1);
-
-	return verify_ag_bno(sbp, agno, agbno);
-}
-
 /*
  * have a separate routine to ensure that we don't accidentally
  * lose illegally set bits in the agino by turning it into an FSINO
diff --git a/repair/dinode.h b/repair/dinode.h
index aa177465..98238357 100644
--- a/repair/dinode.h
+++ b/repair/dinode.h
@@ -77,10 +77,6 @@ verify_uncertain_dinode(xfs_mount_t *mp,
 		xfs_agnumber_t agno,
 		xfs_agino_t ino);
 
-int
-verify_inum(xfs_mount_t		*mp,
-		xfs_ino_t	ino);
-
 int
 verify_aginum(xfs_mount_t	*mp,
 		xfs_agnumber_t	agno,
diff --git a/repair/dir2.c b/repair/dir2.c
index e43a9732..723aee1f 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -215,7 +215,7 @@ process_sf_dir2(
 		if (lino == ino) {
 			junkit = 1;
 			junkreason = _("current");
-		} else if (verify_inum(mp, lino)) {
+		} else if (!libxfs_verify_dir_ino(mp, lino)) {
 			junkit = 1;
 			junkreason = _("invalid");
 		} else if (lino == mp->m_sb.sb_rbmino)  {
@@ -486,8 +486,7 @@ _("corrected entry offsets in directory %" PRIu64 "\n"),
 	 * If the validation fails for the root inode we fix it in
 	 * the next else case.
 	 */
-	if (verify_inum(mp, *parent) && ino != mp->m_sb.sb_rootino)  {
-
+	if (!libxfs_verify_dir_ino(mp, *parent) && ino != mp->m_sb.sb_rootino) {
 		do_warn(
 _("bogus .. inode number (%" PRIu64 ") in directory inode %" PRIu64 ", "),
 				*parent, ino);
@@ -674,7 +673,7 @@ process_dir2_data(
 			 * (or did it ourselves) during phase 3.
 			 */
 			clearino = 0;
-		} else if (verify_inum(mp, ent_ino)) {
+		} else if (!libxfs_verify_dir_ino(mp, ent_ino)) {
 			/*
 			 * Bad inode number.  Clear the inode number and the
 			 * entry will get removed later.  We don't trash the
diff --git a/repair/phase4.c b/repair/phase4.c
index e1ba778f..8197db06 100644
--- a/repair/phase4.c
+++ b/repair/phase4.c
@@ -36,7 +36,7 @@ quotino_check(xfs_mount_t *mp)
 	ino_tree_node_t *irec;
 
 	if (mp->m_sb.sb_uquotino != NULLFSINO && mp->m_sb.sb_uquotino != 0)  {
-		if (verify_inum(mp, mp->m_sb.sb_uquotino))
+		if (!libxfs_verify_ino(mp, mp->m_sb.sb_uquotino))
 			irec = NULL;
 		else
 			irec = find_inode_rec(mp,
@@ -52,7 +52,7 @@ quotino_check(xfs_mount_t *mp)
 	}
 
 	if (mp->m_sb.sb_gquotino != NULLFSINO && mp->m_sb.sb_gquotino != 0)  {
-		if (verify_inum(mp, mp->m_sb.sb_gquotino))
+		if (!libxfs_verify_ino(mp, mp->m_sb.sb_gquotino))
 			irec = NULL;
 		else
 			irec = find_inode_rec(mp,
@@ -68,7 +68,7 @@ quotino_check(xfs_mount_t *mp)
 	}
 
 	if (mp->m_sb.sb_pquotino != NULLFSINO && mp->m_sb.sb_pquotino != 0)  {
-		if (verify_inum(mp, mp->m_sb.sb_pquotino))
+		if (!libxfs_verify_ino(mp, mp->m_sb.sb_pquotino))
 			irec = NULL;
 		else
 			irec = find_inode_rec(mp,
@@ -112,9 +112,9 @@ quota_sb_check(xfs_mount_t *mp)
 	    (mp->m_sb.sb_pquotino == NULLFSINO || mp->m_sb.sb_pquotino == 0))  {
 		lost_quotas = 1;
 		fs_quotas = 0;
-	} else if (!verify_inum(mp, mp->m_sb.sb_uquotino) &&
-			!verify_inum(mp, mp->m_sb.sb_gquotino) &&
-			!verify_inum(mp, mp->m_sb.sb_pquotino)) {
+	} else if (libxfs_verify_ino(mp, mp->m_sb.sb_uquotino) &&
+		   libxfs_verify_ino(mp, mp->m_sb.sb_gquotino) &&
+		   libxfs_verify_ino(mp, mp->m_sb.sb_pquotino)) {
 		fs_quotas = 1;
 	}
 }
diff --git a/repair/phase6.c b/repair/phase6.c
index 0874b649..70135694 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -1814,7 +1814,7 @@ longform_dir2_entry_check_data(
 			}
 			continue;
 		}
-		ASSERT(no_modify || !verify_inum(mp, inum));
+		ASSERT(no_modify || libxfs_verify_dir_ino(mp, inum));
 		/*
 		 * special case the . entry.  we know there's only one
 		 * '.' and only '.' points to itself because bogus entries
@@ -1845,7 +1845,7 @@ longform_dir2_entry_check_data(
 		/*
 		 * skip entries with bogus inumbers if we're in no modify mode
 		 */
-		if (no_modify && verify_inum(mp, inum))
+		if (no_modify && !libxfs_verify_dir_ino(mp, inum))
 			continue;
 
 		/* validate ftype field if supported */
@@ -2634,14 +2634,14 @@ shortform_dir2_entry_check(xfs_mount_t	*mp,
 		fname[sfep->namelen] = '\0';
 
 		ASSERT(no_modify || (lino != NULLFSINO && lino != 0));
-		ASSERT(no_modify || !verify_inum(mp, lino));
+		ASSERT(no_modify || libxfs_verify_dir_ino(mp, lino));
 
 		/*
 		 * Also skip entries with bogus inode numbers if we're
 		 * in no modify mode.
 		 */
 
-		if (no_modify && verify_inum(mp, lino))  {
+		if (no_modify && !libxfs_verify_dir_ino(mp, lino))  {
 			next_sfep = libxfs_dir2_sf_nextentry(mp, sfp, sfep);
 			continue;
 		}


  reply	other threads:[~2020-02-05  0:46 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-05  0:46 [PATCH v4 0/7] xfs_repair: do not trash valid root dirs Darrick J. Wong
2020-02-05  0:46 ` Darrick J. Wong [this message]
2020-02-05  0:50   ` [PATCH 1/7] xfs_repair: replace verify_inum with libxfs inode validators Darrick J. Wong
2020-02-17 13:50   ` Christoph Hellwig
2020-02-19  4:32     ` Darrick J. Wong
2020-02-26 16:55   ` Eric Sandeen
2020-02-05  0:46 ` [PATCH 2/7] mkfs: check root inode location Darrick J. Wong
2020-02-17 13:50   ` Christoph Hellwig
2020-02-05  0:46 ` [PATCH 3/7] xfs_repair: enforce that inode btree chunks can't point to AG headers Darrick J. Wong
2020-02-17 13:51   ` Christoph Hellwig
2020-02-26 17:09   ` Eric Sandeen
2020-02-26 17:24     ` Darrick J. Wong
2020-02-26 17:34       ` Eric Sandeen
2020-02-26 17:19   ` Eric Sandeen
2020-02-26 17:32     ` Darrick J. Wong
2020-02-26 17:40       ` Eric Sandeen
2020-02-05  0:47 ` [PATCH 4/7] xfs_repair: refactor fixed inode location checks Darrick J. Wong
2020-02-17 13:51   ` Christoph Hellwig
2020-02-05  0:47 ` [PATCH 5/7] xfs_repair: use libxfs function to calculate root inode location Darrick J. Wong
2020-02-17 13:53   ` Christoph Hellwig
2020-02-05  0:47 ` [PATCH 6/7] xfs_repair: check plausibility of root dir pointer before trashing it Darrick J. Wong
2020-02-17 13:53   ` Christoph Hellwig
2020-02-05  0:47 ` [PATCH 7/7] xfs_repair: try to correct sb_unit value from secondaries Darrick J. Wong
2020-02-17 13:55   ` Christoph Hellwig
2020-02-26 17:42   ` Eric Sandeen
2020-02-26 17:55     ` 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=158086360402.2079685.8627541630086580270.stgit@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=alex@zadara.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@sandeen.net \
    /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.