On Mon, Mar 02, 2020 at 11:12:17AM -0700, Allison Collins wrote:
> On 2/28/20 6:48 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Check the owner field of dir3 data block headers.
> "Check the owner field of dir3 data block headers, and release the buffer on
> error." ?
>
> It's a bit of an api change though isnt it? Do we need to go find all the
> callers and make sure there's not going to be a double release if error ==
> -EFSCORRUPTED ?
There shouldn't be, since we set *bpp to NULL before returning
EFSCORRUPTED. The callers all seemed to handle nonzero return and/or
null bp properly.
(I dunno, did I miss one? It's entirely likely... :))
--D
> Allison
>
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_dir2_data.c | 32 +++++++++++++++++++++++++++++++-
> > 1 file changed, 31 insertions(+), 1 deletion(-)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> > index b9eba8213180..e5910bc9ab83 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_data.c
> > +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> > @@ -394,6 +394,22 @@ static const struct xfs_buf_ops xfs_dir3_data_reada_buf_ops = {
> > .verify_write = xfs_dir3_data_write_verify,
> > };
> > +static xfs_failaddr_t
> > +xfs_dir3_data_header_check(
> > + struct xfs_inode *dp,
> > + struct xfs_buf *bp)
> > +{
> > + struct xfs_mount *mp = dp->i_mount;
> > +
> > + if (xfs_sb_version_hascrc(&mp->m_sb)) {
> > + struct xfs_dir3_data_hdr *hdr3 = bp->b_addr;
> > +
> > + if (be64_to_cpu(hdr3->hdr.owner) != dp->i_ino)
> > + return __this_address;
> > + }
> > +
> > + return NULL;
> > +}
> > int
> > xfs_dir3_data_read(
> > @@ -403,11 +419,25 @@ xfs_dir3_data_read(
> > unsigned int flags,
> > struct xfs_buf **bpp)
> > {
> > + xfs_failaddr_t fa;
> > int err;
> > err = xfs_da_read_buf(tp, dp, bno, flags, bpp, XFS_DATA_FORK,
> > &xfs_dir3_data_buf_ops);
> > - if (!err && tp && *bpp)
> > + if (err || !*bpp)
> > + return err;
> > +
> > + /* Check things that we can't do in the verifier. */
> > + fa = xfs_dir3_data_header_check(dp, *bpp);
> > + if (fa) {
> > + xfs_verifier_error(*bpp, -EFSCORRUPTED, fa);
> > + (*bpp)->b_flags &= ~XBF_DONE;
> > + xfs_trans_brelse(tp, *bpp);
> > + *bpp = NULL;
> > + return -EFSCORRUPTED;
> > + }
> > +
> > + if (tp)
> > xfs_trans_buf_set_type(tp, *bpp, XFS_BLFT_DIR_DATA_BUF);
> > return err;
> > }
> >
