| Message ID | 160704433239.734470.17983823126192826984.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: strengthen log intent validation | expand |
On Thu, Dec 03, 2020 at 05:12:12PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > The code that validates recovered refcount intent items is kind of a > mess -- it doesn't use the standard xfs type validators, and it doesn't > check for things that it should. Fix the validator function to use the > standard validation helpers and look for more types of obvious errors. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/xfs_refcount_item.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > > diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c > index a456a2fb794c..8ad6c81f6d8f 100644 > --- a/fs/xfs/xfs_refcount_item.c > +++ b/fs/xfs/xfs_refcount_item.c > @@ -423,27 +423,26 @@ xfs_cui_validate_phys( > struct xfs_mount *mp, > struct xfs_phys_extent *refc) > { > - xfs_fsblock_t startblock_fsb; > - bool op_ok; > + if (refc->pe_flags & ~XFS_REFCOUNT_EXTENT_FLAGS) > + return false; > > - startblock_fsb = XFS_BB_TO_FSB(mp, > - XFS_FSB_TO_DADDR(mp, refc->pe_startblock)); > switch (refc->pe_flags & XFS_REFCOUNT_EXTENT_TYPE_MASK) { > case XFS_REFCOUNT_INCREASE: > case XFS_REFCOUNT_DECREASE: > case XFS_REFCOUNT_ALLOC_COW: > case XFS_REFCOUNT_FREE_COW: > - op_ok = true; > break; > default: > - op_ok = false; > - break; > + return false; > } > - if (!op_ok || startblock_fsb == 0 || > - refc->pe_len == 0 || > - startblock_fsb >= mp->m_sb.sb_dblocks || > - refc->pe_len >= mp->m_sb.sb_agblocks || > - (refc->pe_flags & ~XFS_REFCOUNT_EXTENT_FLAGS)) > + > + if (refc->pe_startblock + refc->pe_len <= refc->pe_startblock) > + return false; > + > + if (!xfs_verify_fsbno(mp, refc->pe_startblock)) > + return false; > + > + if (!xfs_verify_fsbno(mp, refc->pe_startblock + refc->pe_len - 1)) > return false; > > return true; >
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c index a456a2fb794c..8ad6c81f6d8f 100644 --- a/fs/xfs/xfs_refcount_item.c +++ b/fs/xfs/xfs_refcount_item.c @@ -423,27 +423,26 @@ xfs_cui_validate_phys( struct xfs_mount *mp, struct xfs_phys_extent *refc) { - xfs_fsblock_t startblock_fsb; - bool op_ok; + if (refc->pe_flags & ~XFS_REFCOUNT_EXTENT_FLAGS) + return false; - startblock_fsb = XFS_BB_TO_FSB(mp, - XFS_FSB_TO_DADDR(mp, refc->pe_startblock)); switch (refc->pe_flags & XFS_REFCOUNT_EXTENT_TYPE_MASK) { case XFS_REFCOUNT_INCREASE: case XFS_REFCOUNT_DECREASE: case XFS_REFCOUNT_ALLOC_COW: case XFS_REFCOUNT_FREE_COW: - op_ok = true; break; default: - op_ok = false; - break; + return false; } - if (!op_ok || startblock_fsb == 0 || - refc->pe_len == 0 || - startblock_fsb >= mp->m_sb.sb_dblocks || - refc->pe_len >= mp->m_sb.sb_agblocks || - (refc->pe_flags & ~XFS_REFCOUNT_EXTENT_FLAGS)) + + if (refc->pe_startblock + refc->pe_len <= refc->pe_startblock) + return false; + + if (!xfs_verify_fsbno(mp, refc->pe_startblock)) + return false; + + if (!xfs_verify_fsbno(mp, refc->pe_startblock + refc->pe_len - 1)) return false; return true;