| Message ID | 160704432023.734470.2330496983684892697.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:00PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > The code that validates recovered rmap 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: Brian Foster <bfoster@redhat.com> > fs/xfs/xfs_rmap_item.c | 30 ++++++++++++++++++------------ > 1 file changed, 18 insertions(+), 12 deletions(-) > > > diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c > index d77d104d93c6..f296ec349936 100644 > --- a/fs/xfs/xfs_rmap_item.c > +++ b/fs/xfs/xfs_rmap_item.c > @@ -466,11 +466,9 @@ xfs_rui_validate_map( > struct xfs_mount *mp, > struct xfs_map_extent *rmap) > { > - xfs_fsblock_t startblock_fsb; > - bool op_ok; > + if (rmap->me_flags & ~XFS_RMAP_EXTENT_FLAGS) > + return false; > > - startblock_fsb = XFS_BB_TO_FSB(mp, > - XFS_FSB_TO_DADDR(mp, rmap->me_startblock)); > switch (rmap->me_flags & XFS_RMAP_EXTENT_TYPE_MASK) { > case XFS_RMAP_EXTENT_MAP: > case XFS_RMAP_EXTENT_MAP_SHARED: > @@ -480,17 +478,25 @@ xfs_rui_validate_map( > case XFS_RMAP_EXTENT_CONVERT_SHARED: > case XFS_RMAP_EXTENT_ALLOC: > case XFS_RMAP_EXTENT_FREE: > - op_ok = true; > break; > default: > - op_ok = false; > - break; > + return false; > } > - if (!op_ok || startblock_fsb == 0 || > - rmap->me_len == 0 || > - startblock_fsb >= mp->m_sb.sb_dblocks || > - rmap->me_len >= mp->m_sb.sb_agblocks || > - (rmap->me_flags & ~XFS_RMAP_EXTENT_FLAGS)) > + > + if (!XFS_RMAP_NON_INODE_OWNER(rmap->me_owner) && > + !xfs_verify_ino(mp, rmap->me_owner)) > + return false; > + > + if (rmap->me_startoff + rmap->me_len <= rmap->me_startoff) > + return false; > + > + if (rmap->me_startblock + rmap->me_len <= rmap->me_startblock) > + return false; > + > + if (!xfs_verify_fsbno(mp, rmap->me_startblock)) > + return false; > + > + if (!xfs_verify_fsbno(mp, rmap->me_startblock + rmap->me_len - 1)) > return false; > > return true; >
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c index d77d104d93c6..f296ec349936 100644 --- a/fs/xfs/xfs_rmap_item.c +++ b/fs/xfs/xfs_rmap_item.c @@ -466,11 +466,9 @@ xfs_rui_validate_map( struct xfs_mount *mp, struct xfs_map_extent *rmap) { - xfs_fsblock_t startblock_fsb; - bool op_ok; + if (rmap->me_flags & ~XFS_RMAP_EXTENT_FLAGS) + return false; - startblock_fsb = XFS_BB_TO_FSB(mp, - XFS_FSB_TO_DADDR(mp, rmap->me_startblock)); switch (rmap->me_flags & XFS_RMAP_EXTENT_TYPE_MASK) { case XFS_RMAP_EXTENT_MAP: case XFS_RMAP_EXTENT_MAP_SHARED: @@ -480,17 +478,25 @@ xfs_rui_validate_map( case XFS_RMAP_EXTENT_CONVERT_SHARED: case XFS_RMAP_EXTENT_ALLOC: case XFS_RMAP_EXTENT_FREE: - op_ok = true; break; default: - op_ok = false; - break; + return false; } - if (!op_ok || startblock_fsb == 0 || - rmap->me_len == 0 || - startblock_fsb >= mp->m_sb.sb_dblocks || - rmap->me_len >= mp->m_sb.sb_agblocks || - (rmap->me_flags & ~XFS_RMAP_EXTENT_FLAGS)) + + if (!XFS_RMAP_NON_INODE_OWNER(rmap->me_owner) && + !xfs_verify_ino(mp, rmap->me_owner)) + return false; + + if (rmap->me_startoff + rmap->me_len <= rmap->me_startoff) + return false; + + if (rmap->me_startblock + rmap->me_len <= rmap->me_startblock) + return false; + + if (!xfs_verify_fsbno(mp, rmap->me_startblock)) + return false; + + if (!xfs_verify_fsbno(mp, rmap->me_startblock + rmap->me_len - 1)) return false; return true;