Commit 0c1d9e4a authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Darrick J. Wong

xfs: simplify validation of the unwritten extent bit

XFS only supports the unwritten extent bit in the data fork, and only if
the file system has a version 5 superblock or the unwritten extent
feature bit.

We currently have two routines that validate the invariant:
xfs_check_nostate_extents which return -EFSCORRUPTED when it's not met,
and xfs_validate_extent that triggers and assert in debug build.

Both of them iterate over all extents of an inode fork when called,
which isn't very efficient.

This patch instead adds a new helper that verifies the invariant one
extent at a time, and calls it from the places where we iterate over
all extents to converted them from or two the in-memory format.  The
callers then return -EFSCORRUPTED when reading invalid extents from
disk, or trigger an assert when writing them to disk.
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent 37f7f9bb
...@@ -1231,7 +1231,6 @@ xfs_bmap_read_extents( ...@@ -1231,7 +1231,6 @@ xfs_bmap_read_extents(
xfs_fsblock_t bno; /* block # of "block" */ xfs_fsblock_t bno; /* block # of "block" */
xfs_buf_t *bp; /* buffer for "block" */ xfs_buf_t *bp; /* buffer for "block" */
int error; /* error return value */ int error; /* error return value */
xfs_exntfmt_t exntf; /* XFS_EXTFMT_NOSTATE, if checking */
xfs_extnum_t i, j; /* index into the extents list */ xfs_extnum_t i, j; /* index into the extents list */
xfs_ifork_t *ifp; /* fork structure */ xfs_ifork_t *ifp; /* fork structure */
int level; /* btree level, for checking */ int level; /* btree level, for checking */
...@@ -1242,8 +1241,6 @@ xfs_bmap_read_extents( ...@@ -1242,8 +1241,6 @@ xfs_bmap_read_extents(
mp = ip->i_mount; mp = ip->i_mount;
ifp = XFS_IFORK_PTR(ip, whichfork); ifp = XFS_IFORK_PTR(ip, whichfork);
exntf = (whichfork != XFS_DATA_FORK) ? XFS_EXTFMT_NOSTATE :
XFS_EXTFMT_INODE(ip);
block = ifp->if_broot; block = ifp->if_broot;
/* /*
* Root level must use BMAP_BROOT_PTR_ADDR macro to get ptr out. * Root level must use BMAP_BROOT_PTR_ADDR macro to get ptr out.
...@@ -1311,18 +1308,9 @@ xfs_bmap_read_extents( ...@@ -1311,18 +1308,9 @@ xfs_bmap_read_extents(
xfs_bmbt_rec_host_t *trp = xfs_iext_get_ext(ifp, i); xfs_bmbt_rec_host_t *trp = xfs_iext_get_ext(ifp, i);
trp->l0 = be64_to_cpu(frp->l0); trp->l0 = be64_to_cpu(frp->l0);
trp->l1 = be64_to_cpu(frp->l1); trp->l1 = be64_to_cpu(frp->l1);
} if (!xfs_bmbt_validate_extent(mp, whichfork, trp)) {
if (exntf == XFS_EXTFMT_NOSTATE) {
/*
* Check all attribute bmap btree records and
* any "older" data bmap btree records for a
* set bit in the "extent flag" position.
*/
if (unlikely(xfs_check_nostate_extents(ifp,
start, num_recs))) {
XFS_ERROR_REPORT("xfs_bmap_read_extents(2)", XFS_ERROR_REPORT("xfs_bmap_read_extents(2)",
XFS_ERRLEVEL_LOW, XFS_ERRLEVEL_LOW, mp);
ip->i_mount);
goto error0; goto error0;
} }
} }
......
...@@ -244,8 +244,6 @@ int xfs_bmap_del_extent_delay(struct xfs_inode *ip, int whichfork, ...@@ -244,8 +244,6 @@ int xfs_bmap_del_extent_delay(struct xfs_inode *ip, int whichfork,
struct xfs_bmbt_irec *del); struct xfs_bmbt_irec *del);
void xfs_bmap_del_extent_cow(struct xfs_inode *ip, xfs_extnum_t *idx, void xfs_bmap_del_extent_cow(struct xfs_inode *ip, xfs_extnum_t *idx,
struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *del); struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *del);
int xfs_check_nostate_extents(struct xfs_ifork *ifp, xfs_extnum_t idx,
xfs_extnum_t num);
uint xfs_default_attroffset(struct xfs_inode *ip); uint xfs_default_attroffset(struct xfs_inode *ip);
int xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip, int xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip,
xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb, xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
......
...@@ -366,32 +366,6 @@ xfs_bmbt_to_bmdr( ...@@ -366,32 +366,6 @@ xfs_bmbt_to_bmdr(
memcpy(tpp, fpp, sizeof(*fpp) * dmxr); memcpy(tpp, fpp, sizeof(*fpp) * dmxr);
} }
/*
* Check extent records, which have just been read, for
* any bit in the extent flag field. ASSERT on debug
* kernels, as this condition should not occur.
* Return an error condition (1) if any flags found,
* otherwise return 0.
*/
int
xfs_check_nostate_extents(
xfs_ifork_t *ifp,
xfs_extnum_t idx,
xfs_extnum_t num)
{
for (; num > 0; num--, idx++) {
xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, idx);
if ((ep->l0 >>
(64 - BMBT_EXNTFLAG_BITLEN)) != 0) {
ASSERT(0);
return 1;
}
}
return 0;
}
STATIC struct xfs_btree_cur * STATIC struct xfs_btree_cur *
xfs_bmbt_dup_cursor( xfs_bmbt_dup_cursor(
struct xfs_btree_cur *cur) struct xfs_btree_cur *cur)
......
...@@ -24,13 +24,6 @@ struct xfs_mount; ...@@ -24,13 +24,6 @@ struct xfs_mount;
struct xfs_inode; struct xfs_inode;
struct xfs_trans; struct xfs_trans;
/*
* Extent state and extent format macros.
*/
#define XFS_EXTFMT_INODE(x) \
(xfs_sb_version_hasextflgbit(&((x)->i_mount->m_sb)) ? \
XFS_EXTFMT_HASSTATE : XFS_EXTFMT_NOSTATE)
/* /*
* Btree block header size depends on a superblock flag. * Btree block header size depends on a superblock flag.
*/ */
...@@ -139,4 +132,18 @@ extern int xfs_bmbt_change_owner(struct xfs_trans *tp, struct xfs_inode *ip, ...@@ -139,4 +132,18 @@ extern int xfs_bmbt_change_owner(struct xfs_trans *tp, struct xfs_inode *ip,
extern struct xfs_btree_cur *xfs_bmbt_init_cursor(struct xfs_mount *, extern struct xfs_btree_cur *xfs_bmbt_init_cursor(struct xfs_mount *,
struct xfs_trans *, struct xfs_inode *, int); struct xfs_trans *, struct xfs_inode *, int);
/*
* Check that the extent does not contain an invalid unwritten extent flag.
*/
static inline bool xfs_bmbt_validate_extent(struct xfs_mount *mp, int whichfork,
struct xfs_bmbt_rec_host *ep)
{
if (ep->l0 >> (64 - BMBT_EXNTFLAG_BITLEN) == 0)
return true;
if (whichfork == XFS_DATA_FORK &&
xfs_sb_version_hasextflgbit(&mp->m_sb))
return true;
return false;
}
#endif /* __XFS_BMAP_BTREE_H__ */ #endif /* __XFS_BMAP_BTREE_H__ */
...@@ -1575,14 +1575,6 @@ static inline xfs_filblks_t startblockval(xfs_fsblock_t x) ...@@ -1575,14 +1575,6 @@ static inline xfs_filblks_t startblockval(xfs_fsblock_t x)
return (xfs_filblks_t)((x) & ~STARTBLOCKMASK); return (xfs_filblks_t)((x) & ~STARTBLOCKMASK);
} }
/*
* Possible extent formats.
*/
typedef enum {
XFS_EXTFMT_NOSTATE = 0,
XFS_EXTFMT_HASSTATE
} xfs_exntfmt_t;
/* /*
* Possible extent states. * Possible extent states.
*/ */
......
...@@ -42,35 +42,6 @@ STATIC int xfs_iformat_local(xfs_inode_t *, xfs_dinode_t *, int, int); ...@@ -42,35 +42,6 @@ STATIC int xfs_iformat_local(xfs_inode_t *, xfs_dinode_t *, int, int);
STATIC int xfs_iformat_extents(xfs_inode_t *, xfs_dinode_t *, int); STATIC int xfs_iformat_extents(xfs_inode_t *, xfs_dinode_t *, int);
STATIC int xfs_iformat_btree(xfs_inode_t *, xfs_dinode_t *, int); STATIC int xfs_iformat_btree(xfs_inode_t *, xfs_dinode_t *, int);
#ifdef DEBUG
/*
* Make sure that the extents in the given memory buffer
* are valid.
*/
void
xfs_validate_extents(
xfs_ifork_t *ifp,
int nrecs,
xfs_exntfmt_t fmt)
{
xfs_bmbt_irec_t irec;
xfs_bmbt_rec_host_t rec;
int i;
for (i = 0; i < nrecs; i++) {
xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, i);
rec.l0 = get_unaligned(&ep->l0);
rec.l1 = get_unaligned(&ep->l1);
xfs_bmbt_get_all(&rec, &irec);
if (fmt == XFS_EXTFMT_NOSTATE)
ASSERT(irec.br_state == XFS_EXT_NORM);
}
}
#else /* DEBUG */
#define xfs_validate_extents(ifp, nrecs, fmt)
#endif /* DEBUG */
/* /*
* Move inode type and inode format specific information from the * Move inode type and inode format specific information from the
* on-disk inode to the in-core inode. For fifos, devs, and sockets * on-disk inode to the in-core inode. For fifos, devs, and sockets
...@@ -352,40 +323,33 @@ xfs_iformat_local( ...@@ -352,40 +323,33 @@ xfs_iformat_local(
} }
/* /*
* The file consists of a set of extents all * The file consists of a set of extents all of which fit into the on-disk
* of which fit into the on-disk inode. * inode. If there are few enough extents to fit into the if_inline_ext, then
* If there are few enough extents to fit into * copy them there. Otherwise allocate a buffer for them and copy them into it.
* the if_inline_ext, then copy them there. * Either way, set if_extents to point at the extents.
* Otherwise allocate a buffer for them and copy
* them into it. Either way, set if_extents
* to point at the extents.
*/ */
STATIC int STATIC int
xfs_iformat_extents( xfs_iformat_extents(
xfs_inode_t *ip, struct xfs_inode *ip,
xfs_dinode_t *dip, struct xfs_dinode *dip,
int whichfork) int whichfork)
{ {
xfs_bmbt_rec_t *dp; struct xfs_mount *mp = ip->i_mount;
xfs_ifork_t *ifp; struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
int nex; int nex = XFS_DFORK_NEXTENTS(dip, whichfork);
int size; int size = nex * sizeof(xfs_bmbt_rec_t);
struct xfs_bmbt_rec *dp;
int i; int i;
ifp = XFS_IFORK_PTR(ip, whichfork);
nex = XFS_DFORK_NEXTENTS(dip, whichfork);
size = nex * (uint)sizeof(xfs_bmbt_rec_t);
/* /*
* If the number of extents is unreasonable, then something * If the number of extents is unreasonable, then something is wrong and
* is wrong and we just bail out rather than crash in * we just bail out rather than crash in kmem_alloc() or memcpy() below.
* kmem_alloc() or memcpy() below.
*/ */
if (unlikely(size < 0 || size > XFS_DFORK_SIZE(dip, ip->i_mount, whichfork))) { if (unlikely(size < 0 || size > XFS_DFORK_SIZE(dip, mp, whichfork))) {
xfs_warn(ip->i_mount, "corrupt inode %Lu ((a)extents = %d).", xfs_warn(ip->i_mount, "corrupt inode %Lu ((a)extents = %d).",
(unsigned long long) ip->i_ino, nex); (unsigned long long) ip->i_ino, nex);
XFS_CORRUPTION_ERROR("xfs_iformat_extents(1)", XFS_ERRLEVEL_LOW, XFS_CORRUPTION_ERROR("xfs_iformat_extents(1)", XFS_ERRLEVEL_LOW,
ip->i_mount, dip); mp, dip);
return -EFSCORRUPTED; return -EFSCORRUPTED;
} }
...@@ -400,23 +364,18 @@ xfs_iformat_extents( ...@@ -400,23 +364,18 @@ xfs_iformat_extents(
ifp->if_bytes = size; ifp->if_bytes = size;
if (size) { if (size) {
dp = (xfs_bmbt_rec_t *) XFS_DFORK_PTR(dip, whichfork); dp = (xfs_bmbt_rec_t *) XFS_DFORK_PTR(dip, whichfork);
xfs_validate_extents(ifp, nex, XFS_EXTFMT_INODE(ip));
for (i = 0; i < nex; i++, dp++) { for (i = 0; i < nex; i++, dp++) {
xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, i); xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, i);
ep->l0 = get_unaligned_be64(&dp->l0); ep->l0 = get_unaligned_be64(&dp->l0);
ep->l1 = get_unaligned_be64(&dp->l1); ep->l1 = get_unaligned_be64(&dp->l1);
} if (!xfs_bmbt_validate_extent(mp, whichfork, ep)) {
XFS_BMAP_TRACE_EXLIST(ip, nex, whichfork);
if (whichfork != XFS_DATA_FORK ||
XFS_EXTFMT_INODE(ip) == XFS_EXTFMT_NOSTATE)
if (unlikely(xfs_check_nostate_extents(
ifp, 0, nex))) {
XFS_ERROR_REPORT("xfs_iformat_extents(2)", XFS_ERROR_REPORT("xfs_iformat_extents(2)",
XFS_ERRLEVEL_LOW, XFS_ERRLEVEL_LOW, mp);
ip->i_mount);
return -EFSCORRUPTED; return -EFSCORRUPTED;
} }
} }
XFS_BMAP_TRACE_EXLIST(ip, nex, whichfork);
}
ifp->if_flags |= XFS_IFEXTENTS; ifp->if_flags |= XFS_IFEXTENTS;
return 0; return 0;
} }
...@@ -518,7 +477,6 @@ xfs_iread_extents( ...@@ -518,7 +477,6 @@ xfs_iread_extents(
xfs_iext_destroy(ifp); xfs_iext_destroy(ifp);
return error; return error;
} }
xfs_validate_extents(ifp, nextents, XFS_EXTFMT_INODE(ip));
ifp->if_flags |= XFS_IFEXTENTS; ifp->if_flags |= XFS_IFEXTENTS;
return 0; return 0;
} }
...@@ -837,6 +795,9 @@ xfs_iextents_copy( ...@@ -837,6 +795,9 @@ xfs_iextents_copy(
copied = 0; copied = 0;
for (i = 0; i < nrecs; i++) { for (i = 0; i < nrecs; i++) {
xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, i); xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, i);
ASSERT(xfs_bmbt_validate_extent(ip->i_mount, whichfork, ep));
start_block = xfs_bmbt_get_startblock(ep); start_block = xfs_bmbt_get_startblock(ep);
if (isnullstartblock(start_block)) { if (isnullstartblock(start_block)) {
/* /*
...@@ -852,7 +813,6 @@ xfs_iextents_copy( ...@@ -852,7 +813,6 @@ xfs_iextents_copy(
copied++; copied++;
} }
ASSERT(copied != 0); ASSERT(copied != 0);
xfs_validate_extents(ifp, copied, XFS_EXTFMT_INODE(ip));
return (copied * (uint)sizeof(xfs_bmbt_rec_t)); return (copied * (uint)sizeof(xfs_bmbt_rec_t));
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment