All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v2 7/7] xfs: factor xfs_da3_blkinfo verification into common helper
Date: Thu, 31 Jan 2019 13:04:28 -0500	[thread overview]
Message-ID: <20190131180428.GB36239@bfoster> (raw)
In-Reply-To: <20190131175113.GB5761@magnolia>

On Thu, Jan 31, 2019 at 09:51:13AM -0800, Darrick J. Wong wrote:
> On Thu, Jan 31, 2019 at 10:46:08AM -0500, Brian Foster wrote:
> > With the verifier magic value helper in place, we've left a bit more
> > duplicate code across the verifiers that involve struct
> > xfs_da3_blkinfo. This includes the da node, xattr leaf and dir leaf
> > verifiers, all of which perform similar checks for v4 and v5
> > filesystems.
> > 
> > Create a common helper to verify an xfs_da3_blkinfo structure,
> > taking care to only access v5 fields where appropriate, and refactor
> > the aforementioned verifiers to use the helper. No functional
> > changes.
> > 
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > ---
> >  fs/xfs/libxfs/xfs_attr_leaf.c | 17 +++----------
> >  fs/xfs/libxfs/xfs_da_btree.c  | 46 +++++++++++++++++++++++++----------
> >  fs/xfs/libxfs/xfs_da_format.h |  2 ++
> >  fs/xfs/libxfs/xfs_dir2_leaf.c | 18 +++-----------
> >  4 files changed, 43 insertions(+), 40 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> > index dde3a550d7ce..0c92987f57fc 100644
> > --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> > @@ -245,23 +245,14 @@ xfs_attr3_leaf_verify(
> >  	struct xfs_attr_leaf_entry	*entries;
> >  	uint32_t			end;	/* must be 32bit - see below */
> >  	int				i;
> > +	xfs_failaddr_t			fa;
> >  
> >  	xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &ichdr, leaf);
> >  
> > -	if (!xfs_verify_magic(bp, leaf->hdr.info.magic))
> > -		return __this_address;
> > -
> > -	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> > -		struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
> > +	fa = xfs_da3_blkinfo_verify(bp, bp->b_addr);
> > +	if (fa)
> > +		return fa;
> >  
> > -		ASSERT(leaf->hdr.info.magic == hdr3->info.hdr.magic);
> > -		if (!uuid_equal(&hdr3->info.uuid, &mp->m_sb.sb_meta_uuid))
> > -			return __this_address;
> > -		if (be64_to_cpu(hdr3->info.blkno) != bp->b_bn)
> > -			return __this_address;
> > -		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->info.lsn)))
> > -			return __this_address;
> > -	}
> >  	/*
> >  	 * In recovery there is a transient state where count == 0 is valid
> >  	 * because we may have transitioned an empty shortform attr to a leaf
> > diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> > index 6b8ca390eecc..622f88a1b1aa 100644
> > --- a/fs/xfs/libxfs/xfs_da_btree.c
> > +++ b/fs/xfs/libxfs/xfs_da_btree.c
> > @@ -116,6 +116,35 @@ xfs_da_state_free(xfs_da_state_t *state)
> >  	kmem_zone_free(xfs_da_state_zone, state);
> >  }
> >  
> > +/*
> > + * Verify an xfs_da3_blkinfo structure. Note that the da3 fields are only
> > + * accessible on v5 filesystems. This header format is common across da node,
> > + * attr leaf and dir leaf blocks.
> > + */
> > +xfs_failaddr_t
> > +xfs_da3_blkinfo_verify(
> > +	struct xfs_buf		*bp,
> > +	struct xfs_da3_blkinfo	*hdr3)
> > +{
> > +	struct xfs_mount	*mp = bp->b_target->bt_mount;
> > +	struct xfs_da_blkinfo	*hdr = &hdr3->hdr;
> > +
> > +	if (!xfs_verify_magic(bp, hdr->magic))
> > +		return __this_address;
> > +
> > +	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> > +		ASSERT(hdr3->hdr.magic == hdr->magic);
> > +		if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
> > +			return __this_address;
> > +		if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
> > +			return __this_address;
> > +		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
> > +			return __this_address;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static xfs_failaddr_t
> >  xfs_da3_node_verify(
> >  	struct xfs_buf		*bp)
> > @@ -124,25 +153,16 @@ xfs_da3_node_verify(
> >  	struct xfs_da_intnode	*hdr = bp->b_addr;
> >  	struct xfs_da3_icnode_hdr ichdr;
> >  	const struct xfs_dir_ops *ops;
> > +	xfs_failaddr_t		fa;
> >  
> >  	ops = xfs_dir_get_ops(mp, NULL);
> >  
> >  	ops->node_hdr_from_disk(&ichdr, hdr);
> >  
> > -	if (!xfs_verify_magic(bp, hdr->hdr.info.magic))
> > -		return __this_address;
> > +	fa = xfs_da3_blkinfo_verify(bp, bp->b_addr);
> > +	if (fa)
> > +		return fa;
> >  
> > -	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> > -		struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
> > -
> > -		ASSERT(hdr3->info.hdr.magic == hdr->hdr.info.magic);
> > -		if (!uuid_equal(&hdr3->info.uuid, &mp->m_sb.sb_meta_uuid))
> > -			return __this_address;
> > -		if (be64_to_cpu(hdr3->info.blkno) != bp->b_bn)
> > -			return __this_address;
> > -		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->info.lsn)))
> > -			return __this_address;
> > -	}
> >  	if (ichdr.level == 0)
> >  		return __this_address;
> >  	if (ichdr.level > XFS_DA_NODE_MAXDEPTH)
> > diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> > index 5d5bf3bffc78..bf0c53bd2e60 100644
> > --- a/fs/xfs/libxfs/xfs_da_format.h
> > +++ b/fs/xfs/libxfs/xfs_da_format.h
> > @@ -869,4 +869,6 @@ static inline unsigned int xfs_dir2_dirblock_bytes(struct xfs_sb *sbp)
> >  	return 1 << (sbp->sb_blocklog + sbp->sb_dirblklog);
> >  }
> >  
> > +xfs_failaddr_t xfs_da3_blkinfo_verify(struct xfs_buf *, struct xfs_da3_blkinfo*);
> 
> Specify parameter names in the declaration, please.
> 

Ok, will fix.

Brian

> --D
> 
> > +
> >  #endif /* __XFS_DA_FORMAT_H__ */
> > diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> > index a99ae2cd292e..094028b7b162 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> > +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> > @@ -146,21 +146,11 @@ xfs_dir3_leaf_verify(
> >  {
> >  	struct xfs_mount	*mp = bp->b_target->bt_mount;
> >  	struct xfs_dir2_leaf	*leaf = bp->b_addr;
> > +	xfs_failaddr_t		fa;
> >  
> > -	if (!xfs_verify_magic(bp, leaf->hdr.info.magic))
> > -		return __this_address;
> > -
> > -	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> > -		struct xfs_dir3_leaf_hdr *leaf3 = bp->b_addr;
> > -
> > -		ASSERT(leaf3->info.hdr.magic == leaf->hdr.info.magic);
> > -		if (!uuid_equal(&leaf3->info.uuid, &mp->m_sb.sb_meta_uuid))
> > -			return __this_address;
> > -		if (be64_to_cpu(leaf3->info.blkno) != bp->b_bn)
> > -			return __this_address;
> > -		if (!xfs_log_check_lsn(mp, be64_to_cpu(leaf3->info.lsn)))
> > -			return __this_address;
> > -	}
> > +	fa = xfs_da3_blkinfo_verify(bp, bp->b_addr);
> > +	if (fa)
> > +		return fa;
> >  
> >  	return xfs_dir3_leaf_check_int(mp, NULL, NULL, leaf);
> >  }
> > -- 
> > 2.17.2
> > 

  reply	other threads:[~2019-01-31 18:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-31 15:46 [PATCH v2 0/7] xfs: fix [f]inobt magic value verification Brian Foster
2019-01-31 15:46 ` [PATCH v2 1/7] xfs: set buffer ops when repair probes for btree type Brian Foster
2019-01-31 15:46 ` [PATCH v2 2/7] xfs: always check magic values in on-disk byte order Brian Foster
2019-01-31 15:46 ` [PATCH v2 3/7] xfs: create a separate finobt verifier Brian Foster
2019-01-31 15:46 ` [PATCH v2 4/7] xfs: distinguish between inobt and finobt magic values Brian Foster
2019-01-31 15:46 ` [PATCH v2 5/7] xfs: use verifier magic field in dir2 leaf verifiers Brian Foster
2019-01-31 15:46 ` [PATCH v2 6/7] xfs: miscellaneous verifier magic value fixups Brian Foster
2019-01-31 15:46 ` [PATCH v2 7/7] xfs: factor xfs_da3_blkinfo verification into common helper Brian Foster
2019-01-31 17:51   ` Darrick J. Wong
2019-01-31 18:04     ` Brian Foster [this message]
2019-01-31 17:54 ` [PATCH v2 0/7] xfs: fix [f]inobt magic value verification Darrick J. Wong
2019-01-31 18:03   ` Brian Foster

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=20190131180428.GB36239@bfoster \
    --to=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --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.