From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: sandeen@sandeen.net
Cc: linux-xfs@vger.kernel.org, alex@zadara.com
Subject: Re: [PATCH 1/7] xfs_repair: replace verify_inum with libxfs inode validators
Date: Tue, 4 Feb 2020 16:50:33 -0800 [thread overview]
Message-ID: <20200205005033.GC6870@magnolia> (raw)
In-Reply-To: <158086360402.2079685.8627541630086580270.stgit@magnolia>
On Tue, Feb 04, 2020 at 04:46:44PM -0800, Darrick J. Wong wrote:
> 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.
Whoops, this was supposed to be in the previous series, not this one.
--D
> 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;
> }
>
next prev parent reply other threads:[~2020-02-05 0:50 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 ` [PATCH 1/7] xfs_repair: replace verify_inum with libxfs inode validators Darrick J. Wong
2020-02-05 0:50 ` Darrick J. Wong [this message]
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=20200205005033.GC6870@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).