* [PATCH 00/13] xfs: more and better verifiers
@ 2017-12-13 23:58 Darrick J. Wong
2017-12-13 23:58 ` [PATCH 01/13] xfs: refactor long-format btree header verification routines Darrick J. Wong
` (12 more replies)
0 siblings, 13 replies; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:58 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
Hi all,
This patch series combines all the random little fixes and improvements
to the verifiers that we've been talking about for the past month or so
into a single patch series!
We start by refactoring the long format btree block header verifier into
a single helper functionn and de-macroing dir block verifiers to make
them less shouty. Next, we change verifier functions to return the
approximate instruction pointer of the faulting test so that we can
report more precise fault information to dmesg/tracepoints. Then, we
move on to creating verifiers for all the inline format fork formats,
and create a helper function for _iget that can dispatch inline
verification. The ability to supply custom verifier functions will be
used by xfs_repair to deal with corrupt inodes. The last patch enables
us to (re)check the structure of incore metadata buffers to try to
detect memory corruption.
--D
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 01/13] xfs: refactor long-format btree header verification routines
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
@ 2017-12-13 23:58 ` Darrick J. Wong
2017-12-14 22:06 ` Dave Chinner
2017-12-13 23:58 ` [PATCH 02/13] xfs: remove XFS_WANT_CORRUPTED_RETURN from dir3 data verifiers Darrick J. Wong
` (11 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:58 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Create two helper functions to verify the headers of a long format
btree block. We'll use this later for the realtime rmapbt.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_bmap_btree.c | 22 ++-----------------
fs/xfs/libxfs/xfs_btree.c | 47 ++++++++++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_btree.h | 3 +++
3 files changed, 52 insertions(+), 20 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index c10aeca..862be9c 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -435,17 +435,11 @@ xfs_bmbt_verify(
switch (block->bb_magic) {
case cpu_to_be32(XFS_BMAP_CRC_MAGIC):
- if (!xfs_sb_version_hascrc(&mp->m_sb))
- return false;
- if (!uuid_equal(&block->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid))
- return false;
- if (be64_to_cpu(block->bb_u.l.bb_blkno) != bp->b_bn)
- return false;
/*
* XXX: need a better way of verifying the owner here. Right now
* just make sure there has been one set.
*/
- if (be64_to_cpu(block->bb_u.l.bb_owner) == 0)
+ if (!xfs_btree_lblock_v5hdr_verify(bp, XFS_RMAP_OWN_UNKNOWN))
return false;
/* fall through */
case cpu_to_be32(XFS_BMAP_MAGIC):
@@ -464,20 +458,8 @@ xfs_bmbt_verify(
level = be16_to_cpu(block->bb_level);
if (level > max(mp->m_bm_maxlevels[0], mp->m_bm_maxlevels[1]))
return false;
- if (be16_to_cpu(block->bb_numrecs) > mp->m_bmap_dmxr[level != 0])
- return false;
-
- /* sibling pointer verification */
- if (!block->bb_u.l.bb_leftsib ||
- (block->bb_u.l.bb_leftsib != cpu_to_be64(NULLFSBLOCK) &&
- !XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_leftsib))))
- return false;
- if (!block->bb_u.l.bb_rightsib ||
- (block->bb_u.l.bb_rightsib != cpu_to_be64(NULLFSBLOCK) &&
- !XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_rightsib))))
- return false;
- return true;
+ return xfs_btree_lblock_verify(bp, mp->m_bmap_dmxr[level != 0]);
}
static void
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index e0bdff3..72d05b1 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -4529,6 +4529,53 @@ xfs_btree_change_owner(
&bbcoi);
}
+/* Verify the v5 fields of a long-format btree block. */
+bool
+xfs_btree_lblock_v5hdr_verify(
+ struct xfs_buf *bp,
+ uint64_t owner)
+{
+ struct xfs_mount *mp = bp->b_target->bt_mount;
+ struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
+
+ if (!xfs_sb_version_hascrc(&mp->m_sb))
+ return false;
+ if (!uuid_equal(&block->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid))
+ return false;
+ if (block->bb_u.l.bb_blkno != cpu_to_be64(bp->b_bn))
+ return false;
+ if (owner != XFS_RMAP_OWN_UNKNOWN &&
+ be64_to_cpu(block->bb_u.l.bb_owner) != owner)
+ return false;
+ return true;
+}
+
+/* Verify a long-format btree block. */
+bool
+xfs_btree_lblock_verify(
+ struct xfs_buf *bp,
+ unsigned int max_recs)
+{
+ struct xfs_mount *mp = bp->b_target->bt_mount;
+ struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
+
+ /* numrecs verification */
+ if (be16_to_cpu(block->bb_numrecs) > max_recs)
+ return false;
+
+ /* sibling pointer verification */
+ if (!block->bb_u.l.bb_leftsib ||
+ (block->bb_u.l.bb_leftsib != cpu_to_be64(NULLFSBLOCK) &&
+ !XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_leftsib))))
+ return false;
+ if (!block->bb_u.l.bb_rightsib ||
+ (block->bb_u.l.bb_rightsib != cpu_to_be64(NULLFSBLOCK) &&
+ !XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_rightsib))))
+ return false;
+
+ return true;
+}
+
/**
* xfs_btree_sblock_v5hdr_verify() -- verify the v5 fields of a short-format
* btree block
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index 551a2a0..a046d0f 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -498,6 +498,9 @@ static inline int xfs_btree_get_level(struct xfs_btree_block *block)
bool xfs_btree_sblock_v5hdr_verify(struct xfs_buf *bp);
bool xfs_btree_sblock_verify(struct xfs_buf *bp, unsigned int max_recs);
+bool xfs_btree_lblock_v5hdr_verify(struct xfs_buf *bp, uint64_t owner);
+bool xfs_btree_lblock_verify(struct xfs_buf *bp, unsigned int max_recs);
+
uint xfs_btree_compute_maxlevels(struct xfs_mount *mp, uint *limits,
unsigned long len);
xfs_extlen_t xfs_btree_calc_size(struct xfs_mount *mp, uint *limits,
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 02/13] xfs: remove XFS_WANT_CORRUPTED_RETURN from dir3 data verifiers
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
2017-12-13 23:58 ` [PATCH 01/13] xfs: refactor long-format btree header verification routines Darrick J. Wong
@ 2017-12-13 23:58 ` Darrick J. Wong
2017-12-19 3:50 ` Dave Chinner
2017-12-13 23:58 ` [PATCH 03/13] xfs: have buffer verifier functions report failing address Darrick J. Wong
` (10 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:58 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Since __xfs_dir3_data_check verifies on-disk metadata, we can't have it
noisily blowing asserts and hanging the system on corrupt data coming in
off the disk. Instead, have it return a boolean like all the other
checker functions, and only have it noisily fail if we fail in debug
mode.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_dir2_block.c | 4 --
fs/xfs/libxfs/xfs_dir2_data.c | 100 +++++++++++++++++++++-------------------
fs/xfs/libxfs/xfs_dir2_priv.h | 10 +++-
3 files changed, 61 insertions(+), 53 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 43c902f..e427249 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -78,9 +78,7 @@ xfs_dir3_block_verify(
if (hdr3->magic != cpu_to_be32(XFS_DIR2_BLOCK_MAGIC))
return false;
}
- if (__xfs_dir3_data_check(NULL, bp))
- return false;
- return true;
+ return __xfs_dir3_data_check(NULL, bp);
}
static void
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index 8727a43..af15f705 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -36,9 +36,9 @@
/*
* Check the consistency of the data block.
* The input can also be a block-format directory.
- * Return 0 is the buffer is good, otherwise an error.
+ * Return true if the buffer is good.
*/
-int
+bool
__xfs_dir3_data_check(
struct xfs_inode *dp, /* incore inode pointer */
struct xfs_buf *bp) /* data block's buffer */
@@ -90,16 +90,16 @@ __xfs_dir3_data_check(
* so just ensure that the count falls somewhere inside the
* block right now.
*/
- XFS_WANT_CORRUPTED_RETURN(mp, be32_to_cpu(btp->count) <
- ((char *)btp - p) / sizeof(struct xfs_dir2_leaf_entry));
+ if (be32_to_cpu(btp->count) >=
+ ((char *)btp - p) / sizeof(struct xfs_dir2_leaf_entry))
+ return false;
break;
case cpu_to_be32(XFS_DIR3_DATA_MAGIC):
case cpu_to_be32(XFS_DIR2_DATA_MAGIC):
endp = (char *)hdr + geo->blksize;
break;
default:
- XFS_ERROR_REPORT("Bad Magic", XFS_ERRLEVEL_LOW, mp);
- return -EFSCORRUPTED;
+ return false;
}
/*
@@ -108,22 +108,25 @@ __xfs_dir3_data_check(
bf = ops->data_bestfree_p(hdr);
count = lastfree = freeseen = 0;
if (!bf[0].length) {
- XFS_WANT_CORRUPTED_RETURN(mp, !bf[0].offset);
+ if (bf[0].offset)
+ return false;
freeseen |= 1 << 0;
}
if (!bf[1].length) {
- XFS_WANT_CORRUPTED_RETURN(mp, !bf[1].offset);
+ if (bf[1].offset)
+ return false;
freeseen |= 1 << 1;
}
if (!bf[2].length) {
- XFS_WANT_CORRUPTED_RETURN(mp, !bf[2].offset);
+ if (bf[2].offset)
+ return false;
freeseen |= 1 << 2;
}
- XFS_WANT_CORRUPTED_RETURN(mp, be16_to_cpu(bf[0].length) >=
- be16_to_cpu(bf[1].length));
- XFS_WANT_CORRUPTED_RETURN(mp, be16_to_cpu(bf[1].length) >=
- be16_to_cpu(bf[2].length));
+ if (be16_to_cpu(bf[0].length) < be16_to_cpu(bf[1].length))
+ return false;
+ if (be16_to_cpu(bf[1].length) < be16_to_cpu(bf[2].length))
+ return false;
/*
* Loop over the data/unused entries.
*/
@@ -135,22 +138,23 @@ __xfs_dir3_data_check(
* doesn't need to be there.
*/
if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
- XFS_WANT_CORRUPTED_RETURN(mp, lastfree == 0);
- XFS_WANT_CORRUPTED_RETURN(mp, endp >=
- p + be16_to_cpu(dup->length));
- XFS_WANT_CORRUPTED_RETURN(mp,
- be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)) ==
- (char *)dup - (char *)hdr);
+ if (lastfree != 0)
+ return false;
+ if (endp < p + be16_to_cpu(dup->length))
+ return false;
+ if (be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)) !=
+ (char *)dup - (char *)hdr)
+ return false;
dfp = xfs_dir2_data_freefind(hdr, bf, dup);
if (dfp) {
i = (int)(dfp - bf);
- XFS_WANT_CORRUPTED_RETURN(mp,
- (freeseen & (1 << i)) == 0);
+ if ((freeseen & (1 << i)) != 0)
+ return false;
freeseen |= 1 << i;
} else {
- XFS_WANT_CORRUPTED_RETURN(mp,
- be16_to_cpu(dup->length) <=
- be16_to_cpu(bf[2].length));
+ if (be16_to_cpu(dup->length) >
+ be16_to_cpu(bf[2].length))
+ return false;
}
p += be16_to_cpu(dup->length);
lastfree = 1;
@@ -163,16 +167,17 @@ __xfs_dir3_data_check(
* The linear search is crude but this is DEBUG code.
*/
dep = (xfs_dir2_data_entry_t *)p;
- XFS_WANT_CORRUPTED_RETURN(mp, dep->namelen != 0);
- XFS_WANT_CORRUPTED_RETURN(mp,
- !xfs_dir_ino_validate(mp, be64_to_cpu(dep->inumber)));
- XFS_WANT_CORRUPTED_RETURN(mp, endp >=
- p + ops->data_entsize(dep->namelen));
- XFS_WANT_CORRUPTED_RETURN(mp,
- be16_to_cpu(*ops->data_entry_tag_p(dep)) ==
- (char *)dep - (char *)hdr);
- XFS_WANT_CORRUPTED_RETURN(mp,
- ops->data_get_ftype(dep) < XFS_DIR3_FT_MAX);
+ if (dep->namelen == 0)
+ return false;
+ if (xfs_dir_ino_validate(mp, be64_to_cpu(dep->inumber)))
+ return false;
+ if (endp < p + ops->data_entsize(dep->namelen))
+ return false;
+ if (be16_to_cpu(*ops->data_entry_tag_p(dep)) !=
+ (char *)dep - (char *)hdr)
+ return false;
+ if (ops->data_get_ftype(dep) >= XFS_DIR3_FT_MAX)
+ return false;
count++;
lastfree = 0;
if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
@@ -188,31 +193,32 @@ __xfs_dir3_data_check(
be32_to_cpu(lep[i].hashval) == hash)
break;
}
- XFS_WANT_CORRUPTED_RETURN(mp,
- i < be32_to_cpu(btp->count));
+ if (i >= be32_to_cpu(btp->count))
+ return false;
}
p += ops->data_entsize(dep->namelen);
}
/*
* Need to have seen all the entries and all the bestfree slots.
*/
- XFS_WANT_CORRUPTED_RETURN(mp, freeseen == 7);
+ if (freeseen != 7)
+ return false;
if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
hdr->magic == cpu_to_be32(XFS_DIR3_BLOCK_MAGIC)) {
for (i = stale = 0; i < be32_to_cpu(btp->count); i++) {
if (lep[i].address ==
cpu_to_be32(XFS_DIR2_NULL_DATAPTR))
stale++;
- if (i > 0)
- XFS_WANT_CORRUPTED_RETURN(mp,
- be32_to_cpu(lep[i].hashval) >=
- be32_to_cpu(lep[i - 1].hashval));
+ if (i > 0 && be32_to_cpu(lep[i].hashval) <
+ be32_to_cpu(lep[i - 1].hashval))
+ return false;
}
- XFS_WANT_CORRUPTED_RETURN(mp, count ==
- be32_to_cpu(btp->count) - be32_to_cpu(btp->stale));
- XFS_WANT_CORRUPTED_RETURN(mp, stale == be32_to_cpu(btp->stale));
+ if (count != be32_to_cpu(btp->count) - be32_to_cpu(btp->stale))
+ return false;
+ if (stale != be32_to_cpu(btp->stale))
+ return false;
}
- return 0;
+ return true;
}
static bool
@@ -235,9 +241,7 @@ xfs_dir3_data_verify(
if (hdr3->magic != cpu_to_be32(XFS_DIR2_DATA_MAGIC))
return false;
}
- if (__xfs_dir3_data_check(NULL, bp))
- return false;
- return true;
+ return __xfs_dir3_data_check(NULL, bp);
}
/*
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index 4badd26..45c68d0 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -39,12 +39,18 @@ extern int xfs_dir2_leaf_to_block(struct xfs_da_args *args,
/* xfs_dir2_data.c */
#ifdef DEBUG
-#define xfs_dir3_data_check(dp,bp) __xfs_dir3_data_check(dp, bp);
+#define xfs_dir3_data_check(dp, bp) \
+do { \
+ if (!__xfs_dir3_data_check((dp), (bp))) { \
+ XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, \
+ (bp)->b_target->bt_mount, (bp)->b_addr); \
+ } \
+} while (0)
#else
#define xfs_dir3_data_check(dp,bp)
#endif
-extern int __xfs_dir3_data_check(struct xfs_inode *dp, struct xfs_buf *bp);
+extern bool __xfs_dir3_data_check(struct xfs_inode *dp, struct xfs_buf *bp);
extern int xfs_dir3_data_read(struct xfs_trans *tp, struct xfs_inode *dp,
xfs_dablk_t bno, xfs_daddr_t mapped_bno, struct xfs_buf **bpp);
extern int xfs_dir3_data_readahead(struct xfs_inode *dp, xfs_dablk_t bno,
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 03/13] xfs: have buffer verifier functions report failing address
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
2017-12-13 23:58 ` [PATCH 01/13] xfs: refactor long-format btree header verification routines Darrick J. Wong
2017-12-13 23:58 ` [PATCH 02/13] xfs: remove XFS_WANT_CORRUPTED_RETURN from dir3 data verifiers Darrick J. Wong
@ 2017-12-13 23:58 ` Darrick J. Wong
2017-12-19 4:12 ` Dave Chinner
2017-12-13 23:58 ` [PATCH 04/13] xfs: refactor verifier callers to print address of failing check Darrick J. Wong
` (9 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:58 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Modify each function that checks the contents of a metadata buffer to
return the instruction address of the failing test so that we can report
more precise failure errors to the log.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_alloc.c | 43 ++++++++++++-----------
fs/xfs/libxfs/xfs_alloc_btree.c | 25 +++++++-------
fs/xfs/libxfs/xfs_attr_leaf.c | 20 +++++------
fs/xfs/libxfs/xfs_attr_remote.c | 36 ++++++++++----------
fs/xfs/libxfs/xfs_bmap_btree.c | 16 +++++----
fs/xfs/libxfs/xfs_btree.c | 46 +++++++++++++------------
fs/xfs/libxfs/xfs_btree.h | 11 ++++--
fs/xfs/libxfs/xfs_da_btree.c | 26 +++++++-------
fs/xfs/libxfs/xfs_dir2_block.c | 16 ++++-----
fs/xfs/libxfs/xfs_dir2_data.c | 66 ++++++++++++++++++------------------
fs/xfs/libxfs/xfs_dir2_leaf.c | 30 ++++++++--------
fs/xfs/libxfs/xfs_dir2_node.c | 36 ++++++++++----------
fs/xfs/libxfs/xfs_dir2_priv.h | 10 +++--
fs/xfs/libxfs/xfs_ialloc.c | 22 ++++++------
fs/xfs/libxfs/xfs_ialloc_btree.c | 15 ++++----
fs/xfs/libxfs/xfs_inode_buf.c | 30 ++++++++--------
fs/xfs/libxfs/xfs_inode_buf.h | 4 +-
fs/xfs/libxfs/xfs_refcount_btree.c | 19 +++++-----
fs/xfs/libxfs/xfs_rmap_btree.c | 19 +++++-----
fs/xfs/libxfs/xfs_symlink_remote.c | 22 ++++++------
20 files changed, 262 insertions(+), 250 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 82333fc..81ed3c8 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -520,7 +520,7 @@ xfs_alloc_fixup_trees(
return 0;
}
-static bool
+static xfs_failaddr_t
xfs_agfl_verify(
struct xfs_buf *bp)
{
@@ -529,9 +529,9 @@ xfs_agfl_verify(
int i;
if (!uuid_equal(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (be32_to_cpu(agfl->agfl_magicnum) != XFS_AGFL_MAGIC)
- return false;
+ return __this_address;
/*
* during growfs operations, the perag is not fully initialised,
* so we can't use it for any useful checking. growfs ensures we can't
@@ -539,16 +539,17 @@ xfs_agfl_verify(
* so we can detect and avoid this problem.
*/
if (bp->b_pag && be32_to_cpu(agfl->agfl_seqno) != bp->b_pag->pag_agno)
- return false;
+ return __this_address;
for (i = 0; i < XFS_AGFL_SIZE(mp); i++) {
if (be32_to_cpu(agfl->agfl_bno[i]) != NULLAGBLOCK &&
be32_to_cpu(agfl->agfl_bno[i]) >= mp->m_sb.sb_agblocks)
- return false;
+ return __this_address;
}
- return xfs_log_check_lsn(mp,
- be64_to_cpu(XFS_BUF_TO_AGFL(bp)->agfl_lsn));
+ if (!xfs_log_check_lsn(mp, be64_to_cpu(XFS_BUF_TO_AGFL(bp)->agfl_lsn)))
+ return __this_address;
+ return NULL;
}
static void
@@ -568,7 +569,7 @@ xfs_agfl_read_verify(
if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_agfl_verify(bp))
+ else if (xfs_agfl_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
@@ -586,7 +587,7 @@ xfs_agfl_write_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- if (!xfs_agfl_verify(bp)) {
+ if (xfs_agfl_verify(bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
@@ -2397,7 +2398,7 @@ xfs_alloc_put_freelist(
return 0;
}
-static bool
+static xfs_failaddr_t
xfs_agf_verify(
struct xfs_mount *mp,
struct xfs_buf *bp)
@@ -2406,10 +2407,10 @@ xfs_agf_verify(
if (xfs_sb_version_hascrc(&mp->m_sb)) {
if (!uuid_equal(&agf->agf_uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (!xfs_log_check_lsn(mp,
be64_to_cpu(XFS_BUF_TO_AGF(bp)->agf_lsn)))
- return false;
+ return __this_address;
}
if (!(agf->agf_magicnum == cpu_to_be32(XFS_AGF_MAGIC) &&
@@ -2418,18 +2419,18 @@ xfs_agf_verify(
be32_to_cpu(agf->agf_flfirst) < XFS_AGFL_SIZE(mp) &&
be32_to_cpu(agf->agf_fllast) < XFS_AGFL_SIZE(mp) &&
be32_to_cpu(agf->agf_flcount) <= XFS_AGFL_SIZE(mp)))
- return false;
+ return __this_address;
if (be32_to_cpu(agf->agf_levels[XFS_BTNUM_BNO]) < 1 ||
be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNT]) < 1 ||
be32_to_cpu(agf->agf_levels[XFS_BTNUM_BNO]) > XFS_BTREE_MAXLEVELS ||
be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNT]) > XFS_BTREE_MAXLEVELS)
- return false;
+ return __this_address;
if (xfs_sb_version_hasrmapbt(&mp->m_sb) &&
(be32_to_cpu(agf->agf_levels[XFS_BTNUM_RMAP]) < 1 ||
be32_to_cpu(agf->agf_levels[XFS_BTNUM_RMAP]) > XFS_BTREE_MAXLEVELS))
- return false;
+ return __this_address;
/*
* during growfs operations, the perag is not fully initialised,
@@ -2438,18 +2439,18 @@ xfs_agf_verify(
* so we can detect and avoid this problem.
*/
if (bp->b_pag && be32_to_cpu(agf->agf_seqno) != bp->b_pag->pag_agno)
- return false;
+ return __this_address;
if (xfs_sb_version_haslazysbcount(&mp->m_sb) &&
be32_to_cpu(agf->agf_btreeblks) > be32_to_cpu(agf->agf_length))
- return false;
+ return __this_address;
if (xfs_sb_version_hasreflink(&mp->m_sb) &&
(be32_to_cpu(agf->agf_refcount_level) < 1 ||
be32_to_cpu(agf->agf_refcount_level) > XFS_BTREE_MAXLEVELS))
- return false;
+ return __this_address;
- return true;;
+ return NULL;
}
@@ -2462,7 +2463,7 @@ xfs_agf_read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb) &&
!xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (XFS_TEST_ERROR(!xfs_agf_verify(mp, bp), mp,
+ else if (XFS_TEST_ERROR(xfs_agf_verify(mp, bp), mp,
XFS_ERRTAG_ALLOC_READ_AGF))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
@@ -2477,7 +2478,7 @@ xfs_agf_write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
- if (!xfs_agf_verify(mp, bp)) {
+ if (xfs_agf_verify(mp, bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index cfde0a0..153fe98 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -307,13 +307,14 @@ xfs_cntbt_diff_two_keys(
be32_to_cpu(k2->alloc.ar_startblock);
}
-static bool
+static xfs_failaddr_t
xfs_allocbt_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
struct xfs_perag *pag = bp->b_pag;
+ xfs_failaddr_t fa;
unsigned int level;
/*
@@ -331,29 +332,29 @@ xfs_allocbt_verify(
level = be16_to_cpu(block->bb_level);
switch (block->bb_magic) {
case cpu_to_be32(XFS_ABTB_CRC_MAGIC):
- if (!xfs_btree_sblock_v5hdr_verify(bp))
- return false;
+ if ((fa = xfs_btree_sblock_v5hdr_verify(bp)))
+ return fa;
/* fall through */
case cpu_to_be32(XFS_ABTB_MAGIC):
if (pag && pag->pagf_init) {
if (level >= pag->pagf_levels[XFS_BTNUM_BNOi])
- return false;
+ return __this_address;
} else if (level >= mp->m_ag_maxlevels)
- return false;
+ return __this_address;
break;
case cpu_to_be32(XFS_ABTC_CRC_MAGIC):
- if (!xfs_btree_sblock_v5hdr_verify(bp))
- return false;
+ if ((fa = xfs_btree_sblock_v5hdr_verify(bp)))
+ return fa;
/* fall through */
case cpu_to_be32(XFS_ABTC_MAGIC):
if (pag && pag->pagf_init) {
if (level >= pag->pagf_levels[XFS_BTNUM_CNTi])
- return false;
+ return __this_address;
} else if (level >= mp->m_ag_maxlevels)
- return false;
+ return __this_address;
break;
default:
- return false;
+ return __this_address;
}
return xfs_btree_sblock_verify(bp, mp->m_alloc_mxr[level != 0]);
@@ -365,7 +366,7 @@ xfs_allocbt_read_verify(
{
if (!xfs_btree_sblock_verify_crc(bp))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_allocbt_verify(bp))
+ else if (xfs_allocbt_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
@@ -378,7 +379,7 @@ static void
xfs_allocbt_write_verify(
struct xfs_buf *bp)
{
- if (!xfs_allocbt_verify(bp)) {
+ if (xfs_allocbt_verify(bp)) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 601eaa3..2c675f5 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -247,7 +247,7 @@ xfs_attr3_leaf_hdr_to_disk(
}
}
-static bool
+static xfs_failaddr_t
xfs_attr3_leaf_verify(
struct xfs_buf *bp)
{
@@ -262,17 +262,17 @@ xfs_attr3_leaf_verify(
struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
if (ichdr.magic != XFS_ATTR3_LEAF_MAGIC)
- return false;
+ return __this_address;
if (!uuid_equal(&hdr3->info.uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (be64_to_cpu(hdr3->info.blkno) != bp->b_bn)
- return false;
+ return __this_address;
if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->info.lsn)))
- return false;
+ return __this_address;
} else {
if (ichdr.magic != XFS_ATTR_LEAF_MAGIC)
- return false;
+ return __this_address;
}
/*
* In recovery there is a transient state where count == 0 is valid
@@ -280,12 +280,12 @@ xfs_attr3_leaf_verify(
* if the attr didn't fit in shortform.
*/
if (pag && pag->pagf_init && ichdr.count == 0)
- return false;
+ return __this_address;
/* XXX: need to range check rest of attr header values */
/* XXX: hash order check? */
- return true;
+ return NULL;
}
static void
@@ -296,7 +296,7 @@ xfs_attr3_leaf_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_attr3_leaf_hdr *hdr3 = bp->b_addr;
- if (!xfs_attr3_leaf_verify(bp)) {
+ if (xfs_attr3_leaf_verify(bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
@@ -326,7 +326,7 @@ xfs_attr3_leaf_read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb) &&
!xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_attr3_leaf_verify(bp))
+ else if (xfs_attr3_leaf_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index d56caf0..8647fda 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -65,7 +65,7 @@ xfs_attr3_rmt_blocks(
* does CRC, location and bounds checking, the unpacking function checks the
* attribute parameters and owner.
*/
-static bool
+static xfs_failaddr_t
xfs_attr3_rmt_hdr_ok(
void *ptr,
xfs_ino_t ino,
@@ -76,19 +76,19 @@ xfs_attr3_rmt_hdr_ok(
struct xfs_attr3_rmt_hdr *rmt = ptr;
if (bno != be64_to_cpu(rmt->rm_blkno))
- return false;
+ return __this_address;
if (offset != be32_to_cpu(rmt->rm_offset))
- return false;
+ return __this_address;
if (size != be32_to_cpu(rmt->rm_bytes))
- return false;
+ return __this_address;
if (ino != be64_to_cpu(rmt->rm_owner))
- return false;
+ return __this_address;
/* ok */
- return true;
+ return NULL;
}
-static bool
+static xfs_failaddr_t
xfs_attr3_rmt_verify(
struct xfs_mount *mp,
void *ptr,
@@ -98,22 +98,22 @@ xfs_attr3_rmt_verify(
struct xfs_attr3_rmt_hdr *rmt = ptr;
if (!xfs_sb_version_hascrc(&mp->m_sb))
- return false;
+ return __this_address;
if (rmt->rm_magic != cpu_to_be32(XFS_ATTR3_RMT_MAGIC))
- return false;
+ return __this_address;
if (!uuid_equal(&rmt->rm_uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (be64_to_cpu(rmt->rm_blkno) != bno)
- return false;
+ return __this_address;
if (be32_to_cpu(rmt->rm_bytes) > fsbsize - sizeof(*rmt))
- return false;
+ return __this_address;
if (be32_to_cpu(rmt->rm_offset) +
be32_to_cpu(rmt->rm_bytes) > XFS_XATTR_SIZE_MAX)
- return false;
+ return __this_address;
if (rmt->rm_owner == 0)
- return false;
+ return __this_address;
- return true;
+ return NULL;
}
static void
@@ -140,7 +140,7 @@ xfs_attr3_rmt_read_verify(
xfs_buf_ioerror(bp, -EFSBADCRC);
break;
}
- if (!xfs_attr3_rmt_verify(mp, ptr, blksize, bno)) {
+ if (xfs_attr3_rmt_verify(mp, ptr, blksize, bno)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
break;
}
@@ -177,7 +177,7 @@ xfs_attr3_rmt_write_verify(
while (len > 0) {
struct xfs_attr3_rmt_hdr *rmt = (struct xfs_attr3_rmt_hdr *)ptr;
- if (!xfs_attr3_rmt_verify(mp, ptr, blksize, bno)) {
+ if (xfs_attr3_rmt_verify(mp, ptr, blksize, bno)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
@@ -269,7 +269,7 @@ xfs_attr_rmtval_copyout(
byte_cnt = min(*valuelen, byte_cnt);
if (xfs_sb_version_hascrc(&mp->m_sb)) {
- if (!xfs_attr3_rmt_hdr_ok(src, ino, *offset,
+ if (xfs_attr3_rmt_hdr_ok(src, ino, *offset,
byte_cnt, bno)) {
xfs_alert(mp,
"remote attribute header mismatch bno/off/len/owner (0x%llx/0x%x/Ox%x/0x%llx)",
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index 862be9c..d44f1ad 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -425,12 +425,13 @@ xfs_bmbt_diff_two_keys(
be64_to_cpu(k2->bmbt.br_startoff);
}
-static bool
+static xfs_failaddr_t
xfs_bmbt_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
+ xfs_failaddr_t fa;
unsigned int level;
switch (block->bb_magic) {
@@ -439,13 +440,14 @@ xfs_bmbt_verify(
* XXX: need a better way of verifying the owner here. Right now
* just make sure there has been one set.
*/
- if (!xfs_btree_lblock_v5hdr_verify(bp, XFS_RMAP_OWN_UNKNOWN))
- return false;
+ if ((fa = xfs_btree_lblock_v5hdr_verify(bp,
+ XFS_RMAP_OWN_UNKNOWN)))
+ return fa;
/* fall through */
case cpu_to_be32(XFS_BMAP_MAGIC):
break;
default:
- return false;
+ return __this_address;
}
/*
@@ -457,7 +459,7 @@ xfs_bmbt_verify(
*/
level = be16_to_cpu(block->bb_level);
if (level > max(mp->m_bm_maxlevels[0], mp->m_bm_maxlevels[1]))
- return false;
+ return __this_address;
return xfs_btree_lblock_verify(bp, mp->m_bmap_dmxr[level != 0]);
}
@@ -468,7 +470,7 @@ xfs_bmbt_read_verify(
{
if (!xfs_btree_lblock_verify_crc(bp))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_bmbt_verify(bp))
+ else if (xfs_bmbt_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
@@ -481,7 +483,7 @@ static void
xfs_bmbt_write_verify(
struct xfs_buf *bp)
{
- if (!xfs_bmbt_verify(bp)) {
+ if (xfs_bmbt_verify(bp)) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 72d05b1..82e2dfc 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -329,7 +329,7 @@ xfs_btree_sblock_verify_crc(
if (xfs_sb_version_hascrc(&mp->m_sb)) {
if (!xfs_log_check_lsn(mp, be64_to_cpu(block->bb_u.s.bb_lsn)))
- return false;
+ return __this_address;
return xfs_buf_verify_cksum(bp, XFS_BTREE_SBLOCK_CRC_OFF);
}
@@ -4530,7 +4530,7 @@ xfs_btree_change_owner(
}
/* Verify the v5 fields of a long-format btree block. */
-bool
+xfs_failaddr_t
xfs_btree_lblock_v5hdr_verify(
struct xfs_buf *bp,
uint64_t owner)
@@ -4539,19 +4539,19 @@ xfs_btree_lblock_v5hdr_verify(
struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
if (!xfs_sb_version_hascrc(&mp->m_sb))
- return false;
+ return __this_address;
if (!uuid_equal(&block->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (block->bb_u.l.bb_blkno != cpu_to_be64(bp->b_bn))
- return false;
+ return __this_address;
if (owner != XFS_RMAP_OWN_UNKNOWN &&
be64_to_cpu(block->bb_u.l.bb_owner) != owner)
- return false;
- return true;
+ return __this_address;
+ return NULL;
}
/* Verify a long-format btree block. */
-bool
+xfs_failaddr_t
xfs_btree_lblock_verify(
struct xfs_buf *bp,
unsigned int max_recs)
@@ -4561,19 +4561,19 @@ xfs_btree_lblock_verify(
/* numrecs verification */
if (be16_to_cpu(block->bb_numrecs) > max_recs)
- return false;
+ return __this_address;
/* sibling pointer verification */
if (!block->bb_u.l.bb_leftsib ||
(block->bb_u.l.bb_leftsib != cpu_to_be64(NULLFSBLOCK) &&
!XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_leftsib))))
- return false;
+ return __this_address;
if (!block->bb_u.l.bb_rightsib ||
(block->bb_u.l.bb_rightsib != cpu_to_be64(NULLFSBLOCK) &&
!XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_rightsib))))
- return false;
+ return __this_address;
- return true;
+ return NULL;
}
/**
@@ -4584,7 +4584,7 @@ xfs_btree_lblock_verify(
* @max_recs: pointer to the m_*_mxr max records field in the xfs mount
* @pag_max_level: pointer to the per-ag max level field
*/
-bool
+xfs_failaddr_t
xfs_btree_sblock_v5hdr_verify(
struct xfs_buf *bp)
{
@@ -4593,14 +4593,14 @@ xfs_btree_sblock_v5hdr_verify(
struct xfs_perag *pag = bp->b_pag;
if (!xfs_sb_version_hascrc(&mp->m_sb))
- return false;
+ return __this_address;
if (!uuid_equal(&block->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (block->bb_u.s.bb_blkno != cpu_to_be64(bp->b_bn))
- return false;
+ return __this_address;
if (pag && be32_to_cpu(block->bb_u.s.bb_owner) != pag->pag_agno)
- return false;
- return true;
+ return __this_address;
+ return NULL;
}
/**
@@ -4609,7 +4609,7 @@ xfs_btree_sblock_v5hdr_verify(
* @bp: buffer containing the btree block
* @max_recs: maximum records allowed in this btree node
*/
-bool
+xfs_failaddr_t
xfs_btree_sblock_verify(
struct xfs_buf *bp,
unsigned int max_recs)
@@ -4619,19 +4619,19 @@ xfs_btree_sblock_verify(
/* numrecs verification */
if (be16_to_cpu(block->bb_numrecs) > max_recs)
- return false;
+ return __this_address;
/* sibling pointer verification */
if (!block->bb_u.s.bb_leftsib ||
(be32_to_cpu(block->bb_u.s.bb_leftsib) >= mp->m_sb.sb_agblocks &&
block->bb_u.s.bb_leftsib != cpu_to_be32(NULLAGBLOCK)))
- return false;
+ return __this_address;
if (!block->bb_u.s.bb_rightsib ||
(be32_to_cpu(block->bb_u.s.bb_rightsib) >= mp->m_sb.sb_agblocks &&
block->bb_u.s.bb_rightsib != cpu_to_be32(NULLAGBLOCK)))
- return false;
+ return __this_address;
- return true;
+ return NULL;
}
/*
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index a046d0f..ce5ca07 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -496,10 +496,13 @@ static inline int xfs_btree_get_level(struct xfs_btree_block *block)
#define XFS_BTREE_TRACE_ARGR(c, r)
#define XFS_BTREE_TRACE_CURSOR(c, t)
-bool xfs_btree_sblock_v5hdr_verify(struct xfs_buf *bp);
-bool xfs_btree_sblock_verify(struct xfs_buf *bp, unsigned int max_recs);
-bool xfs_btree_lblock_v5hdr_verify(struct xfs_buf *bp, uint64_t owner);
-bool xfs_btree_lblock_verify(struct xfs_buf *bp, unsigned int max_recs);
+xfs_failaddr_t xfs_btree_sblock_v5hdr_verify(struct xfs_buf *bp);
+xfs_failaddr_t xfs_btree_sblock_verify(struct xfs_buf *bp,
+ unsigned int max_recs);
+xfs_failaddr_t xfs_btree_lblock_v5hdr_verify(struct xfs_buf *bp,
+ uint64_t owner);
+xfs_failaddr_t xfs_btree_lblock_verify(struct xfs_buf *bp,
+ unsigned int max_recs);
uint xfs_btree_compute_maxlevels(struct xfs_mount *mp, uint *limits,
unsigned long len);
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 6516115..2d06036 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -128,7 +128,7 @@ xfs_da_state_free(xfs_da_state_t *state)
kmem_zone_free(xfs_da_state_zone, state);
}
-static bool
+static xfs_failaddr_t
xfs_da3_node_verify(
struct xfs_buf *bp)
{
@@ -145,24 +145,24 @@ xfs_da3_node_verify(
struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
if (ichdr.magic != XFS_DA3_NODE_MAGIC)
- return false;
+ return __this_address;
if (!uuid_equal(&hdr3->info.uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (be64_to_cpu(hdr3->info.blkno) != bp->b_bn)
- return false;
+ return __this_address;
if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->info.lsn)))
- return false;
+ return __this_address;
} else {
if (ichdr.magic != XFS_DA_NODE_MAGIC)
- return false;
+ return __this_address;
}
if (ichdr.level == 0)
- return false;
+ return __this_address;
if (ichdr.level > XFS_DA_NODE_MAXDEPTH)
- return false;
+ return __this_address;
if (ichdr.count == 0)
- return false;
+ return __this_address;
/*
* we don't know if the node is for and attribute or directory tree,
@@ -170,11 +170,11 @@ xfs_da3_node_verify(
*/
if (ichdr.count > mp->m_dir_geo->node_ents &&
ichdr.count > mp->m_attr_geo->node_ents)
- return false;
+ return __this_address;
/* XXX: hash order check? */
- return true;
+ return NULL;
}
static void
@@ -185,7 +185,7 @@ xfs_da3_node_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
- if (!xfs_da3_node_verify(bp)) {
+ if (xfs_da3_node_verify(bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
@@ -220,7 +220,7 @@ xfs_da3_node_read_verify(
}
/* fall through */
case XFS_DA_NODE_MAGIC:
- if (!xfs_da3_node_verify(bp)) {
+ if (xfs_da3_node_verify(bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
break;
}
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index e427249..ceae3ad 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -58,7 +58,7 @@ xfs_dir_startup(void)
xfs_dir_hash_dotdot = xfs_da_hashname((unsigned char *)"..", 2);
}
-static bool
+static xfs_failaddr_t
xfs_dir3_block_verify(
struct xfs_buf *bp)
{
@@ -67,16 +67,16 @@ xfs_dir3_block_verify(
if (xfs_sb_version_hascrc(&mp->m_sb)) {
if (hdr3->magic != cpu_to_be32(XFS_DIR3_BLOCK_MAGIC))
- return false;
+ return __this_address;
if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
- return false;
+ return __this_address;
if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
- return false;
+ return __this_address;
} else {
if (hdr3->magic != cpu_to_be32(XFS_DIR2_BLOCK_MAGIC))
- return false;
+ return __this_address;
}
return __xfs_dir3_data_check(NULL, bp);
}
@@ -90,7 +90,7 @@ xfs_dir3_block_read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb) &&
!xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_dir3_block_verify(bp))
+ else if (xfs_dir3_block_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
@@ -105,7 +105,7 @@ xfs_dir3_block_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
- if (!xfs_dir3_block_verify(bp)) {
+ if (xfs_dir3_block_verify(bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index af15f705..cd3c1ee 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -36,9 +36,9 @@
/*
* Check the consistency of the data block.
* The input can also be a block-format directory.
- * Return true if the buffer is good.
+ * Return NULL if the buffer is good, otherwise the address of the error.
*/
-bool
+xfs_failaddr_t
__xfs_dir3_data_check(
struct xfs_inode *dp, /* incore inode pointer */
struct xfs_buf *bp) /* data block's buffer */
@@ -92,14 +92,14 @@ __xfs_dir3_data_check(
*/
if (be32_to_cpu(btp->count) >=
((char *)btp - p) / sizeof(struct xfs_dir2_leaf_entry))
- return false;
+ return __this_address;
break;
case cpu_to_be32(XFS_DIR3_DATA_MAGIC):
case cpu_to_be32(XFS_DIR2_DATA_MAGIC):
endp = (char *)hdr + geo->blksize;
break;
default:
- return false;
+ return __this_address;
}
/*
@@ -109,24 +109,24 @@ __xfs_dir3_data_check(
count = lastfree = freeseen = 0;
if (!bf[0].length) {
if (bf[0].offset)
- return false;
+ return __this_address;
freeseen |= 1 << 0;
}
if (!bf[1].length) {
if (bf[1].offset)
- return false;
+ return __this_address;
freeseen |= 1 << 1;
}
if (!bf[2].length) {
if (bf[2].offset)
- return false;
+ return __this_address;
freeseen |= 1 << 2;
}
if (be16_to_cpu(bf[0].length) < be16_to_cpu(bf[1].length))
- return false;
+ return __this_address;
if (be16_to_cpu(bf[1].length) < be16_to_cpu(bf[2].length))
- return false;
+ return __this_address;
/*
* Loop over the data/unused entries.
*/
@@ -139,22 +139,22 @@ __xfs_dir3_data_check(
*/
if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
if (lastfree != 0)
- return false;
+ return __this_address;
if (endp < p + be16_to_cpu(dup->length))
- return false;
+ return __this_address;
if (be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)) !=
(char *)dup - (char *)hdr)
- return false;
+ return __this_address;
dfp = xfs_dir2_data_freefind(hdr, bf, dup);
if (dfp) {
i = (int)(dfp - bf);
if ((freeseen & (1 << i)) != 0)
- return false;
+ return __this_address;
freeseen |= 1 << i;
} else {
if (be16_to_cpu(dup->length) >
be16_to_cpu(bf[2].length))
- return false;
+ return __this_address;
}
p += be16_to_cpu(dup->length);
lastfree = 1;
@@ -168,16 +168,16 @@ __xfs_dir3_data_check(
*/
dep = (xfs_dir2_data_entry_t *)p;
if (dep->namelen == 0)
- return false;
+ return __this_address;
if (xfs_dir_ino_validate(mp, be64_to_cpu(dep->inumber)))
- return false;
+ return __this_address;
if (endp < p + ops->data_entsize(dep->namelen))
- return false;
+ return __this_address;
if (be16_to_cpu(*ops->data_entry_tag_p(dep)) !=
(char *)dep - (char *)hdr)
- return false;
+ return __this_address;
if (ops->data_get_ftype(dep) >= XFS_DIR3_FT_MAX)
- return false;
+ return __this_address;
count++;
lastfree = 0;
if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
@@ -194,7 +194,7 @@ __xfs_dir3_data_check(
break;
}
if (i >= be32_to_cpu(btp->count))
- return false;
+ return __this_address;
}
p += ops->data_entsize(dep->namelen);
}
@@ -202,7 +202,7 @@ __xfs_dir3_data_check(
* Need to have seen all the entries and all the bestfree slots.
*/
if (freeseen != 7)
- return false;
+ return __this_address;
if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
hdr->magic == cpu_to_be32(XFS_DIR3_BLOCK_MAGIC)) {
for (i = stale = 0; i < be32_to_cpu(btp->count); i++) {
@@ -211,17 +211,17 @@ __xfs_dir3_data_check(
stale++;
if (i > 0 && be32_to_cpu(lep[i].hashval) <
be32_to_cpu(lep[i - 1].hashval))
- return false;
+ return __this_address;
}
if (count != be32_to_cpu(btp->count) - be32_to_cpu(btp->stale))
- return false;
+ return __this_address;
if (stale != be32_to_cpu(btp->stale))
- return false;
+ return __this_address;
}
- return true;
+ return NULL;
}
-static bool
+static xfs_failaddr_t
xfs_dir3_data_verify(
struct xfs_buf *bp)
{
@@ -230,16 +230,16 @@ xfs_dir3_data_verify(
if (xfs_sb_version_hascrc(&mp->m_sb)) {
if (hdr3->magic != cpu_to_be32(XFS_DIR3_DATA_MAGIC))
- return false;
+ return __this_address;
if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
- return false;
+ return __this_address;
if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
- return false;
+ return __this_address;
} else {
if (hdr3->magic != cpu_to_be32(XFS_DIR2_DATA_MAGIC))
- return false;
+ return __this_address;
}
return __xfs_dir3_data_check(NULL, bp);
}
@@ -282,7 +282,7 @@ xfs_dir3_data_read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb) &&
!xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_dir3_data_verify(bp))
+ else if (xfs_dir3_data_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
@@ -297,7 +297,7 @@ xfs_dir3_data_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
- if (!xfs_dir3_data_verify(bp)) {
+ if (xfs_dir3_data_verify(bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index 27297a6..87565b6 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -73,13 +73,13 @@ xfs_dir3_leaf1_check(
} else if (leafhdr.magic != XFS_DIR2_LEAF1_MAGIC)
return false;
- return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf);
+ return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf) == NULL;
}
#else
#define xfs_dir3_leaf_check(dp, bp)
#endif
-bool
+xfs_failaddr_t
xfs_dir3_leaf_check_int(
struct xfs_mount *mp,
struct xfs_inode *dp,
@@ -114,27 +114,27 @@ xfs_dir3_leaf_check_int(
* We can deduce a value for that from di_size.
*/
if (hdr->count > ops->leaf_max_ents(geo))
- return false;
+ return __this_address;
/* Leaves and bests don't overlap in leaf format. */
if ((hdr->magic == XFS_DIR2_LEAF1_MAGIC ||
hdr->magic == XFS_DIR3_LEAF1_MAGIC) &&
(char *)&ents[hdr->count] > (char *)xfs_dir2_leaf_bests_p(ltp))
- return false;
+ return __this_address;
/* Check hash value order, count stale entries. */
for (i = stale = 0; i < hdr->count; i++) {
if (i + 1 < hdr->count) {
if (be32_to_cpu(ents[i].hashval) >
be32_to_cpu(ents[i + 1].hashval))
- return false;
+ return __this_address;
}
if (ents[i].address == cpu_to_be32(XFS_DIR2_NULL_DATAPTR))
stale++;
}
if (hdr->stale != stale)
- return false;
- return true;
+ return __this_address;
+ return NULL;
}
/*
@@ -142,7 +142,7 @@ xfs_dir3_leaf_check_int(
* kernels we don't get assertion failures in xfs_dir3_leaf_hdr_from_disk() due
* to incorrect magic numbers.
*/
-static bool
+static xfs_failaddr_t
xfs_dir3_leaf_verify(
struct xfs_buf *bp,
uint16_t magic)
@@ -160,16 +160,16 @@ xfs_dir3_leaf_verify(
: XFS_DIR3_LEAFN_MAGIC;
if (leaf3->info.hdr.magic != cpu_to_be16(magic3))
- return false;
+ return __this_address;
if (!uuid_equal(&leaf3->info.uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (be64_to_cpu(leaf3->info.blkno) != bp->b_bn)
- return false;
+ return __this_address;
if (!xfs_log_check_lsn(mp, be64_to_cpu(leaf3->info.lsn)))
- return false;
+ return __this_address;
} else {
if (leaf->hdr.info.magic != cpu_to_be16(magic))
- return false;
+ return __this_address;
}
return xfs_dir3_leaf_check_int(mp, NULL, NULL, leaf);
@@ -185,7 +185,7 @@ __read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb) &&
!xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_dir3_leaf_verify(bp, magic))
+ else if (xfs_dir3_leaf_verify(bp, magic))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
@@ -201,7 +201,7 @@ __write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_dir3_leaf_hdr *hdr3 = bp->b_addr;
- if (!xfs_dir3_leaf_verify(bp, magic)) {
+ if (xfs_dir3_leaf_verify(bp, magic)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 682e2bf..00ded0a 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -76,13 +76,13 @@ xfs_dir3_leafn_check(
} else if (leafhdr.magic != XFS_DIR2_LEAFN_MAGIC)
return false;
- return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf);
+ return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf) == NULL;
}
#else
#define xfs_dir3_leaf_check(dp, bp)
#endif
-static bool
+static xfs_failaddr_t
xfs_dir3_free_verify(
struct xfs_buf *bp)
{
@@ -93,21 +93,21 @@ xfs_dir3_free_verify(
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
if (hdr3->magic != cpu_to_be32(XFS_DIR3_FREE_MAGIC))
- return false;
+ return __this_address;
if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
- return false;
+ return __this_address;
if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
- return false;
+ return __this_address;
} else {
if (hdr->magic != cpu_to_be32(XFS_DIR2_FREE_MAGIC))
- return false;
+ return __this_address;
}
/* XXX: should bounds check the xfs_dir3_icfree_hdr here */
- return true;
+ return NULL;
}
static void
@@ -119,7 +119,7 @@ xfs_dir3_free_read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb) &&
!xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_dir3_free_verify(bp))
+ else if (xfs_dir3_free_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
@@ -134,7 +134,7 @@ xfs_dir3_free_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
- if (!xfs_dir3_free_verify(bp)) {
+ if (xfs_dir3_free_verify(bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
@@ -174,22 +174,22 @@ xfs_dir3_free_header_check(
struct xfs_dir3_free_hdr *hdr3 = bp->b_addr;
if (be32_to_cpu(hdr3->firstdb) != firstdb)
- return false;
+ return __this_address;
if (be32_to_cpu(hdr3->nvalid) > maxbests)
- return false;
+ return __this_address;
if (be32_to_cpu(hdr3->nvalid) < be32_to_cpu(hdr3->nused))
- return false;
+ return __this_address;
} else {
struct xfs_dir2_free_hdr *hdr = bp->b_addr;
if (be32_to_cpu(hdr->firstdb) != firstdb)
- return false;
+ return __this_address;
if (be32_to_cpu(hdr->nvalid) > maxbests)
- return false;
+ return __this_address;
if (be32_to_cpu(hdr->nvalid) < be32_to_cpu(hdr->nused))
- return false;
+ return __this_address;
}
- return true;
+ return NULL;
}
static int
@@ -208,7 +208,7 @@ __xfs_dir3_free_read(
return err;
/* Check things that we can't do in the verifier. */
- if (!xfs_dir3_free_header_check(dp, fbno, *bpp)) {
+ if (xfs_dir3_free_header_check(dp, fbno, *bpp)) {
xfs_buf_ioerror(*bpp, -EFSCORRUPTED);
xfs_verifier_error(*bpp);
xfs_trans_brelse(tp, *bpp);
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index 45c68d0..c441efb 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -41,7 +41,7 @@ extern int xfs_dir2_leaf_to_block(struct xfs_da_args *args,
#ifdef DEBUG
#define xfs_dir3_data_check(dp, bp) \
do { \
- if (!__xfs_dir3_data_check((dp), (bp))) { \
+ if (__xfs_dir3_data_check((dp), (bp))) { \
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, \
(bp)->b_target->bt_mount, (bp)->b_addr); \
} \
@@ -50,7 +50,8 @@ do { \
#define xfs_dir3_data_check(dp,bp)
#endif
-extern bool __xfs_dir3_data_check(struct xfs_inode *dp, struct xfs_buf *bp);
+extern xfs_failaddr_t __xfs_dir3_data_check(struct xfs_inode *dp,
+ struct xfs_buf *bp);
extern int xfs_dir3_data_read(struct xfs_trans *tp, struct xfs_inode *dp,
xfs_dablk_t bno, xfs_daddr_t mapped_bno, struct xfs_buf **bpp);
extern int xfs_dir3_data_readahead(struct xfs_inode *dp, xfs_dablk_t bno,
@@ -95,8 +96,9 @@ xfs_dir3_leaf_find_entry(struct xfs_dir3_icleaf_hdr *leafhdr,
int lowstale, int highstale, int *lfloglow, int *lfloghigh);
extern int xfs_dir2_node_to_leaf(struct xfs_da_state *state);
-extern bool xfs_dir3_leaf_check_int(struct xfs_mount *mp, struct xfs_inode *dp,
- struct xfs_dir3_icleaf_hdr *hdr, struct xfs_dir2_leaf *leaf);
+extern xfs_failaddr_t xfs_dir3_leaf_check_int(struct xfs_mount *mp,
+ struct xfs_inode *dp, struct xfs_dir3_icleaf_hdr *hdr,
+ struct xfs_dir2_leaf *leaf);
/* xfs_dir2_node.c */
extern int xfs_dir2_leaf_to_node(struct xfs_da_args *args,
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 72b6b74..0186cc0 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -2495,7 +2495,7 @@ xfs_check_agi_unlinked(
#define xfs_check_agi_unlinked(agi)
#endif
-static bool
+static xfs_failaddr_t
xfs_agi_verify(
struct xfs_buf *bp)
{
@@ -2504,28 +2504,28 @@ xfs_agi_verify(
if (xfs_sb_version_hascrc(&mp->m_sb)) {
if (!uuid_equal(&agi->agi_uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (!xfs_log_check_lsn(mp,
be64_to_cpu(XFS_BUF_TO_AGI(bp)->agi_lsn)))
- return false;
+ return __this_address;
}
/*
* Validate the magic number of the agi block.
*/
if (agi->agi_magicnum != cpu_to_be32(XFS_AGI_MAGIC))
- return false;
+ return __this_address;
if (!XFS_AGI_GOOD_VERSION(be32_to_cpu(agi->agi_versionnum)))
- return false;
+ return __this_address;
if (be32_to_cpu(agi->agi_level) < 1 ||
be32_to_cpu(agi->agi_level) > XFS_BTREE_MAXLEVELS)
- return false;
+ return __this_address;
if (xfs_sb_version_hasfinobt(&mp->m_sb) &&
(be32_to_cpu(agi->agi_free_level) < 1 ||
be32_to_cpu(agi->agi_free_level) > XFS_BTREE_MAXLEVELS))
- return false;
+ return __this_address;
/*
* during growfs operations, the perag is not fully initialised,
@@ -2534,10 +2534,10 @@ xfs_agi_verify(
* so we can detect and avoid this problem.
*/
if (bp->b_pag && be32_to_cpu(agi->agi_seqno) != bp->b_pag->pag_agno)
- return false;
+ return __this_address;
xfs_check_agi_unlinked(agi);
- return true;
+ return NULL;
}
static void
@@ -2549,7 +2549,7 @@ xfs_agi_read_verify(
if (xfs_sb_version_hascrc(&mp->m_sb) &&
!xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (XFS_TEST_ERROR(!xfs_agi_verify(bp), mp,
+ else if (XFS_TEST_ERROR(xfs_agi_verify(bp), mp,
XFS_ERRTAG_IALLOC_READ_AGI))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
@@ -2564,7 +2564,7 @@ xfs_agi_write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
- if (!xfs_agi_verify(bp)) {
+ if (xfs_agi_verify(bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index 317caba..d9267d8 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -250,12 +250,13 @@ xfs_inobt_diff_two_keys(
be32_to_cpu(k2->inobt.ir_startino);
}
-static int
+static xfs_failaddr_t
xfs_inobt_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
+ xfs_failaddr_t fa;
unsigned int level;
/*
@@ -271,20 +272,20 @@ xfs_inobt_verify(
switch (block->bb_magic) {
case cpu_to_be32(XFS_IBT_CRC_MAGIC):
case cpu_to_be32(XFS_FIBT_CRC_MAGIC):
- if (!xfs_btree_sblock_v5hdr_verify(bp))
- return false;
+ if ((fa = xfs_btree_sblock_v5hdr_verify(bp)))
+ return fa;
/* fall through */
case cpu_to_be32(XFS_IBT_MAGIC):
case cpu_to_be32(XFS_FIBT_MAGIC):
break;
default:
- return 0;
+ return NULL;
}
/* level verification */
level = be16_to_cpu(block->bb_level);
if (level >= mp->m_in_maxlevels)
- return false;
+ return __this_address;
return xfs_btree_sblock_verify(bp, mp->m_inobt_mxr[level != 0]);
}
@@ -295,7 +296,7 @@ xfs_inobt_read_verify(
{
if (!xfs_btree_sblock_verify_crc(bp))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_inobt_verify(bp))
+ else if (xfs_inobt_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
@@ -308,7 +309,7 @@ static void
xfs_inobt_write_verify(
struct xfs_buf *bp)
{
- if (!xfs_inobt_verify(bp)) {
+ if (xfs_inobt_verify(bp)) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 6b79890..145c47d 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -381,7 +381,7 @@ xfs_log_dinode_to_disk(
}
}
-bool
+xfs_failaddr_t
xfs_dinode_verify(
struct xfs_mount *mp,
xfs_ino_t ino,
@@ -392,33 +392,33 @@ xfs_dinode_verify(
uint64_t flags2;
if (dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC))
- return false;
+ return __this_address;
/* don't allow invalid i_size */
if (be64_to_cpu(dip->di_size) & (1ULL << 63))
- return false;
+ return __this_address;
mode = be16_to_cpu(dip->di_mode);
if (mode && xfs_mode_to_ftype(mode) == XFS_DIR3_FT_UNKNOWN)
- return false;
+ return __this_address;
/* No zero-length symlinks/dirs. */
if ((S_ISLNK(mode) || S_ISDIR(mode)) && dip->di_size == 0)
- return false;
+ return __this_address;
/* only version 3 or greater inodes are extensively verified here */
if (dip->di_version < 3)
- return true;
+ return NULL;
if (!xfs_sb_version_hascrc(&mp->m_sb))
- return false;
+ return __this_address;
if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
XFS_DINODE_CRC_OFF))
- return false;
+ return __this_address;
if (be64_to_cpu(dip->di_ino) != ino)
- return false;
+ return __this_address;
if (!uuid_equal(&dip->di_uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
flags = be16_to_cpu(dip->di_flags);
flags2 = be64_to_cpu(dip->di_flags2);
@@ -426,17 +426,17 @@ xfs_dinode_verify(
/* don't allow reflink/cowextsize if we don't have reflink */
if ((flags2 & (XFS_DIFLAG2_REFLINK | XFS_DIFLAG2_COWEXTSIZE)) &&
!xfs_sb_version_hasreflink(&mp->m_sb))
- return false;
+ return __this_address;
/* don't let reflink and realtime mix */
if ((flags2 & XFS_DIFLAG2_REFLINK) && (flags & XFS_DIFLAG_REALTIME))
- return false;
+ return __this_address;
/* don't let reflink and dax mix */
if ((flags2 & XFS_DIFLAG2_REFLINK) && (flags2 & XFS_DIFLAG2_DAX))
- return false;
+ return __this_address;
- return true;
+ return NULL;
}
void
@@ -507,7 +507,7 @@ xfs_iread(
return error;
/* even unallocated inodes are verified */
- if (!xfs_dinode_verify(mp, ip->i_ino, dip)) {
+ if (xfs_dinode_verify(mp, ip->i_ino, dip)) {
xfs_alert(mp, "%s: validation failed for inode %lld",
__func__, ip->i_ino);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
index a9c97a3..2317511 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.h
+++ b/fs/xfs/libxfs/xfs_inode_buf.h
@@ -82,7 +82,7 @@ void xfs_inobp_check(struct xfs_mount *, struct xfs_buf *);
#define xfs_inobp_check(mp, bp)
#endif /* DEBUG */
-bool xfs_dinode_verify(struct xfs_mount *mp, xfs_ino_t ino,
- struct xfs_dinode *dip);
+void *xfs_dinode_verify(struct xfs_mount *mp, xfs_ino_t ino,
+ struct xfs_dinode *dip);
#endif /* __XFS_INODE_BUF_H__ */
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 3c59dd3..66ecdb91 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -223,29 +223,30 @@ xfs_refcountbt_diff_two_keys(
be32_to_cpu(k2->refc.rc_startblock);
}
-STATIC bool
+STATIC xfs_failaddr_t
xfs_refcountbt_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
struct xfs_perag *pag = bp->b_pag;
+ xfs_failaddr_t fa;
unsigned int level;
if (block->bb_magic != cpu_to_be32(XFS_REFC_CRC_MAGIC))
- return false;
+ return __this_address;
if (!xfs_sb_version_hasreflink(&mp->m_sb))
- return false;
- if (!xfs_btree_sblock_v5hdr_verify(bp))
- return false;
+ return __this_address;
+ if ((fa = xfs_btree_sblock_v5hdr_verify(bp)))
+ return fa;
level = be16_to_cpu(block->bb_level);
if (pag && pag->pagf_init) {
if (level >= pag->pagf_refcount_level)
- return false;
+ return __this_address;
} else if (level >= mp->m_refc_maxlevels)
- return false;
+ return __this_address;
return xfs_btree_sblock_verify(bp, mp->m_refc_mxr[level != 0]);
}
@@ -256,7 +257,7 @@ xfs_refcountbt_read_verify(
{
if (!xfs_btree_sblock_verify_crc(bp))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_refcountbt_verify(bp))
+ else if (xfs_refcountbt_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
@@ -269,7 +270,7 @@ STATIC void
xfs_refcountbt_write_verify(
struct xfs_buf *bp)
{
- if (!xfs_refcountbt_verify(bp)) {
+ if (xfs_refcountbt_verify(bp)) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index 9d9c919..506482f 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -303,13 +303,14 @@ xfs_rmapbt_diff_two_keys(
return 0;
}
-static bool
+static xfs_failaddr_t
xfs_rmapbt_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
struct xfs_perag *pag = bp->b_pag;
+ xfs_failaddr_t fa;
unsigned int level;
/*
@@ -325,19 +326,19 @@ xfs_rmapbt_verify(
* in this case.
*/
if (block->bb_magic != cpu_to_be32(XFS_RMAP_CRC_MAGIC))
- return false;
+ return __this_address;
if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
- return false;
- if (!xfs_btree_sblock_v5hdr_verify(bp))
- return false;
+ return __this_address;
+ if ((fa = xfs_btree_sblock_v5hdr_verify(bp)))
+ return fa;
level = be16_to_cpu(block->bb_level);
if (pag && pag->pagf_init) {
if (level >= pag->pagf_levels[XFS_BTNUM_RMAPi])
- return false;
+ return __this_address;
} else if (level >= mp->m_rmap_maxlevels)
- return false;
+ return __this_address;
return xfs_btree_sblock_verify(bp, mp->m_rmap_mxr[level != 0]);
}
@@ -348,7 +349,7 @@ xfs_rmapbt_read_verify(
{
if (!xfs_btree_sblock_verify_crc(bp))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_rmapbt_verify(bp))
+ else if (xfs_rmapbt_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
@@ -361,7 +362,7 @@ static void
xfs_rmapbt_write_verify(
struct xfs_buf *bp)
{
- if (!xfs_rmapbt_verify(bp)) {
+ if (xfs_rmapbt_verify(bp)) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
index c484877..7ac900b 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.c
+++ b/fs/xfs/libxfs/xfs_symlink_remote.c
@@ -98,7 +98,7 @@ xfs_symlink_hdr_ok(
return true;
}
-static bool
+static xfs_failaddr_t
xfs_symlink_verify(
struct xfs_buf *bp)
{
@@ -106,22 +106,22 @@ xfs_symlink_verify(
struct xfs_dsymlink_hdr *dsl = bp->b_addr;
if (!xfs_sb_version_hascrc(&mp->m_sb))
- return false;
+ return __this_address;
if (dsl->sl_magic != cpu_to_be32(XFS_SYMLINK_MAGIC))
- return false;
+ return __this_address;
if (!uuid_equal(&dsl->sl_uuid, &mp->m_sb.sb_meta_uuid))
- return false;
+ return __this_address;
if (bp->b_bn != be64_to_cpu(dsl->sl_blkno))
- return false;
+ return __this_address;
if (be32_to_cpu(dsl->sl_offset) +
be32_to_cpu(dsl->sl_bytes) >= XFS_SYMLINK_MAXLEN)
- return false;
+ return __this_address;
if (dsl->sl_owner == 0)
- return false;
+ return __this_address;
if (!xfs_log_check_lsn(mp, be64_to_cpu(dsl->sl_lsn)))
- return false;
+ return __this_address;
- return true;
+ return NULL;
}
static void
@@ -136,7 +136,7 @@ xfs_symlink_read_verify(
if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF))
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (!xfs_symlink_verify(bp))
+ else if (xfs_symlink_verify(bp))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
@@ -154,7 +154,7 @@ xfs_symlink_write_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- if (!xfs_symlink_verify(bp)) {
+ if (xfs_symlink_verify(bp)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp);
return;
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 04/13] xfs: refactor verifier callers to print address of failing check
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (2 preceding siblings ...)
2017-12-13 23:58 ` [PATCH 03/13] xfs: have buffer verifier functions report failing address Darrick J. Wong
@ 2017-12-13 23:58 ` Darrick J. Wong
2017-12-14 22:03 ` Dave Chinner
2017-12-13 23:58 ` [PATCH 05/13] xfs: verify dinode header first Darrick J. Wong
` (8 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:58 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Refactor the callers of verifiers to print the instruction address of a
failing check.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_alloc.c | 28 +++++++++++++++++-----------
fs/xfs/libxfs/xfs_alloc_btree.c | 15 ++++++++++-----
fs/xfs/libxfs/xfs_attr_leaf.c | 13 ++++++++-----
fs/xfs/libxfs/xfs_attr_remote.c | 13 ++++++++-----
fs/xfs/libxfs/xfs_bmap_btree.c | 15 ++++++++++-----
fs/xfs/libxfs/xfs_da_btree.c | 12 ++++++++----
fs/xfs/libxfs/xfs_dir2_block.c | 13 ++++++++-----
fs/xfs/libxfs/xfs_dir2_data.c | 17 ++++++++++-------
fs/xfs/libxfs/xfs_dir2_leaf.c | 13 ++++++++-----
fs/xfs/libxfs/xfs_dir2_node.c | 20 ++++++++++++--------
fs/xfs/libxfs/xfs_dquot_buf.c | 4 ++--
fs/xfs/libxfs/xfs_ialloc.c | 15 +++++++++------
fs/xfs/libxfs/xfs_ialloc_btree.c | 15 ++++++++++-----
fs/xfs/libxfs/xfs_inode_buf.c | 24 +++++++++++++-----------
fs/xfs/libxfs/xfs_refcount_btree.c | 15 ++++++++++-----
fs/xfs/libxfs/xfs_rmap_btree.c | 15 ++++++++++-----
fs/xfs/libxfs/xfs_sb.c | 4 ++--
fs/xfs/libxfs/xfs_symlink_remote.c | 13 ++++++++-----
fs/xfs/xfs_error.c | 6 ++++--
fs/xfs/xfs_error.h | 2 +-
20 files changed, 168 insertions(+), 104 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 81ed3c8..d600dcb 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -557,6 +557,7 @@ xfs_agfl_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
/*
* There is no verification of non-crc AGFLs because mkfs does not
@@ -567,13 +568,14 @@ xfs_agfl_read_verify(
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF))
+ if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_agfl_verify(bp))
+ } else if ((fa = xfs_agfl_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
static void
@@ -582,14 +584,15 @@ xfs_agfl_write_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
+ xfs_failaddr_t fa;
/* no verification of non-crc AGFLs */
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- if (xfs_agfl_verify(bp)) {
+ if ((fa = xfs_agfl_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
@@ -2459,16 +2462,18 @@ xfs_agf_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
if (xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF))
+ !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (XFS_TEST_ERROR(xfs_agf_verify(mp, bp), mp,
- XFS_ERRTAG_ALLOC_READ_AGF))
+ } else if (XFS_TEST_ERROR((fa = xfs_agf_verify(mp, bp)), mp,
+ XFS_ERRTAG_ALLOC_READ_AGF))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
static void
@@ -2477,10 +2482,11 @@ xfs_agf_write_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
+ xfs_failaddr_t fa;
- if (xfs_agf_verify(mp, bp)) {
+ if ((fa = xfs_agf_verify(mp, bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index 153fe98..a646fc4 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -364,14 +364,17 @@ static void
xfs_allocbt_read_verify(
struct xfs_buf *bp)
{
- if (!xfs_btree_sblock_verify_crc(bp))
+ xfs_failaddr_t fa;
+
+ if (!xfs_btree_sblock_verify_crc(bp)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_allocbt_verify(bp))
+ } else if ((fa = xfs_allocbt_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
}
@@ -379,10 +382,12 @@ static void
xfs_allocbt_write_verify(
struct xfs_buf *bp)
{
- if (xfs_allocbt_verify(bp)) {
+ xfs_failaddr_t fa;
+
+ if ((fa = xfs_allocbt_verify(bp))) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
xfs_btree_sblock_calc_crc(bp);
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 2c675f5..1274872 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -295,10 +295,11 @@ xfs_attr3_leaf_write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_attr3_leaf_hdr *hdr3 = bp->b_addr;
+ xfs_failaddr_t fa;
- if (xfs_attr3_leaf_verify(bp)) {
+ if ((fa = xfs_attr3_leaf_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
@@ -322,15 +323,17 @@ xfs_attr3_leaf_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
if (xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF))
+ !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_attr3_leaf_verify(bp))
+ } else if ((fa = xfs_attr3_leaf_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
const struct xfs_buf_ops xfs_attr3_leaf_buf_ops = {
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 8647fda..d4d2902 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -122,6 +122,7 @@ xfs_attr3_rmt_read_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
char *ptr;
+ xfs_failaddr_t fa;
int len;
xfs_daddr_t bno;
int blksize = mp->m_attr_geo->blksize;
@@ -137,10 +138,11 @@ xfs_attr3_rmt_read_verify(
while (len > 0) {
if (!xfs_verify_cksum(ptr, blksize, XFS_ATTR3_RMT_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
break;
}
- if (xfs_attr3_rmt_verify(mp, ptr, blksize, bno)) {
+ if ((fa = xfs_attr3_rmt_verify(mp, ptr, blksize, bno))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
break;
}
@@ -150,7 +152,7 @@ xfs_attr3_rmt_read_verify(
}
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
else
ASSERT(len == 0);
}
@@ -160,6 +162,7 @@ xfs_attr3_rmt_write_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
int blksize = mp->m_attr_geo->blksize;
char *ptr;
int len;
@@ -177,9 +180,9 @@ xfs_attr3_rmt_write_verify(
while (len > 0) {
struct xfs_attr3_rmt_hdr *rmt = (struct xfs_attr3_rmt_hdr *)ptr;
- if (xfs_attr3_rmt_verify(mp, ptr, blksize, bno)) {
+ if ((fa = xfs_attr3_rmt_verify(mp, ptr, blksize, bno))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
@@ -189,7 +192,7 @@ xfs_attr3_rmt_write_verify(
*/
if (rmt->rm_lsn != cpu_to_be64(NULLCOMMITLSN)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, __this_address);
return;
}
xfs_update_cksum(ptr, blksize, XFS_ATTR3_RMT_CRC_OFF);
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index d44f1ad..c1ddb4c 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -468,14 +468,17 @@ static void
xfs_bmbt_read_verify(
struct xfs_buf *bp)
{
- if (!xfs_btree_lblock_verify_crc(bp))
+ xfs_failaddr_t fa;
+
+ if (!xfs_btree_lblock_verify_crc(bp)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_bmbt_verify(bp))
+ } else if ((fa = xfs_bmbt_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
}
@@ -483,10 +486,12 @@ static void
xfs_bmbt_write_verify(
struct xfs_buf *bp)
{
- if (xfs_bmbt_verify(bp)) {
+ xfs_failaddr_t fa;
+
+ if ((fa = xfs_bmbt_verify(bp))) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
xfs_btree_lblock_calc_crc(bp);
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 2d06036..6d6a9ef 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -184,10 +184,11 @@ xfs_da3_node_write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
+ xfs_failaddr_t fa;
- if (xfs_da3_node_verify(bp)) {
+ if ((fa = xfs_da3_node_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
@@ -211,16 +212,18 @@ xfs_da3_node_read_verify(
struct xfs_buf *bp)
{
struct xfs_da_blkinfo *info = bp->b_addr;
+ xfs_failaddr_t fa;
switch (be16_to_cpu(info->magic)) {
case XFS_DA3_NODE_MAGIC:
if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
break;
}
/* fall through */
case XFS_DA_NODE_MAGIC:
- if (xfs_da3_node_verify(bp)) {
+ if ((fa = xfs_da3_node_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
break;
}
@@ -236,12 +239,13 @@ xfs_da3_node_read_verify(
bp->b_ops->verify_read(bp);
return;
default:
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSCORRUPTED);
break;
}
/* corrupt block */
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
const struct xfs_buf_ops xfs_da3_node_buf_ops = {
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index ceae3ad..7a18777 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -86,15 +86,17 @@ xfs_dir3_block_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
if (xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_dir3_block_verify(bp))
+ } else if ((fa = xfs_dir3_block_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
static void
@@ -104,10 +106,11 @@ xfs_dir3_block_write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
+ xfs_failaddr_t fa;
- if (xfs_dir3_block_verify(bp)) {
+ if ((fa = xfs_dir3_block_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index cd3c1ee..f31bfd9 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -268,7 +268,7 @@ xfs_dir3_data_reada_verify(
return;
default:
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, __this_address);
break;
}
}
@@ -278,15 +278,17 @@ xfs_dir3_data_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
if (xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
- xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_dir3_data_verify(bp))
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) {
+ fa = __this_address;
+ xfs_buf_ioerror(bp, -EFSBADCRC);
+ } else if ((fa = xfs_dir3_data_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
static void
@@ -296,10 +298,11 @@ xfs_dir3_data_write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
+ xfs_failaddr_t fa;
- if (xfs_dir3_data_verify(bp)) {
+ if ((fa = xfs_dir3_data_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index 87565b6..e3fad8d 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -181,15 +181,17 @@ __read_verify(
uint16_t magic)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
if (xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF))
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_dir3_leaf_verify(bp, magic))
+ } else if ((fa = xfs_dir3_leaf_verify(bp, magic)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
static void
@@ -200,10 +202,11 @@ __write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_dir3_leaf_hdr *hdr3 = bp->b_addr;
+ xfs_failaddr_t fa;
- if (xfs_dir3_leaf_verify(bp, magic)) {
+ if ((fa = xfs_dir3_leaf_verify(bp, magic))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 00ded0a..8a6a2e1 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -115,15 +115,17 @@ xfs_dir3_free_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
if (xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF))
+ !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_dir3_free_verify(bp))
+ } else if ((fa = xfs_dir3_free_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
static void
@@ -133,10 +135,11 @@ xfs_dir3_free_write_verify(
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
+ xfs_failaddr_t fa;
- if (xfs_dir3_free_verify(bp)) {
+ if ((fa = xfs_dir3_free_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
@@ -156,7 +159,7 @@ const struct xfs_buf_ops xfs_dir3_free_buf_ops = {
};
/* Everything ok in the free block header? */
-static bool
+static xfs_failaddr_t
xfs_dir3_free_header_check(
struct xfs_inode *dp,
xfs_dablk_t fbno,
@@ -200,6 +203,7 @@ __xfs_dir3_free_read(
xfs_daddr_t mappedbno,
struct xfs_buf **bpp)
{
+ xfs_failaddr_t fa;
int err;
err = xfs_da_read_buf(tp, dp, fbno, mappedbno, bpp,
@@ -208,9 +212,9 @@ __xfs_dir3_free_read(
return err;
/* Check things that we can't do in the verifier. */
- if (xfs_dir3_free_header_check(dp, fbno, *bpp)) {
+ if ((fa = xfs_dir3_free_header_check(dp, fbno, *bpp))) {
xfs_buf_ioerror(*bpp, -EFSCORRUPTED);
- xfs_verifier_error(*bpp);
+ xfs_verifier_error(*bpp, fa);
xfs_trans_brelse(tp, *bpp);
return -EFSCORRUPTED;
}
diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 747085b..5561011 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -254,7 +254,7 @@ xfs_dquot_buf_read_verify(
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, __this_address);
}
/*
@@ -289,7 +289,7 @@ xfs_dquot_buf_write_verify(
if (!xfs_dquot_buf_verify(mp, bp, XFS_QMOPT_DOWARN)) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, __this_address);
return;
}
}
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 0186cc0..a616212 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -2545,16 +2545,18 @@ xfs_agi_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
if (xfs_sb_version_hascrc(&mp->m_sb) &&
- !xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF))
+ !xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (XFS_TEST_ERROR(xfs_agi_verify(bp), mp,
- XFS_ERRTAG_IALLOC_READ_AGI))
+ } else if (XFS_TEST_ERROR((fa = xfs_agi_verify(bp)), mp,
+ XFS_ERRTAG_IALLOC_READ_AGI))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
static void
@@ -2563,10 +2565,11 @@ xfs_agi_write_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
+ xfs_failaddr_t fa;
- if (xfs_agi_verify(bp)) {
+ if ((fa = xfs_agi_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index d9267d8..032613a 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -294,14 +294,17 @@ static void
xfs_inobt_read_verify(
struct xfs_buf *bp)
{
- if (!xfs_btree_sblock_verify_crc(bp))
+ xfs_failaddr_t fa;
+
+ if (!xfs_btree_sblock_verify_crc(bp)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_inobt_verify(bp))
+ } else if ((fa = xfs_inobt_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
}
@@ -309,10 +312,12 @@ static void
xfs_inobt_write_verify(
struct xfs_buf *bp)
{
- if (xfs_inobt_verify(bp)) {
+ xfs_failaddr_t fa;
+
+ if ((fa = xfs_inobt_verify(bp))) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
xfs_btree_sblock_calc_crc(bp);
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 145c47d..a965448 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -114,7 +114,7 @@ xfs_inode_buf_verify(
}
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, __this_address);
#ifdef DEBUG
xfs_alert(mp,
"bad inode magic/vsn daddr %lld #%d (magic=%x)",
@@ -469,14 +469,15 @@ xfs_dinode_calc_crc(
*/
int
xfs_iread(
- xfs_mount_t *mp,
- xfs_trans_t *tp,
- xfs_inode_t *ip,
- uint iget_flags)
+ struct xfs_mount *mp,
+ struct xfs_trans *tp,
+ struct xfs_inode *ip,
+ uint iget_flags)
{
- xfs_buf_t *bp;
- xfs_dinode_t *dip;
- int error;
+ struct xfs_buf *bp;
+ struct xfs_dinode *dip;
+ xfs_failaddr_t fa;
+ int error;
/*
* Fill in the location information in the in-core inode.
@@ -507,9 +508,10 @@ xfs_iread(
return error;
/* even unallocated inodes are verified */
- if (xfs_dinode_verify(mp, ip->i_ino, dip)) {
- xfs_alert(mp, "%s: validation failed for inode %lld",
- __func__, ip->i_ino);
+ fa = xfs_dinode_verify(mp, ip->i_ino, dip);
+ if (fa) {
+ xfs_alert(mp, "%s: validation failed for inode %lld at %pF",
+ __func__, ip->i_ino, fa);
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, dip);
error = -EFSCORRUPTED;
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 66ecdb91..68d4021 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -255,14 +255,17 @@ STATIC void
xfs_refcountbt_read_verify(
struct xfs_buf *bp)
{
- if (!xfs_btree_sblock_verify_crc(bp))
+ xfs_failaddr_t fa;
+
+ if (!xfs_btree_sblock_verify_crc(bp)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_refcountbt_verify(bp))
+ } else if ((fa = xfs_refcountbt_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
}
@@ -270,10 +273,12 @@ STATIC void
xfs_refcountbt_write_verify(
struct xfs_buf *bp)
{
- if (xfs_refcountbt_verify(bp)) {
+ xfs_failaddr_t fa;
+
+ if ((fa = xfs_refcountbt_verify(bp))) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
xfs_btree_sblock_calc_crc(bp);
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index 506482f..ab358b9 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -347,14 +347,17 @@ static void
xfs_rmapbt_read_verify(
struct xfs_buf *bp)
{
- if (!xfs_btree_sblock_verify_crc(bp))
+ xfs_failaddr_t fa;
+
+ if (!xfs_btree_sblock_verify_crc(bp)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_rmapbt_verify(bp))
+ } else if ((fa = xfs_rmapbt_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
}
@@ -362,10 +365,12 @@ static void
xfs_rmapbt_write_verify(
struct xfs_buf *bp)
{
- if (xfs_rmapbt_verify(bp)) {
+ xfs_failaddr_t fa;
+
+ if ((fa = xfs_rmapbt_verify(bp))) {
trace_xfs_btree_corrupt(bp, _RET_IP_);
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
xfs_btree_sblock_calc_crc(bp);
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 9b5aae2..9b49640 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -643,7 +643,7 @@ xfs_sb_read_verify(
if (error) {
xfs_buf_ioerror(bp, error);
if (error == -EFSCORRUPTED || error == -EFSBADCRC)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, __this_address);
}
}
@@ -679,7 +679,7 @@ xfs_sb_write_verify(
error = xfs_sb_verify(bp, false);
if (error) {
xfs_buf_ioerror(bp, error);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, __this_address);
return;
}
diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
index 7ac900b..b6ef6cb 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.c
+++ b/fs/xfs/libxfs/xfs_symlink_remote.c
@@ -129,18 +129,20 @@ xfs_symlink_read_verify(
struct xfs_buf *bp)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
+ xfs_failaddr_t fa;
/* no verification of non-crc buffers */
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF))
+ if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF)) {
+ fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- else if (xfs_symlink_verify(bp))
+ } else if ((fa = xfs_symlink_verify(bp)))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (bp->b_error)
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
}
static void
@@ -149,14 +151,15 @@ xfs_symlink_write_verify(
{
struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_buf_log_item *bip = bp->b_fspriv;
+ xfs_failaddr_t fa;
/* no verification of non-crc buffers */
if (!xfs_sb_version_hascrc(&mp->m_sb))
return;
- if (xfs_symlink_verify(bp)) {
+ if ((fa = xfs_symlink_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
- xfs_verifier_error(bp);
+ xfs_verifier_error(bp, fa);
return;
}
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 4c9f35d..0bbbf0b 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -347,13 +347,15 @@ xfs_corruption_error(
*/
void
xfs_verifier_error(
- struct xfs_buf *bp)
+ struct xfs_buf *bp,
+ xfs_failaddr_t fa)
{
struct xfs_mount *mp = bp->b_target->bt_mount;
xfs_alert(mp, "Metadata %s detected at %pS, %s block 0x%llx",
bp->b_error == -EFSBADCRC ? "CRC error" : "corruption",
- __return_address, bp->b_ops->name, bp->b_bn);
+ fa ? fa : __return_address, bp->b_ops->name,
+ bp->b_bn);
xfs_alert(mp, "Unmount and run xfs_repair");
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index ea816c1..c13b3b4 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -25,7 +25,7 @@ extern void xfs_error_report(const char *tag, int level, struct xfs_mount *mp,
extern void xfs_corruption_error(const char *tag, int level,
struct xfs_mount *mp, void *p, const char *filename,
int linenum, void *ra);
-extern void xfs_verifier_error(struct xfs_buf *bp);
+extern void xfs_verifier_error(struct xfs_buf *bp, xfs_failaddr_t fa);
#define XFS_ERROR_REPORT(e, lvl, mp) \
xfs_error_report(e, lvl, mp, __FILE__, __LINE__, __return_address)
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 05/13] xfs: verify dinode header first
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (3 preceding siblings ...)
2017-12-13 23:58 ` [PATCH 04/13] xfs: refactor verifier callers to print address of failing check Darrick J. Wong
@ 2017-12-13 23:58 ` Darrick J. Wong
2017-12-19 4:13 ` Dave Chinner
2017-12-13 23:58 ` [PATCH 06/13] xfs: move inode fork verifiers to xfs_dinode_verify Darrick J. Wong
` (7 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:58 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Move the v3 inode integrity information (crc, owner, metauuid) before we
look at anything else in the inode so that we don't waste time on a torn
write or a totally garbled block. This makes xfs_dinode_verify more
consistent with the other verifiers.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_inode_buf.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index a965448..0e4c720 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -394,6 +394,19 @@ xfs_dinode_verify(
if (dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC))
return __this_address;
+ /* Verify v3 integrity information first */
+ if (dip->di_version >= 3) {
+ if (!xfs_sb_version_hascrc(&mp->m_sb))
+ return __this_address;
+ if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
+ XFS_DINODE_CRC_OFF))
+ return __this_address;
+ if (be64_to_cpu(dip->di_ino) != ino)
+ return __this_address;
+ if (!uuid_equal(&dip->di_uuid, &mp->m_sb.sb_meta_uuid))
+ return __this_address;
+ }
+
/* don't allow invalid i_size */
if (be64_to_cpu(dip->di_size) & (1ULL << 63))
return __this_address;
@@ -410,16 +423,6 @@ xfs_dinode_verify(
if (dip->di_version < 3)
return NULL;
- if (!xfs_sb_version_hascrc(&mp->m_sb))
- return __this_address;
- if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
- XFS_DINODE_CRC_OFF))
- return __this_address;
- if (be64_to_cpu(dip->di_ino) != ino)
- return __this_address;
- if (!uuid_equal(&dip->di_uuid, &mp->m_sb.sb_meta_uuid))
- return __this_address;
-
flags = be16_to_cpu(dip->di_flags);
flags2 = be64_to_cpu(dip->di_flags2);
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 06/13] xfs: move inode fork verifiers to xfs_dinode_verify
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (4 preceding siblings ...)
2017-12-13 23:58 ` [PATCH 05/13] xfs: verify dinode header first Darrick J. Wong
@ 2017-12-13 23:58 ` Darrick J. Wong
2017-12-19 5:16 ` Dave Chinner
2017-12-13 23:58 ` [PATCH 07/13] xfs: create structure verifier function for shortform xattrs Darrick J. Wong
` (6 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:58 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Consolidate the fork size and format verifiers to xfs_dinode_verify so
that we can reject bad inodes earlier and in a single place.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_inode_buf.c | 72 ++++++++++++++++++++++++++++++++-
fs/xfs/libxfs/xfs_inode_fork.c | 86 ----------------------------------------
2 files changed, 69 insertions(+), 89 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 0e4c720..1392fe9 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -390,6 +390,7 @@ xfs_dinode_verify(
uint16_t mode;
uint16_t flags;
uint64_t flags2;
+ uint64_t di_size;
if (dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC))
return __this_address;
@@ -408,7 +409,8 @@ xfs_dinode_verify(
}
/* don't allow invalid i_size */
- if (be64_to_cpu(dip->di_size) & (1ULL << 63))
+ di_size = be64_to_cpu(dip->di_size);
+ if (di_size & (1ULL << 63))
return __this_address;
mode = be16_to_cpu(dip->di_mode);
@@ -416,14 +418,74 @@ xfs_dinode_verify(
return __this_address;
/* No zero-length symlinks/dirs. */
- if ((S_ISLNK(mode) || S_ISDIR(mode)) && dip->di_size == 0)
+ if ((S_ISLNK(mode) || S_ISDIR(mode)) && di_size == 0)
return __this_address;
+ /* Fork checks carried over from xfs_iformat_fork */
+ if (mode &&
+ be32_to_cpu(dip->di_nextents) + be16_to_cpu(dip->di_anextents) >
+ be64_to_cpu(dip->di_nblocks))
+ return __this_address;
+
+ if (mode && dip->di_forkoff > mp->m_sb.sb_inodesize)
+ return __this_address;
+
+ flags = be16_to_cpu(dip->di_flags);
+
+ if (mode && (flags & XFS_DIFLAG_REALTIME) && !mp->m_rtdev_targp)
+ return __this_address;
+
+ /* Do we have appropriate data fork formats for the mode? */
+ switch (mode & S_IFMT) {
+ case S_IFIFO:
+ case S_IFCHR:
+ case S_IFBLK:
+ case S_IFSOCK:
+ if (dip->di_format != XFS_DINODE_FMT_DEV)
+ return __this_address;
+ break;
+ case S_IFREG:
+ case S_IFLNK:
+ case S_IFDIR:
+ switch (dip->di_format) {
+ case XFS_DINODE_FMT_LOCAL:
+ /*
+ * no local regular files yet
+ */
+ if (S_ISREG(mode))
+ return __this_address;
+ if (di_size > XFS_DFORK_DSIZE(dip, mp))
+ return __this_address;
+ /* fall through */
+ case XFS_DINODE_FMT_EXTENTS:
+ case XFS_DINODE_FMT_BTREE:
+ break;
+ default:
+ return __this_address;
+ }
+ break;
+ case 0:
+ /* Uninitialized inode ok. */
+ break;
+ default:
+ return __this_address;
+ }
+
+ if (XFS_DFORK_Q(dip)) {
+ switch (dip->di_aformat) {
+ case XFS_DINODE_FMT_LOCAL:
+ case XFS_DINODE_FMT_EXTENTS:
+ case XFS_DINODE_FMT_BTREE:
+ break;
+ default:
+ return __this_address;
+ }
+ }
+
/* only version 3 or greater inodes are extensively verified here */
if (dip->di_version < 3)
return NULL;
- flags = be16_to_cpu(dip->di_flags);
flags2 = be64_to_cpu(dip->di_flags2);
/* don't allow reflink/cowextsize if we don't have reflink */
@@ -431,6 +493,10 @@ xfs_dinode_verify(
!xfs_sb_version_hasreflink(&mp->m_sb))
return __this_address;
+ /* only regular files get reflink */
+ if ((flags2 & XFS_DIFLAG2_REFLINK) && (mode & S_IFMT) != S_IFREG)
+ return __this_address;
+
/* don't let reflink and realtime mix */
if ((flags2 & XFS_DIFLAG2_REFLINK) && (flags & XFS_DIFLAG_REALTIME))
return __this_address;
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index c79a161..fd88cbe 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -62,69 +62,11 @@ xfs_iformat_fork(
int error = 0;
xfs_fsize_t di_size;
- if (unlikely(be32_to_cpu(dip->di_nextents) +
- be16_to_cpu(dip->di_anextents) >
- be64_to_cpu(dip->di_nblocks))) {
- xfs_warn(ip->i_mount,
- "corrupt dinode %Lu, extent total = %d, nblocks = %Lu.",
- (unsigned long long)ip->i_ino,
- (int)(be32_to_cpu(dip->di_nextents) +
- be16_to_cpu(dip->di_anextents)),
- (unsigned long long)
- be64_to_cpu(dip->di_nblocks));
- XFS_CORRUPTION_ERROR("xfs_iformat(1)", XFS_ERRLEVEL_LOW,
- ip->i_mount, dip);
- return -EFSCORRUPTED;
- }
-
- if (unlikely(dip->di_forkoff > ip->i_mount->m_sb.sb_inodesize)) {
- xfs_warn(ip->i_mount, "corrupt dinode %Lu, forkoff = 0x%x.",
- (unsigned long long)ip->i_ino,
- dip->di_forkoff);
- XFS_CORRUPTION_ERROR("xfs_iformat(2)", XFS_ERRLEVEL_LOW,
- ip->i_mount, dip);
- return -EFSCORRUPTED;
- }
-
- if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
- !ip->i_mount->m_rtdev_targp)) {
- xfs_warn(ip->i_mount,
- "corrupt dinode %Lu, has realtime flag set.",
- ip->i_ino);
- XFS_CORRUPTION_ERROR("xfs_iformat(realtime)",
- XFS_ERRLEVEL_LOW, ip->i_mount, dip);
- return -EFSCORRUPTED;
- }
-
- if (unlikely(xfs_is_reflink_inode(ip) && !S_ISREG(inode->i_mode))) {
- xfs_warn(ip->i_mount,
- "corrupt dinode %llu, wrong file type for reflink.",
- ip->i_ino);
- XFS_CORRUPTION_ERROR("xfs_iformat(reflink)",
- XFS_ERRLEVEL_LOW, ip->i_mount, dip);
- return -EFSCORRUPTED;
- }
-
- if (unlikely(xfs_is_reflink_inode(ip) &&
- (ip->i_d.di_flags & XFS_DIFLAG_REALTIME))) {
- xfs_warn(ip->i_mount,
- "corrupt dinode %llu, has reflink+realtime flag set.",
- ip->i_ino);
- XFS_CORRUPTION_ERROR("xfs_iformat(reflink)",
- XFS_ERRLEVEL_LOW, ip->i_mount, dip);
- return -EFSCORRUPTED;
- }
-
switch (inode->i_mode & S_IFMT) {
case S_IFIFO:
case S_IFCHR:
case S_IFBLK:
case S_IFSOCK:
- if (unlikely(dip->di_format != XFS_DINODE_FMT_DEV)) {
- XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
- ip->i_mount, dip);
- return -EFSCORRUPTED;
- }
ip->i_d.di_size = 0;
inode->i_rdev = xfs_to_linux_dev_t(xfs_dinode_get_rdev(dip));
break;
@@ -134,32 +76,7 @@ xfs_iformat_fork(
case S_IFDIR:
switch (dip->di_format) {
case XFS_DINODE_FMT_LOCAL:
- /*
- * no local regular files yet
- */
- if (unlikely(S_ISREG(be16_to_cpu(dip->di_mode)))) {
- xfs_warn(ip->i_mount,
- "corrupt inode %Lu (local format for regular file).",
- (unsigned long long) ip->i_ino);
- XFS_CORRUPTION_ERROR("xfs_iformat(4)",
- XFS_ERRLEVEL_LOW,
- ip->i_mount, dip);
- return -EFSCORRUPTED;
- }
-
di_size = be64_to_cpu(dip->di_size);
- if (unlikely(di_size < 0 ||
- di_size > XFS_DFORK_DSIZE(dip, ip->i_mount))) {
- xfs_warn(ip->i_mount,
- "corrupt inode %Lu (bad size %Ld for local inode).",
- (unsigned long long) ip->i_ino,
- (long long) di_size);
- XFS_CORRUPTION_ERROR("xfs_iformat(5)",
- XFS_ERRLEVEL_LOW,
- ip->i_mount, dip);
- return -EFSCORRUPTED;
- }
-
size = (int)di_size;
error = xfs_iformat_local(ip, dip, XFS_DATA_FORK, size);
break;
@@ -170,14 +87,11 @@ xfs_iformat_fork(
error = xfs_iformat_btree(ip, dip, XFS_DATA_FORK);
break;
default:
- XFS_ERROR_REPORT("xfs_iformat(6)", XFS_ERRLEVEL_LOW,
- ip->i_mount);
return -EFSCORRUPTED;
}
break;
default:
- XFS_ERROR_REPORT("xfs_iformat(7)", XFS_ERRLEVEL_LOW, ip->i_mount);
return -EFSCORRUPTED;
}
if (error)
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 07/13] xfs: create structure verifier function for shortform xattrs
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (5 preceding siblings ...)
2017-12-13 23:58 ` [PATCH 06/13] xfs: move inode fork verifiers to xfs_dinode_verify Darrick J. Wong
@ 2017-12-13 23:58 ` Darrick J. Wong
2017-12-19 5:23 ` Dave Chinner
2017-12-13 23:59 ` [PATCH 08/13] xfs: create structure verifier function for short form symlinks Darrick J. Wong
` (5 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:58 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Create a function to perform structure verification for short form
extended attributes.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 70 +++++++++++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_attr_leaf.h | 1 +
2 files changed, 71 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 1274872..a5033f0 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -873,6 +873,76 @@ xfs_attr_shortform_allfit(
return xfs_attr_shortform_bytesfit(dp, bytes);
}
+/* Verify the consistency of an inline attribute fork. */
+xfs_failaddr_t
+xfs_attr_shortform_verify(
+ struct xfs_inode *ip)
+{
+ struct xfs_attr_shortform *sfp;
+ struct xfs_attr_sf_entry *sfep;
+ struct xfs_attr_sf_entry *next_sfep;
+ char *endp;
+ struct xfs_ifork *ifp;
+ int i;
+ int size;
+
+ ASSERT(ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL);
+ ifp = XFS_IFORK_PTR(ip, XFS_ATTR_FORK);
+ sfp = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
+ size = ifp->if_bytes;
+
+ /*
+ * Give up if the attribute is way too short.
+ */
+ if (size < sizeof(struct xfs_attr_sf_hdr))
+ return __this_address;
+
+ endp = (char *)sfp + size;
+
+ /* Check all reported entries */
+ sfep = &sfp->list[0];
+ for (i = 0; i < sfp->hdr.count; i++) {
+ /*
+ * struct xfs_attr_sf_entry has a variable length.
+ * Check the fixed-offset parts of the structure are
+ * within the data buffer.
+ */
+ if (((char *)sfep + sizeof(*sfep)) >= endp)
+ return __this_address;
+
+ /* Don't allow names with known bad length. */
+ if (sfep->namelen == 0)
+ return __this_address;
+
+ /*
+ * Check that the variable-length part of the structure is
+ * within the data buffer. The next entry starts after the
+ * name component, so nextentry is an acceptable test.
+ */
+ next_sfep = XFS_ATTR_SF_NEXTENTRY(sfep);
+ if ((char *)next_sfep > endp)
+ return __this_address;
+
+ /*
+ * Check for unknown flags. Short form doesn't support
+ * the incomplete or local bits.
+ */
+ if (sfep->flags & ~(XFS_ATTR_ROOT | XFS_ATTR_SECURE))
+ return __this_address;
+
+ /* Check for invalid namespace combinations. */
+ if ((sfep->flags & XFS_ATTR_ROOT) &&
+ (sfep->flags & XFS_ATTR_SECURE))
+ return __this_address;
+
+ sfep = next_sfep;
+ }
+ if ((void *)sfep != (void *)endp)
+ return __this_address;
+
+ return NULL;
+}
+
/*
* Convert a leaf attribute list to shortform attribute list
*/
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
index 894124e..4da08af 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.h
+++ b/fs/xfs/libxfs/xfs_attr_leaf.h
@@ -53,6 +53,7 @@ int xfs_attr_shortform_to_leaf(struct xfs_da_args *args,
int xfs_attr_shortform_remove(struct xfs_da_args *args);
int xfs_attr_shortform_allfit(struct xfs_buf *bp, struct xfs_inode *dp);
int xfs_attr_shortform_bytesfit(struct xfs_inode *dp, int bytes);
+xfs_failaddr_t xfs_attr_shortform_verify(struct xfs_inode *ip);
void xfs_attr_fork_remove(struct xfs_inode *ip, struct xfs_trans *tp);
/*
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 08/13] xfs: create structure verifier function for short form symlinks
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (6 preceding siblings ...)
2017-12-13 23:58 ` [PATCH 07/13] xfs: create structure verifier function for shortform xattrs Darrick J. Wong
@ 2017-12-13 23:59 ` Darrick J. Wong
2017-12-19 5:27 ` Dave Chinner
2017-12-13 23:59 ` [PATCH 09/13] xfs: refactor short form directory structure verifier function Darrick J. Wong
` (4 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:59 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Create a function to check the structure of short form symlink targets.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_shared.h | 1 +
fs/xfs/libxfs/xfs_symlink_remote.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h
index c6f4eb4..67ccb1a 100644
--- a/fs/xfs/libxfs/xfs_shared.h
+++ b/fs/xfs/libxfs/xfs_shared.h
@@ -143,5 +143,6 @@ bool xfs_symlink_hdr_ok(xfs_ino_t ino, uint32_t offset,
uint32_t size, struct xfs_buf *bp);
void xfs_symlink_local_to_remote(struct xfs_trans *tp, struct xfs_buf *bp,
struct xfs_inode *ip, struct xfs_ifork *ifp);
+xfs_failaddr_t xfs_symlink_shortform_verify(struct xfs_inode *ip);
#endif /* __XFS_SHARED_H__ */
diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
index b6ef6cb..5d9642e 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.c
+++ b/fs/xfs/libxfs/xfs_symlink_remote.c
@@ -210,3 +210,39 @@ xfs_symlink_local_to_remote(
xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsymlink_hdr) +
ifp->if_bytes - 1);
}
+
+/* Verify the consistency of an inline symlink. */
+xfs_failaddr_t
+xfs_symlink_shortform_verify(
+ struct xfs_inode *ip)
+{
+ char *sfp;
+ char *endp;
+ struct xfs_ifork *ifp;
+ int size;
+
+ ASSERT(ip->i_d.di_format == XFS_DINODE_FMT_LOCAL);
+ ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
+ sfp = (char *)ifp->if_u1.if_data;
+ size = ifp->if_bytes;
+ endp = sfp + size;
+
+ /* Zero length symlinks can exist while we're deleting a remote one. */
+ if (size == 0)
+ return NULL;
+
+ /* No negative sizes or overly long symlink targets. */
+ if (size < 0 || size > XFS_SYMLINK_MAXLEN)
+ return __this_address;
+
+ /* No NULLs in the target either. */
+ for (; sfp < endp; sfp++) {
+ if (*sfp == 0)
+ return __this_address;
+ }
+
+ /* We /did/ null-terminate the buffer, right? */
+ if (*endp != 0)
+ return __this_address;
+ return NULL;
+}
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 09/13] xfs: refactor short form directory structure verifier function
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (7 preceding siblings ...)
2017-12-13 23:59 ` [PATCH 08/13] xfs: create structure verifier function for short form symlinks Darrick J. Wong
@ 2017-12-13 23:59 ` Darrick J. Wong
2017-12-19 5:45 ` Dave Chinner
2017-12-13 23:59 ` [PATCH 10/13] xfs: provide a centralized method for verifying inline fork data Darrick J. Wong
` (3 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:59 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Change the short form directory structure verifier function to return
the instruction pointer of a failing check or NULL if everything's ok.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_dir2_priv.h | 2 +-
fs/xfs/libxfs/xfs_dir2_sf.c | 26 +++++++++++++-------------
fs/xfs/libxfs/xfs_inode_fork.c | 5 ++---
3 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index c441efb..f55245f 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -135,7 +135,7 @@ extern int xfs_dir2_sf_create(struct xfs_da_args *args, xfs_ino_t pino);
extern int xfs_dir2_sf_lookup(struct xfs_da_args *args);
extern int xfs_dir2_sf_removename(struct xfs_da_args *args);
extern int xfs_dir2_sf_replace(struct xfs_da_args *args);
-extern int xfs_dir2_sf_verify(struct xfs_inode *ip);
+extern xfs_failaddr_t xfs_dir2_sf_verify(struct xfs_inode *ip);
/* xfs_dir2_readdir.c */
extern int xfs_readdir(struct xfs_trans *tp, struct xfs_inode *dp,
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index be8b975..8500fa2 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -630,7 +630,7 @@ xfs_dir2_sf_check(
#endif /* DEBUG */
/* Verify the consistency of an inline directory. */
-int
+xfs_failaddr_t
xfs_dir2_sf_verify(
struct xfs_inode *ip)
{
@@ -665,7 +665,7 @@ xfs_dir2_sf_verify(
*/
if (size <= offsetof(struct xfs_dir2_sf_hdr, parent) ||
size < xfs_dir2_sf_hdr_size(sfp->i8count))
- return -EFSCORRUPTED;
+ return __this_address;
endp = (char *)sfp + size;
@@ -674,7 +674,7 @@ xfs_dir2_sf_verify(
i8count = ino > XFS_DIR2_MAX_SHORT_INUM;
error = xfs_dir_ino_validate(mp, ino);
if (error)
- return error;
+ return __this_address;
offset = dops->data_first_offset;
/* Check all reported entries */
@@ -686,11 +686,11 @@ xfs_dir2_sf_verify(
* within the data buffer.
*/
if (((char *)sfep + sizeof(*sfep)) >= endp)
- return -EFSCORRUPTED;
+ return __this_address;
/* Don't allow names with known bad length. */
if (sfep->namelen == 0)
- return -EFSCORRUPTED;
+ return __this_address;
/*
* Check that the variable-length part of the structure is
@@ -699,23 +699,23 @@ xfs_dir2_sf_verify(
*/
next_sfep = dops->sf_nextentry(sfp, sfep);
if (endp < (char *)next_sfep)
- return -EFSCORRUPTED;
+ return __this_address;
/* Check that the offsets always increase. */
if (xfs_dir2_sf_get_offset(sfep) < offset)
- return -EFSCORRUPTED;
+ return __this_address;
/* Check the inode number. */
ino = dops->sf_get_ino(sfp, sfep);
i8count += ino > XFS_DIR2_MAX_SHORT_INUM;
error = xfs_dir_ino_validate(mp, ino);
if (error)
- return error;
+ return __this_address;
/* Check the file type. */
filetype = dops->sf_get_ftype(sfep);
if (filetype >= XFS_DIR3_FT_MAX)
- return -EFSCORRUPTED;
+ return __this_address;
offset = xfs_dir2_sf_get_offset(sfep) +
dops->data_entsize(sfep->namelen);
@@ -723,16 +723,16 @@ xfs_dir2_sf_verify(
sfep = next_sfep;
}
if (i8count != sfp->i8count)
- return -EFSCORRUPTED;
+ return __this_address;
if ((void *)sfep != (void *)endp)
- return -EFSCORRUPTED;
+ return __this_address;
/* Make sure this whole thing ought to be in local format. */
if (offset + (sfp->count + 2) * (uint)sizeof(xfs_dir2_leaf_entry_t) +
(uint)sizeof(xfs_dir2_block_tail_t) > mp->m_dir_geo->blksize)
- return -EFSCORRUPTED;
+ return __this_address;
- return 0;
+ return NULL;
}
/*
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index fd88cbe..a92395a 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -99,10 +99,9 @@ xfs_iformat_fork(
/* Check inline dir contents. */
if (S_ISDIR(inode->i_mode) && dip->di_format == XFS_DINODE_FMT_LOCAL) {
- error = xfs_dir2_sf_verify(ip);
- if (error) {
+ if (xfs_dir2_sf_verify(ip)) {
xfs_idestroy_fork(ip, XFS_DATA_FORK);
- return error;
+ return -EFSCORRUPTED;
}
}
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 10/13] xfs: provide a centralized method for verifying inline fork data
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (8 preceding siblings ...)
2017-12-13 23:59 ` [PATCH 09/13] xfs: refactor short form directory structure verifier function Darrick J. Wong
@ 2017-12-13 23:59 ` Darrick J. Wong
2017-12-19 6:06 ` Dave Chinner
2017-12-13 23:59 ` [PATCH 11/13] xfs: fail out of xfs_attr3_leaf_lookup_int if it looks corrupt Darrick J. Wong
` (2 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:59 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the current haphazard dir2 shortform verifier callsites with a
centralized verifier function that can be called either with the default
verifier functions or with a custom set. This helps us strengthen
integrity checking while providing us with flexibility for repair tools.
xfs_repair wants this to be able to supply its own verifier functions
when trying to fix possibly corrupt metadata.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_inode_fork.c | 73 +++++++++++++++++++++++++++++-----------
fs/xfs/libxfs/xfs_inode_fork.h | 14 ++++++++
fs/xfs/xfs_icache.c | 5 +++
fs/xfs/xfs_inode.c | 34 ++++++++++++++++---
fs/xfs/xfs_inode.h | 2 +
fs/xfs/xfs_log_recover.c | 4 ++
6 files changed, 108 insertions(+), 24 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index a92395a..7756103 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -35,6 +35,8 @@
#include "xfs_da_format.h"
#include "xfs_da_btree.h"
#include "xfs_dir2_priv.h"
+#include "xfs_attr_leaf.h"
+#include "xfs_shared.h"
kmem_zone_t *xfs_ifork_zone;
@@ -97,14 +99,6 @@ xfs_iformat_fork(
if (error)
return error;
- /* Check inline dir contents. */
- if (S_ISDIR(inode->i_mode) && dip->di_format == XFS_DINODE_FMT_LOCAL) {
- if (xfs_dir2_sf_verify(ip)) {
- xfs_idestroy_fork(ip, XFS_DATA_FORK);
- return -EFSCORRUPTED;
- }
- }
-
if (xfs_is_reflink_inode(ip)) {
ASSERT(ip->i_cowfp == NULL);
xfs_ifork_init_cow(ip);
@@ -121,18 +115,6 @@ xfs_iformat_fork(
atp = (xfs_attr_shortform_t *)XFS_DFORK_APTR(dip);
size = be16_to_cpu(atp->hdr.totsize);
- if (unlikely(size < sizeof(struct xfs_attr_sf_hdr))) {
- xfs_warn(ip->i_mount,
- "corrupt inode %Lu (bad attr fork size %Ld).",
- (unsigned long long) ip->i_ino,
- (long long) size);
- XFS_CORRUPTION_ERROR("xfs_iformat(8)",
- XFS_ERRLEVEL_LOW,
- ip->i_mount, dip);
- error = -EFSCORRUPTED;
- break;
- }
-
error = xfs_iformat_local(ip, dip, XFS_ATTR_FORK, size);
break;
case XFS_DINODE_FMT_EXTENTS:
@@ -740,3 +722,54 @@ xfs_ifork_init_cow(
ip->i_cformat = XFS_DINODE_FMT_EXTENTS;
ip->i_cnextents = 0;
}
+
+/* Default fork content verifiers. */
+struct xfs_ifork_ops xfs_default_ifork_ops = {
+ .verify_attr = xfs_attr_shortform_verify,
+ .verify_dir = xfs_dir2_sf_verify,
+ .verify_symlink = xfs_symlink_shortform_verify,
+};
+
+/* Verify the inline contents of the data fork of an inode. */
+xfs_failaddr_t
+xfs_ifork_verify_data(
+ struct xfs_inode *ip,
+ struct xfs_ifork_ops *ops)
+{
+ xfs_ifork_verifier_t fn = NULL;
+
+ /* Non-local data fork, we're done. */
+ if (ip->i_d.di_format != XFS_DINODE_FMT_LOCAL)
+ return NULL;
+
+ /* Check the inline data fork if there is one. */
+ if (S_ISDIR(VFS_I(ip)->i_mode))
+ fn = ops->verify_dir;
+ else if (S_ISLNK(VFS_I(ip)->i_mode))
+ fn = ops->verify_symlink;
+
+ if (fn)
+ return fn(ip);
+ return NULL;
+}
+
+/* Verify the inline contents of the attr fork of an inode. */
+xfs_failaddr_t
+xfs_ifork_verify_attr(
+ struct xfs_inode *ip,
+ struct xfs_ifork_ops *ops)
+{
+ xfs_ifork_verifier_t fn = NULL;
+
+ /* There has to be an attr fork allocated if aformat is local. */
+ if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
+ if (XFS_IFORK_PTR(ip, XFS_ATTR_FORK))
+ fn = ops->verify_attr;
+ else
+ return __this_address;
+ }
+
+ if (fn)
+ return fn(ip);
+ return NULL;
+}
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index b9f0098..dd8aba0 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -186,4 +186,18 @@ extern struct kmem_zone *xfs_ifork_zone;
extern void xfs_ifork_init_cow(struct xfs_inode *ip);
+typedef xfs_failaddr_t (*xfs_ifork_verifier_t)(struct xfs_inode *);
+
+struct xfs_ifork_ops {
+ xfs_ifork_verifier_t verify_symlink;
+ xfs_ifork_verifier_t verify_dir;
+ xfs_ifork_verifier_t verify_attr;
+};
+extern struct xfs_ifork_ops xfs_default_ifork_ops;
+
+xfs_failaddr_t xfs_ifork_verify_data(struct xfs_inode *ip,
+ struct xfs_ifork_ops *ops);
+xfs_failaddr_t xfs_ifork_verify_attr(struct xfs_inode *ip,
+ struct xfs_ifork_ops *ops);
+
#endif /* __XFS_INODE_FORK_H__ */
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 43005fb..81ebb68 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -473,6 +473,11 @@ xfs_iget_cache_miss(
if (error)
goto out_destroy;
+ if (!xfs_inode_verify_forks(ip)) {
+ error = -EFSCORRUPTED;
+ goto out_destroy;
+ }
+
trace_xfs_iget_miss(ip);
if ((VFS_I(ip)->i_mode == 0) && !(flags & XFS_IGET_CREATE)) {
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 8012741..d1e2518 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -3488,6 +3488,34 @@ xfs_iflush(
return error;
}
+/*
+ * If there are inline format data / attr forks attached to this inode,
+ * make sure they're not corrupt.
+ */
+bool
+xfs_inode_verify_forks(
+ struct xfs_inode *ip)
+{
+ xfs_failaddr_t fa;
+
+ fa = xfs_ifork_verify_data(ip, &xfs_default_ifork_ops);
+ if (fa) {
+ xfs_alert(ip->i_mount,
+ "%s: bad inode %llu inline data fork at %pF",
+ __func__, ip->i_ino, fa);
+ return false;
+ }
+
+ fa = xfs_ifork_verify_attr(ip, &xfs_default_ifork_ops);
+ if (fa) {
+ xfs_alert(ip->i_mount,
+ "%s: bad inode %llu inline attr fork at %pF",
+ __func__, ip->i_ino, fa);
+ return false;
+ }
+ return true;
+}
+
STATIC int
xfs_iflush_int(
struct xfs_inode *ip,
@@ -3566,10 +3594,8 @@ xfs_iflush_int(
if (ip->i_d.di_version < 3)
ip->i_d.di_flushiter++;
- /* Check the inline directory data. */
- if (S_ISDIR(VFS_I(ip)->i_mode) &&
- ip->i_d.di_format == XFS_DINODE_FMT_LOCAL &&
- xfs_dir2_sf_verify(ip))
+ /* Check the inline fork data before we write out. */
+ if (!xfs_inode_verify_forks(ip))
goto corrupt_out;
/*
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index cc13c37..e4d1708 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -490,4 +490,6 @@ extern struct kmem_zone *xfs_inode_zone;
/* The default CoW extent size hint. */
#define XFS_DEFAULT_COWEXTSZ_HINT 32
+bool xfs_inode_verify_forks(struct xfs_inode *ip);
+
#endif /* __XFS_INODE_H__ */
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 28d1abf..04f5b30 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2957,6 +2957,10 @@ xfs_recover_inode_owner_change(
if (error)
goto out_free_ip;
+ if (!xfs_inode_verify_forks(ip)) {
+ error = -EFSCORRUPTED;
+ goto out_free_ip;
+ }
if (in_f->ilf_fields & XFS_ILOG_DOWNER) {
ASSERT(in_f->ilf_fields & XFS_ILOG_DBROOT);
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 11/13] xfs: fail out of xfs_attr3_leaf_lookup_int if it looks corrupt
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (9 preceding siblings ...)
2017-12-13 23:59 ` [PATCH 10/13] xfs: provide a centralized method for verifying inline fork data Darrick J. Wong
@ 2017-12-13 23:59 ` Darrick J. Wong
2017-12-19 6:13 ` Dave Chinner
2017-12-13 23:59 ` [PATCH 12/13] xfs: create a new buf_ops pointer to verify structure metadata Darrick J. Wong
2017-12-13 23:59 ` [PATCH 13/13] xfs: scrub in-core metadata Darrick J. Wong
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:59 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
If the xattr leaf block looks corrupt, return -EFSCORRUPTED to userspace
instead of ASSERTing on debug kernels or running off the end of the
buffer on regular kernels.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index a5033f0..e130952 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -2246,7 +2246,8 @@ xfs_attr3_leaf_lookup_int(
leaf = bp->b_addr;
xfs_attr3_leaf_hdr_from_disk(args->geo, &ichdr, leaf);
entries = xfs_attr3_leaf_entryp(leaf);
- ASSERT(ichdr.count < args->geo->blksize / 8);
+ if (ichdr.count >= args->geo->blksize / 8)
+ return -EFSCORRUPTED;
/*
* Binary search. (note: small blocks will skip this loop)
@@ -2262,8 +2263,10 @@ xfs_attr3_leaf_lookup_int(
else
break;
}
- ASSERT(probe >= 0 && (!ichdr.count || probe < ichdr.count));
- ASSERT(span <= 4 || be32_to_cpu(entry->hashval) == hashval);
+ if (!(probe >= 0 && (!ichdr.count || probe < ichdr.count)))
+ return -EFSCORRUPTED;
+ if (!(span <= 4 || be32_to_cpu(entry->hashval) == hashval))
+ return -EFSCORRUPTED;
/*
* Since we may have duplicate hashval's, find the first matching
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 12/13] xfs: create a new buf_ops pointer to verify structure metadata
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (10 preceding siblings ...)
2017-12-13 23:59 ` [PATCH 11/13] xfs: fail out of xfs_attr3_leaf_lookup_int if it looks corrupt Darrick J. Wong
@ 2017-12-13 23:59 ` Darrick J. Wong
2017-12-19 6:22 ` Dave Chinner
2017-12-13 23:59 ` [PATCH 13/13] xfs: scrub in-core metadata Darrick J. Wong
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:59 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Expose all metadata structure buffer verifier functions via buf_ops.
These will be used by the online scrub mechanism to look for problems
with buffers that are already sitting around in memory.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_alloc.c | 23 +++++++++++++++++------
fs/xfs/libxfs/xfs_alloc_btree.c | 1 +
fs/xfs/libxfs/xfs_attr_leaf.c | 1 +
fs/xfs/libxfs/xfs_attr_remote.c | 32 ++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_bmap_btree.c | 1 +
fs/xfs/libxfs/xfs_da_btree.c | 25 +++++++++++++++++++++++++
fs/xfs/libxfs/xfs_dir2_block.c | 1 +
fs/xfs/libxfs/xfs_dir2_data.c | 1 +
fs/xfs/libxfs/xfs_dir2_leaf.c | 16 ++++++++++++++++
fs/xfs/libxfs/xfs_dir2_node.c | 1 +
fs/xfs/libxfs/xfs_dquot_buf.c | 12 ++++++++++++
fs/xfs/libxfs/xfs_ialloc.c | 1 +
fs/xfs/libxfs/xfs_ialloc_btree.c | 1 +
fs/xfs/libxfs/xfs_refcount_btree.c | 1 +
fs/xfs/libxfs/xfs_rmap_btree.c | 1 +
fs/xfs/libxfs/xfs_symlink_remote.c | 1 +
fs/xfs/xfs_buf.h | 1 +
17 files changed, 114 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index d600dcb..ae3e33d 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -528,6 +528,15 @@ xfs_agfl_verify(
struct xfs_agfl *agfl = XFS_BUF_TO_AGFL(bp);
int i;
+ /*
+ * There is no verification of non-crc AGFLs because mkfs does not
+ * initialise the AGFL to zero or NULL. Hence the only valid part of the
+ * AGFL is what the AGF says is active. We can't get to the AGF, so we
+ * can't verify just those entries are valid.
+ */
+ if (!xfs_sb_version_hascrc(&mp->m_sb))
+ return NULL;
+
if (!uuid_equal(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid))
return __this_address;
if (be32_to_cpu(agfl->agfl_magicnum) != XFS_AGFL_MAGIC)
@@ -606,6 +615,7 @@ const struct xfs_buf_ops xfs_agfl_buf_ops = {
.name = "xfs_agfl",
.verify_read = xfs_agfl_read_verify,
.verify_write = xfs_agfl_write_verify,
+ .verify_struct = xfs_agfl_verify,
};
/*
@@ -2403,10 +2413,10 @@ xfs_alloc_put_freelist(
static xfs_failaddr_t
xfs_agf_verify(
- struct xfs_mount *mp,
- struct xfs_buf *bp)
- {
- struct xfs_agf *agf = XFS_BUF_TO_AGF(bp);
+ struct xfs_buf *bp)
+{
+ struct xfs_mount *mp = bp->b_target->bt_mount;
+ struct xfs_agf *agf = XFS_BUF_TO_AGF(bp);
if (xfs_sb_version_hascrc(&mp->m_sb)) {
if (!uuid_equal(&agf->agf_uuid, &mp->m_sb.sb_meta_uuid))
@@ -2468,7 +2478,7 @@ xfs_agf_read_verify(
!xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF)) {
fa = __this_address;
xfs_buf_ioerror(bp, -EFSBADCRC);
- } else if (XFS_TEST_ERROR((fa = xfs_agf_verify(mp, bp)), mp,
+ } else if (XFS_TEST_ERROR((fa = xfs_agf_verify(bp)), mp,
XFS_ERRTAG_ALLOC_READ_AGF))
xfs_buf_ioerror(bp, -EFSCORRUPTED);
@@ -2484,7 +2494,7 @@ xfs_agf_write_verify(
struct xfs_buf_log_item *bip = bp->b_fspriv;
xfs_failaddr_t fa;
- if ((fa = xfs_agf_verify(mp, bp))) {
+ if ((fa = xfs_agf_verify(bp))) {
xfs_buf_ioerror(bp, -EFSCORRUPTED);
xfs_verifier_error(bp, fa);
return;
@@ -2503,6 +2513,7 @@ const struct xfs_buf_ops xfs_agf_buf_ops = {
.name = "xfs_agf",
.verify_read = xfs_agf_read_verify,
.verify_write = xfs_agf_write_verify,
+ .verify_struct = xfs_agf_verify,
};
/*
diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index a646fc4..7d116e8 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -398,6 +398,7 @@ const struct xfs_buf_ops xfs_allocbt_buf_ops = {
.name = "xfs_allocbt",
.verify_read = xfs_allocbt_read_verify,
.verify_write = xfs_allocbt_write_verify,
+ .verify_struct = xfs_allocbt_verify,
};
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index e130952..ebfa477 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -340,6 +340,7 @@ const struct xfs_buf_ops xfs_attr3_leaf_buf_ops = {
.name = "xfs_attr3_leaf",
.verify_read = xfs_attr3_leaf_read_verify,
.verify_write = xfs_attr3_leaf_write_verify,
+ .verify_struct = xfs_attr3_leaf_verify,
};
int
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index d4d2902..1be995b 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -204,10 +204,42 @@ xfs_attr3_rmt_write_verify(
ASSERT(len == 0);
}
+static xfs_failaddr_t
+xfs_attr3_rmt_verify_struct(
+ struct xfs_buf *bp)
+{
+ struct xfs_mount *mp = bp->b_target->bt_mount;
+ char *ptr;
+ void *failed_at;
+ int len;
+ xfs_daddr_t bno;
+ int blksize = mp->m_attr_geo->blksize;
+
+ /* no verification of non-crc buffers */
+ if (!xfs_sb_version_hascrc(&mp->m_sb))
+ return NULL;
+
+ ptr = bp->b_addr;
+ bno = bp->b_bn;
+ len = BBTOB(bp->b_length);
+ ASSERT(len >= blksize);
+
+ while (len > 0) {
+ if ((failed_at = xfs_attr3_rmt_verify(mp, ptr, blksize, bno)))
+ return failed_at;
+ len -= blksize;
+ ptr += blksize;
+ bno += BTOBB(blksize);
+ }
+
+ return NULL;
+}
+
const struct xfs_buf_ops xfs_attr3_rmt_buf_ops = {
.name = "xfs_attr3_rmt",
.verify_read = xfs_attr3_rmt_read_verify,
.verify_write = xfs_attr3_rmt_write_verify,
+ .verify_struct = xfs_attr3_rmt_verify_struct,
};
STATIC int
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index c1ddb4c..1adfc52 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -501,6 +501,7 @@ const struct xfs_buf_ops xfs_bmbt_buf_ops = {
.name = "xfs_bmbt",
.verify_read = xfs_bmbt_read_verify,
.verify_write = xfs_bmbt_write_verify,
+ .verify_struct = xfs_bmbt_verify,
};
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 6d6a9ef..62697bc 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -248,10 +248,35 @@ xfs_da3_node_read_verify(
xfs_verifier_error(bp, fa);
}
+/* Verify the structure of a da3 block. */
+static xfs_failaddr_t
+xfs_da3_node_verify_struct(
+ struct xfs_buf *bp)
+{
+ struct xfs_da_blkinfo *info = bp->b_addr;
+
+ switch (be16_to_cpu(info->magic)) {
+ case XFS_DA3_NODE_MAGIC:
+ case XFS_DA_NODE_MAGIC:
+ return xfs_da3_node_verify(bp);
+ case XFS_ATTR_LEAF_MAGIC:
+ case XFS_ATTR3_LEAF_MAGIC:
+ bp->b_ops = &xfs_attr3_leaf_buf_ops;
+ return bp->b_ops->verify_struct(bp);
+ case XFS_DIR2_LEAFN_MAGIC:
+ case XFS_DIR3_LEAFN_MAGIC:
+ bp->b_ops = &xfs_dir3_leafn_buf_ops;
+ return bp->b_ops->verify_struct(bp);
+ default:
+ return __this_address;
+ }
+}
+
const struct xfs_buf_ops xfs_da3_node_buf_ops = {
.name = "xfs_da3_node",
.verify_read = xfs_da3_node_read_verify,
.verify_write = xfs_da3_node_write_verify,
+ .verify_struct = xfs_da3_node_verify_struct,
};
int
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 7a18777..5e2bade 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -127,6 +127,7 @@ const struct xfs_buf_ops xfs_dir3_block_buf_ops = {
.name = "xfs_dir3_block",
.verify_read = xfs_dir3_block_read_verify,
.verify_write = xfs_dir3_block_write_verify,
+ .verify_struct = xfs_dir3_block_verify,
};
int
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index f31bfd9..b65f915 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -319,6 +319,7 @@ const struct xfs_buf_ops xfs_dir3_data_buf_ops = {
.name = "xfs_dir3_data",
.verify_read = xfs_dir3_data_read_verify,
.verify_write = xfs_dir3_data_write_verify,
+ .verify_struct = xfs_dir3_data_verify,
};
static const struct xfs_buf_ops xfs_dir3_data_reada_buf_ops = {
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index e3fad8d..75bf3b2b 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -219,6 +219,13 @@ __write_verify(
xfs_buf_update_cksum(bp, XFS_DIR3_LEAF_CRC_OFF);
}
+static xfs_failaddr_t
+xfs_dir3_leaf1_verify(
+ struct xfs_buf *bp)
+{
+ return xfs_dir3_leaf_verify(bp, XFS_DIR2_LEAF1_MAGIC);
+}
+
static void
xfs_dir3_leaf1_read_verify(
struct xfs_buf *bp)
@@ -233,6 +240,13 @@ xfs_dir3_leaf1_write_verify(
__write_verify(bp, XFS_DIR2_LEAF1_MAGIC);
}
+static xfs_failaddr_t
+xfs_dir3_leafn_verify(
+ struct xfs_buf *bp)
+{
+ return xfs_dir3_leaf_verify(bp, XFS_DIR2_LEAFN_MAGIC);
+}
+
static void
xfs_dir3_leafn_read_verify(
struct xfs_buf *bp)
@@ -251,12 +265,14 @@ const struct xfs_buf_ops xfs_dir3_leaf1_buf_ops = {
.name = "xfs_dir3_leaf1",
.verify_read = xfs_dir3_leaf1_read_verify,
.verify_write = xfs_dir3_leaf1_write_verify,
+ .verify_struct = xfs_dir3_leaf1_verify,
};
const struct xfs_buf_ops xfs_dir3_leafn_buf_ops = {
.name = "xfs_dir3_leafn",
.verify_read = xfs_dir3_leafn_read_verify,
.verify_write = xfs_dir3_leafn_write_verify,
+ .verify_struct = xfs_dir3_leafn_verify,
};
int
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 8a6a2e1..22acf79 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -156,6 +156,7 @@ const struct xfs_buf_ops xfs_dir3_free_buf_ops = {
.name = "xfs_dir3_free",
.verify_read = xfs_dir3_free_read_verify,
.verify_write = xfs_dir3_free_write_verify,
+ .verify_struct = xfs_dir3_free_verify,
};
/* Everything ok in the free block header? */
diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 5561011..74d377c 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -242,6 +242,17 @@ xfs_dquot_buf_verify(
return true;
}
+static xfs_failaddr_t
+xfs_dquot_buf_verify_struct(
+ struct xfs_buf *bp)
+{
+ struct xfs_mount *mp = bp->b_target->bt_mount;
+
+ if (!xfs_dquot_buf_verify(mp, bp, 0))
+ return __this_address;
+ return NULL;
+}
+
static void
xfs_dquot_buf_read_verify(
struct xfs_buf *bp)
@@ -298,6 +309,7 @@ const struct xfs_buf_ops xfs_dquot_buf_ops = {
.name = "xfs_dquot",
.verify_read = xfs_dquot_buf_read_verify,
.verify_write = xfs_dquot_buf_write_verify,
+ .verify_struct = xfs_dquot_buf_verify_struct,
};
const struct xfs_buf_ops xfs_dquot_buf_ra_ops = {
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index a616212..d9c93b6 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -2585,6 +2585,7 @@ const struct xfs_buf_ops xfs_agi_buf_ops = {
.name = "xfs_agi",
.verify_read = xfs_agi_read_verify,
.verify_write = xfs_agi_write_verify,
+ .verify_struct = xfs_agi_verify,
};
/*
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index 032613a..e5aee4b 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -328,6 +328,7 @@ const struct xfs_buf_ops xfs_inobt_buf_ops = {
.name = "xfs_inobt",
.verify_read = xfs_inobt_read_verify,
.verify_write = xfs_inobt_write_verify,
+ .verify_struct = xfs_inobt_verify,
};
STATIC int
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 68d4021..d754af2 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -289,6 +289,7 @@ const struct xfs_buf_ops xfs_refcountbt_buf_ops = {
.name = "xfs_refcountbt",
.verify_read = xfs_refcountbt_read_verify,
.verify_write = xfs_refcountbt_write_verify,
+ .verify_struct = xfs_refcountbt_verify,
};
STATIC int
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index ab358b9..e568cfb 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -381,6 +381,7 @@ const struct xfs_buf_ops xfs_rmapbt_buf_ops = {
.name = "xfs_rmapbt",
.verify_read = xfs_rmapbt_read_verify,
.verify_write = xfs_rmapbt_write_verify,
+ .verify_struct = xfs_rmapbt_verify,
};
STATIC int
diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
index 5d9642e..04eae02 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.c
+++ b/fs/xfs/libxfs/xfs_symlink_remote.c
@@ -174,6 +174,7 @@ const struct xfs_buf_ops xfs_symlink_buf_ops = {
.name = "xfs_symlink",
.verify_read = xfs_symlink_read_verify,
.verify_write = xfs_symlink_write_verify,
+ .verify_struct = xfs_symlink_verify,
};
void
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index f873bb7..c2fe894 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -140,6 +140,7 @@ struct xfs_buf_ops {
char *name;
void (*verify_read)(struct xfs_buf *);
void (*verify_write)(struct xfs_buf *);
+ xfs_failaddr_t (*verify_struct)(struct xfs_buf *bp);
};
typedef struct xfs_buf {
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 13/13] xfs: scrub in-core metadata
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
` (11 preceding siblings ...)
2017-12-13 23:59 ` [PATCH 12/13] xfs: create a new buf_ops pointer to verify structure metadata Darrick J. Wong
@ 2017-12-13 23:59 ` Darrick J. Wong
2017-12-19 6:23 ` Dave Chinner
12 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-13 23:59 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Whenever we load a buffer, explicitly re-call the structure verifier to
ensure that memory isn't corrupting things.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
| 3 +++
fs/xfs/scrub/btree.c | 4 ++++
fs/xfs/scrub/common.c | 23 +++++++++++++++++++++++
fs/xfs/scrub/common.h | 2 ++
fs/xfs/scrub/dabtree.c | 22 ++++++++++++++++++++++
fs/xfs/scrub/dir.c | 4 ++++
6 files changed, 58 insertions(+)
--git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 1503cbc..2632e00 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -571,6 +571,7 @@ xfs_scrub_agf(
&sc->sa.agf_bp, &sc->sa.agfl_bp);
if (!xfs_scrub_process_error(sc, agno, XFS_AGF_BLOCK(sc->mp), &error))
goto out;
+ xfs_scrub_buffer_recheck(sc, sc->sa.agf_bp);
agf = XFS_BUF_TO_AGF(sc->sa.agf_bp);
@@ -742,6 +743,7 @@ xfs_scrub_agfl(
goto out;
if (!sc->sa.agf_bp)
return -EFSCORRUPTED;
+ xfs_scrub_buffer_recheck(sc, sc->sa.agfl_bp);
if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
goto out;
@@ -855,6 +857,7 @@ xfs_scrub_agi(
&sc->sa.agf_bp, &sc->sa.agfl_bp);
if (!xfs_scrub_process_error(sc, agno, XFS_AGI_BLOCK(sc->mp), &error))
goto out;
+ xfs_scrub_buffer_recheck(sc, sc->sa.agi_bp);
agi = XFS_BUF_TO_AGI(sc->sa.agi_bp);
diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
index 26149cb..b30f30b 100644
--- a/fs/xfs/scrub/btree.c
+++ b/fs/xfs/scrub/btree.c
@@ -312,6 +312,8 @@ xfs_scrub_btree_block_check_sibling(
pp = xfs_btree_ptr_addr(ncur, ncur->bc_ptrs[level + 1], pblock);
if (!xfs_scrub_btree_ptr_ok(bs, level + 1, pp))
goto out;
+ if (pbp)
+ xfs_scrub_buffer_recheck(bs->sc, pbp);
if (xfs_btree_diff_two_ptrs(cur, pp, sibling))
xfs_scrub_btree_set_corrupt(bs->sc, cur, level);
@@ -481,6 +483,8 @@ xfs_scrub_btree_get_block(
xfs_scrub_btree_set_corrupt(bs->sc, bs->cur, level);
return 0;
}
+ if (*pbp)
+ xfs_scrub_buffer_recheck(bs->sc, *pbp);
/*
* Check the block's owner; this function absorbs error codes
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index c36a91b..1e1d843 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -729,3 +729,26 @@ xfs_scrub_should_xref(
*error = 0;
return false;
}
+
+/* Run the structure verifiers on in-memory buffers to detect bad memory. */
+void
+xfs_scrub_buffer_recheck(
+ struct xfs_scrub_context *sc,
+ struct xfs_buf *bp)
+{
+ xfs_failaddr_t fa;
+
+ if (bp->b_ops == NULL) {
+ xfs_scrub_block_set_corrupt(sc, bp);
+ return;
+ }
+ if (bp->b_ops->verify_struct == NULL) {
+ xfs_scrub_set_incomplete(sc);
+ return;
+ }
+ fa = bp->b_ops->verify_struct(bp);
+ if (!fa)
+ return;
+ sc->sm->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
+ trace_xfs_scrub_block_error(sc, bp->b_bn, fa);
+}
diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h
index 4f6a6e1..4db5e11 100644
--- a/fs/xfs/scrub/common.h
+++ b/fs/xfs/scrub/common.h
@@ -165,4 +165,6 @@ static inline __u32 xfs_scrub_corrupt_flag(bool xref)
return xref ? XFS_SCRUB_OFLAG_XCORRUPT : XFS_SCRUB_OFLAG_CORRUPT;
}
+void xfs_scrub_buffer_recheck(struct xfs_scrub_context *sc, struct xfs_buf *bp);
+
#endif /* __XFS_SCRUB_COMMON_H__ */
diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c
index d94edd9..bffdb7d 100644
--- a/fs/xfs/scrub/dabtree.c
+++ b/fs/xfs/scrub/dabtree.c
@@ -233,11 +233,28 @@ xfs_scrub_da_btree_write_verify(
return;
}
}
+static void *
+xfs_scrub_da_btree_verify(
+ struct xfs_buf *bp)
+{
+ struct xfs_da_blkinfo *info = bp->b_addr;
+
+ switch (be16_to_cpu(info->magic)) {
+ case XFS_DIR2_LEAF1_MAGIC:
+ case XFS_DIR3_LEAF1_MAGIC:
+ bp->b_ops = &xfs_dir3_leaf1_buf_ops;
+ return bp->b_ops->verify_struct(bp);
+ default:
+ bp->b_ops = &xfs_da3_node_buf_ops;
+ return bp->b_ops->verify_struct(bp);
+ }
+}
static const struct xfs_buf_ops xfs_scrub_da_btree_buf_ops = {
.name = "xfs_scrub_da_btree",
.verify_read = xfs_scrub_da_btree_read_verify,
.verify_write = xfs_scrub_da_btree_write_verify,
+ .verify_struct = xfs_scrub_da_btree_verify,
};
/* Check a block's sibling. */
@@ -276,6 +293,9 @@ xfs_scrub_da_btree_block_check_sibling(
xfs_scrub_da_set_corrupt(ds, level);
return error;
}
+ if (ds->state->altpath.blk[level].bp)
+ xfs_scrub_buffer_recheck(ds->sc,
+ ds->state->altpath.blk[level].bp);
/* Compare upper level pointer to sibling pointer. */
if (ds->state->altpath.blk[level].blkno != sibling)
@@ -358,6 +378,8 @@ xfs_scrub_da_btree_block(
&xfs_scrub_da_btree_buf_ops);
if (!xfs_scrub_da_process_error(ds, level, &error))
goto out_nobuf;
+ if (blk->bp)
+ xfs_scrub_buffer_recheck(ds->sc, blk->bp);
/*
* We didn't find a dir btree root block, which means that
diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index 69e1efd..0a4e768 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -237,6 +237,7 @@ xfs_scrub_dir_rec(
xfs_scrub_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
goto out;
}
+ xfs_scrub_buffer_recheck(ds->sc, bp);
/* Retrieve the entry, sanity check it, and compare hashes. */
dent = (struct xfs_dir2_data_entry *)(((char *)bp->b_addr) + off);
@@ -324,6 +325,7 @@ xfs_scrub_directory_data_bestfree(
}
if (!xfs_scrub_fblock_process_error(sc, XFS_DATA_FORK, lblk, &error))
goto out;
+ xfs_scrub_buffer_recheck(sc, bp);
/* XXX: Check xfs_dir3_data_hdr.pad is zero once we start setting it. */
@@ -474,6 +476,7 @@ xfs_scrub_directory_leaf1_bestfree(
error = xfs_dir3_leaf_read(sc->tp, sc->ip, lblk, -1, &bp);
if (!xfs_scrub_fblock_process_error(sc, XFS_DATA_FORK, lblk, &error))
goto out;
+ xfs_scrub_buffer_recheck(sc, bp);
leaf = bp->b_addr;
d_ops->leaf_hdr_from_disk(&leafhdr, leaf);
@@ -559,6 +562,7 @@ xfs_scrub_directory_free_bestfree(
error = xfs_dir2_free_read(sc->tp, sc->ip, lblk, &bp);
if (!xfs_scrub_fblock_process_error(sc, XFS_DATA_FORK, lblk, &error))
goto out;
+ xfs_scrub_buffer_recheck(sc, bp);
if (xfs_sb_version_hascrc(&sc->mp->m_sb)) {
struct xfs_dir3_free_hdr *hdr3 = bp->b_addr;
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 04/13] xfs: refactor verifier callers to print address of failing check
2017-12-13 23:58 ` [PATCH 04/13] xfs: refactor verifier callers to print address of failing check Darrick J. Wong
@ 2017-12-14 22:03 ` Dave Chinner
2017-12-15 0:04 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-14 22:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:58:37PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Refactor the callers of verifiers to print the instruction address of a
> failing check.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Just a quick comment about formatting as I browsed the patch...
> @@ -567,13 +568,14 @@ xfs_agfl_read_verify(
> if (!xfs_sb_version_hascrc(&mp->m_sb))
> return;
>
> - if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF))
> + if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
> + fa = __this_address;
> xfs_buf_ioerror(bp, -EFSBADCRC);
> - else if (xfs_agfl_verify(bp))
> + } else if ((fa = xfs_agfl_verify(bp)))
> xfs_buf_ioerror(bp, -EFSCORRUPTED);
>
> if (bp->b_error)
> - xfs_verifier_error(bp);
> + xfs_verifier_error(bp, fa);
We are really trying to get rid of assignments in if() statements,
so I'd prefer we don't add a bunch of new ones. While I understand
there's a lot of mechanical change in this patch, I'd prefer to see
these end up as something more like:
> - if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF))
> + if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
> + fa = __this_address;
> xfs_buf_ioerror(bp, -EFSBADCRC);
> - else if (xfs_agfl_verify(bp))
> + } else if ((fa = xfs_agfl_verify(bp)))
> xfs_buf_ioerror(bp, -EFSCORRUPTED);
if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
fa = __this_address;
error = -EFSBADCRC;
} else {
fa = xfs_agfl_verify(bp);
if (fa)
error = -EFSCORRUPTED;
}
if (error) {
xfs_buf_ioerror(bp, error);
xfs_verifier_error(bp, fa);
}
.....
> @@ -2459,16 +2462,18 @@ xfs_agf_read_verify(
> struct xfs_buf *bp)
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
> + xfs_failaddr_t fa;
>
> if (xfs_sb_version_hascrc(&mp->m_sb) &&
> - !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF))
> + !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF)) {
> + fa = __this_address;
> xfs_buf_ioerror(bp, -EFSBADCRC);
> - else if (XFS_TEST_ERROR(xfs_agf_verify(mp, bp), mp,
> - XFS_ERRTAG_ALLOC_READ_AGF))
> + } else if (XFS_TEST_ERROR((fa = xfs_agf_verify(mp, bp)), mp,
> + XFS_ERRTAG_ALLOC_READ_AGF))
> xfs_buf_ioerror(bp, -EFSCORRUPTED);
>
> if (bp->b_error)
> - xfs_verifier_error(bp);
> + xfs_verifier_error(bp, fa);
> }
Because this sort of thing is now getting towards being unreadable.
With the way we keep adding to verifier checks, it's only going to
get worse if we don't take steps to clean it up...
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 4c9f35d..0bbbf0b 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -347,13 +347,15 @@ xfs_corruption_error(
> */
> void
> xfs_verifier_error(
> - struct xfs_buf *bp)
> + struct xfs_buf *bp,
> + xfs_failaddr_t fa)
> {
> struct xfs_mount *mp = bp->b_target->bt_mount;
>
> xfs_alert(mp, "Metadata %s detected at %pS, %s block 0x%llx",
> bp->b_error == -EFSBADCRC ? "CRC error" : "corruption",
> - __return_address, bp->b_ops->name, bp->b_bn);
> + fa ? fa : __return_address, bp->b_ops->name,
> + bp->b_bn);
>
> xfs_alert(mp, "Unmount and run xfs_repair");
I'm also wondering if we should move the xfs_buf_ioerror() call
inside this function, too, rather than coding multiple calls in the
verifiers to set bp->b_error in each branch of the verifier that
has an error...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 01/13] xfs: refactor long-format btree header verification routines
2017-12-13 23:58 ` [PATCH 01/13] xfs: refactor long-format btree header verification routines Darrick J. Wong
@ 2017-12-14 22:06 ` Dave Chinner
2017-12-15 0:12 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-14 22:06 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:58:19PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Create two helper functions to verify the headers of a long format
> btree block. We'll use this later for the realtime rmapbt.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
.....
> +bool
> +xfs_btree_lblock_verify(
> + struct xfs_buf *bp,
> + unsigned int max_recs)
> +{
> + struct xfs_mount *mp = bp->b_target->bt_mount;
> + struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
> +
> + /* numrecs verification */
> + if (be16_to_cpu(block->bb_numrecs) > max_recs)
> + return false;
> +
> + /* sibling pointer verification */
> + if (!block->bb_u.l.bb_leftsib ||
> + (block->bb_u.l.bb_leftsib != cpu_to_be64(NULLFSBLOCK) &&
> + !XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_leftsib))))
> + return false;
> + if (!block->bb_u.l.bb_rightsib ||
> + (block->bb_u.l.bb_rightsib != cpu_to_be64(NULLFSBLOCK) &&
> + !XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_rightsib))))
XFS_FSB_SANITY_CHECK() is the same thing as xfs_verify_fsbno(),
right? Perhaps they should be converted at the same time so we can
get rid of XFS_FSB_SANITY_CHECK()?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 04/13] xfs: refactor verifier callers to print address of failing check
2017-12-14 22:03 ` Dave Chinner
@ 2017-12-15 0:04 ` Darrick J. Wong
2017-12-15 3:09 ` Dave Chinner
0 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-15 0:04 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Fri, Dec 15, 2017 at 09:03:26AM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:58:37PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Refactor the callers of verifiers to print the instruction address of a
> > failing check.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> Just a quick comment about formatting as I browsed the patch...
>
> > @@ -567,13 +568,14 @@ xfs_agfl_read_verify(
> > if (!xfs_sb_version_hascrc(&mp->m_sb))
> > return;
> >
> > - if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF))
> > + if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
> > + fa = __this_address;
> > xfs_buf_ioerror(bp, -EFSBADCRC);
> > - else if (xfs_agfl_verify(bp))
> > + } else if ((fa = xfs_agfl_verify(bp)))
> > xfs_buf_ioerror(bp, -EFSCORRUPTED);
> >
> > if (bp->b_error)
> > - xfs_verifier_error(bp);
> > + xfs_verifier_error(bp, fa);
>
> We are really trying to get rid of assignments in if() statements,
> so I'd prefer we don't add a bunch of new ones. While I understand
> there's a lot of mechanical change in this patch, I'd prefer to see
> these end up as something more like:
>
> > - if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF))
> > + if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
> > + fa = __this_address;
> > xfs_buf_ioerror(bp, -EFSBADCRC);
> > - else if (xfs_agfl_verify(bp))
> > + } else if ((fa = xfs_agfl_verify(bp)))
> > xfs_buf_ioerror(bp, -EFSCORRUPTED);
>
> if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
> fa = __this_address;
> error = -EFSBADCRC;
> } else {
> fa = xfs_agfl_verify(bp);
> if (fa)
> error = -EFSCORRUPTED;
> }
>
> if (error) {
> xfs_buf_ioerror(bp, error);
> xfs_verifier_error(bp, fa);
> }
>
> .....
>
> > @@ -2459,16 +2462,18 @@ xfs_agf_read_verify(
> > struct xfs_buf *bp)
> > {
> > struct xfs_mount *mp = bp->b_target->bt_mount;
> > + xfs_failaddr_t fa;
> >
> > if (xfs_sb_version_hascrc(&mp->m_sb) &&
> > - !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF))
> > + !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF)) {
> > + fa = __this_address;
> > xfs_buf_ioerror(bp, -EFSBADCRC);
> > - else if (XFS_TEST_ERROR(xfs_agf_verify(mp, bp), mp,
> > - XFS_ERRTAG_ALLOC_READ_AGF))
> > + } else if (XFS_TEST_ERROR((fa = xfs_agf_verify(mp, bp)), mp,
> > + XFS_ERRTAG_ALLOC_READ_AGF))
> > xfs_buf_ioerror(bp, -EFSCORRUPTED);
> >
> > if (bp->b_error)
> > - xfs_verifier_error(bp);
> > + xfs_verifier_error(bp, fa);
> > }
>
> Because this sort of thing is now getting towards being unreadable.
> With the way we keep adding to verifier checks, it's only going to
> get worse if we don't take steps to clean it up...
Ok.
> > diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> > index 4c9f35d..0bbbf0b 100644
> > --- a/fs/xfs/xfs_error.c
> > +++ b/fs/xfs/xfs_error.c
> > @@ -347,13 +347,15 @@ xfs_corruption_error(
> > */
> > void
> > xfs_verifier_error(
> > - struct xfs_buf *bp)
> > + struct xfs_buf *bp,
> > + xfs_failaddr_t fa)
> > {
> > struct xfs_mount *mp = bp->b_target->bt_mount;
> >
> > xfs_alert(mp, "Metadata %s detected at %pS, %s block 0x%llx",
> > bp->b_error == -EFSBADCRC ? "CRC error" : "corruption",
> > - __return_address, bp->b_ops->name, bp->b_bn);
> > + fa ? fa : __return_address, bp->b_ops->name,
> > + bp->b_bn);
> >
> > xfs_alert(mp, "Unmount and run xfs_repair");
>
> I'm also wondering if we should move the xfs_buf_ioerror() call
> inside this function, too, rather than coding multiple calls in the
> verifiers to set bp->b_error in each branch of the verifier that
> has an error...
What, something like:
if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
fa = __this_address;
error = -EFSBADCRC;
} else {
fa = xfs_agfl_verify(bp);
if (fa)
error = -EFSCORRUPTED;
}
if (error)
xfs_verifier_error(bp, fa, error);
???
--D
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 01/13] xfs: refactor long-format btree header verification routines
2017-12-14 22:06 ` Dave Chinner
@ 2017-12-15 0:12 ` Darrick J. Wong
0 siblings, 0 replies; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-15 0:12 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Fri, Dec 15, 2017 at 09:06:14AM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:58:19PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Create two helper functions to verify the headers of a long format
> > btree block. We'll use this later for the realtime rmapbt.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> .....
>
> > +bool
> > +xfs_btree_lblock_verify(
> > + struct xfs_buf *bp,
> > + unsigned int max_recs)
> > +{
> > + struct xfs_mount *mp = bp->b_target->bt_mount;
> > + struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
> > +
> > + /* numrecs verification */
> > + if (be16_to_cpu(block->bb_numrecs) > max_recs)
> > + return false;
> > +
> > + /* sibling pointer verification */
> > + if (!block->bb_u.l.bb_leftsib ||
> > + (block->bb_u.l.bb_leftsib != cpu_to_be64(NULLFSBLOCK) &&
> > + !XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_leftsib))))
> > + return false;
> > + if (!block->bb_u.l.bb_rightsib ||
> > + (block->bb_u.l.bb_rightsib != cpu_to_be64(NULLFSBLOCK) &&
> > + !XFS_FSB_SANITY_CHECK(mp, be64_to_cpu(block->bb_u.l.bb_rightsib))))
>
> XFS_FSB_SANITY_CHECK() is the same thing as xfs_verify_fsbno(),
> right? Perhaps they should be converted at the same time so we can
> get rid of XFS_FSB_SANITY_CHECK()?
Oops, yes. Fixed.
--D
>
> Cheers,
>
> Dave.
>
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 04/13] xfs: refactor verifier callers to print address of failing check
2017-12-15 0:04 ` Darrick J. Wong
@ 2017-12-15 3:09 ` Dave Chinner
2017-12-19 20:29 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-15 3:09 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Thu, Dec 14, 2017 at 04:04:09PM -0800, Darrick J. Wong wrote:
> On Fri, Dec 15, 2017 at 09:03:26AM +1100, Dave Chinner wrote:
> > On Wed, Dec 13, 2017 at 03:58:37PM -0800, Darrick J. Wong wrote:
> > > */
> > > void
> > > xfs_verifier_error(
> > > - struct xfs_buf *bp)
> > > + struct xfs_buf *bp,
> > > + xfs_failaddr_t fa)
> > > {
> > > struct xfs_mount *mp = bp->b_target->bt_mount;
> > >
> > > xfs_alert(mp, "Metadata %s detected at %pS, %s block 0x%llx",
> > > bp->b_error == -EFSBADCRC ? "CRC error" : "corruption",
> > > - __return_address, bp->b_ops->name, bp->b_bn);
> > > + fa ? fa : __return_address, bp->b_ops->name,
> > > + bp->b_bn);
> > >
> > > xfs_alert(mp, "Unmount and run xfs_repair");
> >
> > I'm also wondering if we should move the xfs_buf_ioerror() call
> > inside this function, too, rather than coding multiple calls in the
> > verifiers to set bp->b_error in each branch of the verifier that
> > has an error...
>
> What, something like:
>
> if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
> fa = __this_address;
> error = -EFSBADCRC;
> } else {
> fa = xfs_agfl_verify(bp);
> if (fa)
> error = -EFSCORRUPTED;
> }
>
> if (error)
> xfs_verifier_error(bp, fa, error);
>
Yeah, seems like a better approach because we don't have to
put the error on the buffer before calling xfs_verifier_error(). fa
tells us exactly where the error wasdetected, so we don't need
xfs_buf_ioerror() to tell us that...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 02/13] xfs: remove XFS_WANT_CORRUPTED_RETURN from dir3 data verifiers
2017-12-13 23:58 ` [PATCH 02/13] xfs: remove XFS_WANT_CORRUPTED_RETURN from dir3 data verifiers Darrick J. Wong
@ 2017-12-19 3:50 ` Dave Chinner
0 siblings, 0 replies; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 3:50 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:58:25PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Since __xfs_dir3_data_check verifies on-disk metadata, we can't have it
> noisily blowing asserts and hanging the system on corrupt data coming in
> off the disk. Instead, have it return a boolean like all the other
> checker functions, and only have it noisily fail if we fail in debug
> mode.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks fine, though we do lose fine-grained debug information here.
I guess we get that back from the conversion to _this_address as
the return value, though...
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 03/13] xfs: have buffer verifier functions report failing address
2017-12-13 23:58 ` [PATCH 03/13] xfs: have buffer verifier functions report failing address Darrick J. Wong
@ 2017-12-19 4:12 ` Dave Chinner
2017-12-19 20:26 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 4:12 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:58:31PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Modify each function that checks the contents of a metadata buffer to
> return the instruction address of the failing test so that we can report
> more precise failure errors to the log.
> @@ -331,29 +332,29 @@ xfs_allocbt_verify(
> level = be16_to_cpu(block->bb_level);
> switch (block->bb_magic) {
> case cpu_to_be32(XFS_ABTB_CRC_MAGIC):
> - if (!xfs_btree_sblock_v5hdr_verify(bp))
> - return false;
> + if ((fa = xfs_btree_sblock_v5hdr_verify(bp)))
> + return fa;
I see a few of these.
fa = xfs_btree_sblock_v5hdr_verify(bp);
if (fa)
return fa;
> @@ -4609,7 +4609,7 @@ xfs_btree_sblock_v5hdr_verify(
> * @bp: buffer containing the btree block
> * @max_recs: maximum records allowed in this btree node
> */
> -bool
> +xfs_failaddr_t
> xfs_btree_sblock_verify(
> struct xfs_buf *bp,
> unsigned int max_recs)
> @@ -4619,19 +4619,19 @@ xfs_btree_sblock_verify(
>
> /* numrecs verification */
> if (be16_to_cpu(block->bb_numrecs) > max_recs)
> - return false;
> + return __this_address;
>
> /* sibling pointer verification */
> if (!block->bb_u.s.bb_leftsib ||
> (be32_to_cpu(block->bb_u.s.bb_leftsib) >= mp->m_sb.sb_agblocks &&
> block->bb_u.s.bb_leftsib != cpu_to_be32(NULLAGBLOCK)))
> - return false;
> + return __this_address;
> if (!block->bb_u.s.bb_rightsib ||
> (be32_to_cpu(block->bb_u.s.bb_rightsib) >= mp->m_sb.sb_agblocks &&
> block->bb_u.s.bb_rightsib != cpu_to_be32(NULLAGBLOCK)))
> - return false;
> + return __this_address;
Out if scope for this patch, but like th e lblock verifying, I think
these are candidates for xfs_verify_agbno()?
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index 27297a6..87565b6 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -73,13 +73,13 @@ xfs_dir3_leaf1_check(
> } else if (leafhdr.magic != XFS_DIR2_LEAF1_MAGIC)
> return false;
>
> - return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf);
> + return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf) == NULL;
> }
Hmmmm - shouldn't we make xfs_dir3_leaf_check() dump the failaddr
so we know what debug check failed? This would improve the debug
failure reporting in this code, and seeing as we now have the info it
doesn't really make sense to throw it away without reporting it...
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index 682e2bf..00ded0a 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -76,13 +76,13 @@ xfs_dir3_leafn_check(
> } else if (leafhdr.magic != XFS_DIR2_LEAFN_MAGIC)
> return false;
>
> - return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf);
> + return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf) == NULL;
> }
> #else
> #define xfs_dir3_leaf_check(dp, bp)
> #endif
Same here.
> @@ -93,21 +93,21 @@ xfs_dir3_free_verify(
> struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
>
> if (hdr3->magic != cpu_to_be32(XFS_DIR3_FREE_MAGIC))
> - return false;
> + return __this_address;
> if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
> - return false;
> + return __this_address;
> if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
> - return false;
> + return __this_address;
> if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
> - return false;
> + return __this_address;
> } else {
> if (hdr->magic != cpu_to_be32(XFS_DIR2_FREE_MAGIC))
> - return false;
> + return __this_address;
> }
/me wonders if there is opportunity for factoring this header check
seeing as it's repeated a few times, just with different magic
numbers...
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index 45c68d0..c441efb 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -41,7 +41,7 @@ extern int xfs_dir2_leaf_to_block(struct xfs_da_args *args,
> #ifdef DEBUG
> #define xfs_dir3_data_check(dp, bp) \
> do { \
> - if (!__xfs_dir3_data_check((dp), (bp))) { \
> + if (__xfs_dir3_data_check((dp), (bp))) { \
> XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, \
> (bp)->b_target->bt_mount, (bp)->b_addr); \
> } \
This should capture the failaddr and dump it as part of the
corruption error, I think. Otherwise we lose what part of the
structure was considered corrupt from the error report.
> @@ -507,7 +507,7 @@ xfs_iread(
> return error;
>
> /* even unallocated inodes are verified */
> - if (!xfs_dinode_verify(mp, ip->i_ino, dip)) {
> + if (xfs_dinode_verify(mp, ip->i_ino, dip)) {
> xfs_alert(mp, "%s: validation failed for inode %lld",
> __func__, ip->i_ino);
Capture the failaddr in the error message?
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index a9c97a3..2317511 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -82,7 +82,7 @@ void xfs_inobp_check(struct xfs_mount *, struct xfs_buf *);
> #define xfs_inobp_check(mp, bp)
> #endif /* DEBUG */
>
> -bool xfs_dinode_verify(struct xfs_mount *mp, xfs_ino_t ino,
> - struct xfs_dinode *dip);
> +void *xfs_dinode_verify(struct xfs_mount *mp, xfs_ino_t ino,
> + struct xfs_dinode *dip);
xfs_failaddr_t?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 05/13] xfs: verify dinode header first
2017-12-13 23:58 ` [PATCH 05/13] xfs: verify dinode header first Darrick J. Wong
@ 2017-12-19 4:13 ` Dave Chinner
0 siblings, 0 replies; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 4:13 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:58:43PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Move the v3 inode integrity information (crc, owner, metauuid) before we
> look at anything else in the inode so that we don't waste time on a torn
> write or a totally garbled block. This makes xfs_dinode_verify more
> consistent with the other verifiers.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 06/13] xfs: move inode fork verifiers to xfs_dinode_verify
2017-12-13 23:58 ` [PATCH 06/13] xfs: move inode fork verifiers to xfs_dinode_verify Darrick J. Wong
@ 2017-12-19 5:16 ` Dave Chinner
2017-12-19 20:34 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 5:16 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:58:49PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Consolidate the fork size and format verifiers to xfs_dinode_verify so
> that we can reject bad inodes earlier and in a single place.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_inode_buf.c | 72 ++++++++++++++++++++++++++++++++-
> fs/xfs/libxfs/xfs_inode_fork.c | 86 ----------------------------------------
> 2 files changed, 69 insertions(+), 89 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 0e4c720..1392fe9 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -390,6 +390,7 @@ xfs_dinode_verify(
> uint16_t mode;
> uint16_t flags;
> uint64_t flags2;
> + uint64_t di_size;
>
> if (dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC))
> return __this_address;
> @@ -408,7 +409,8 @@ xfs_dinode_verify(
> }
>
> /* don't allow invalid i_size */
> - if (be64_to_cpu(dip->di_size) & (1ULL << 63))
> + di_size = be64_to_cpu(dip->di_size);
> + if (di_size & (1ULL << 63))
> return __this_address;
>
> mode = be16_to_cpu(dip->di_mode);
> @@ -416,14 +418,74 @@ xfs_dinode_verify(
> return __this_address;
>
> /* No zero-length symlinks/dirs. */
> - if ((S_ISLNK(mode) || S_ISDIR(mode)) && dip->di_size == 0)
> + if ((S_ISLNK(mode) || S_ISDIR(mode)) && di_size == 0)
> return __this_address;
>
> + /* Fork checks carried over from xfs_iformat_fork */
> + if (mode &&
> + be32_to_cpu(dip->di_nextents) + be16_to_cpu(dip->di_anextents) >
> + be64_to_cpu(dip->di_nblocks))
Can you indent this last line so it doesn't look like a spearate
logic check?
if (mode &&
be32_to_cpu(dip->di_nextents) + be16_to_cpu(dip->di_anextents) >
be64_to_cpu(dip->di_nblocks))
> + return __this_address;
> +
> + if (mode && dip->di_forkoff > mp->m_sb.sb_inodesize)
> + return __this_address;
Hold on, this check is completely bogus. di_forkoff is in units of 8 bytes,
which inode size is in bytes. Also, di_forkoff is a u8, so it can't
/ever/ be larger than the inode size which are >= 256 bytes. Yeah,
though so:
#define XFS_DFORK_BOFF(dip) ((int)((dip)->di_forkoff << 3))
This check needs to be:
if (mode && XFS_DFORK_BOFF(dip) > mp->m_sb.sb_inodesize)
return __this_address;
Otherwise looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 07/13] xfs: create structure verifier function for shortform xattrs
2017-12-13 23:58 ` [PATCH 07/13] xfs: create structure verifier function for shortform xattrs Darrick J. Wong
@ 2017-12-19 5:23 ` Dave Chinner
2017-12-19 20:41 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 5:23 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:58:55PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Create a function to perform structure verification for short form
> extended attributes.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_attr_leaf.c | 70 +++++++++++++++++++++++++++++++++++++++++
> fs/xfs/libxfs/xfs_attr_leaf.h | 1 +
> 2 files changed, 71 insertions(+)
>
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 1274872..a5033f0 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -873,6 +873,76 @@ xfs_attr_shortform_allfit(
> return xfs_attr_shortform_bytesfit(dp, bytes);
> }
>
> +/* Verify the consistency of an inline attribute fork. */
> +xfs_failaddr_t
> +xfs_attr_shortform_verify(
> + struct xfs_inode *ip)
> +{
> + struct xfs_attr_shortform *sfp;
> + struct xfs_attr_sf_entry *sfep;
> + struct xfs_attr_sf_entry *next_sfep;
> + char *endp;
> + struct xfs_ifork *ifp;
> + int i;
> + int size;
> +
> + ASSERT(ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL);
> + ifp = XFS_IFORK_PTR(ip, XFS_ATTR_FORK);
> + sfp = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
> + size = ifp->if_bytes;
> +
> + /*
> + * Give up if the attribute is way too short.
> + */
> + if (size < sizeof(struct xfs_attr_sf_hdr))
> + return __this_address;
> +
> + endp = (char *)sfp + size;
> +
> + /* Check all reported entries */
> + sfep = &sfp->list[0];
> + for (i = 0; i < sfp->hdr.count; i++) {
> + /*
> + * struct xfs_attr_sf_entry has a variable length.
> + * Check the fixed-offset parts of the structure are
> + * within the data buffer.
> + */
> + if (((char *)sfep + sizeof(*sfep)) >= endp)
> + return __this_address;
> +
> + /* Don't allow names with known bad length. */
> + if (sfep->namelen == 0)
> + return __this_address;
> +
> + /*
> + * Check that the variable-length part of the structure is
> + * within the data buffer. The next entry starts after the
> + * name component, so nextentry is an acceptable test.
> + */
> + next_sfep = XFS_ATTR_SF_NEXTENTRY(sfep);
> + if ((char *)next_sfep > endp)
> + return __this_address;
> +
> + /*
> + * Check for unknown flags. Short form doesn't support
> + * the incomplete or local bits.
> + */
> + if (sfep->flags & ~(XFS_ATTR_ROOT | XFS_ATTR_SECURE))
> + return __this_address;
> +
> + /* Check for invalid namespace combinations. */
> + if ((sfep->flags & XFS_ATTR_ROOT) &&
> + (sfep->flags & XFS_ATTR_SECURE))
> + return __this_address;
#define ATTR_SF_FLAGS (XFS_ATTR_ROOT | XFS_ATTR_SECURE)
if (sfep->flags & ~ATTR_SF_FLAGS)
return __this_address;
if ((sfep->flags & ATTR_SF_FLAGS) == ATTR_SF_FLAGS)
return __this_address;
Otherwise looks ok.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 08/13] xfs: create structure verifier function for short form symlinks
2017-12-13 23:59 ` [PATCH 08/13] xfs: create structure verifier function for short form symlinks Darrick J. Wong
@ 2017-12-19 5:27 ` Dave Chinner
2017-12-19 20:45 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 5:27 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:59:01PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Create a function to check the structure of short form symlink targets.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_shared.h | 1 +
> fs/xfs/libxfs/xfs_symlink_remote.c | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 37 insertions(+)
>
>
> diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h
> index c6f4eb4..67ccb1a 100644
> --- a/fs/xfs/libxfs/xfs_shared.h
> +++ b/fs/xfs/libxfs/xfs_shared.h
> @@ -143,5 +143,6 @@ bool xfs_symlink_hdr_ok(xfs_ino_t ino, uint32_t offset,
> uint32_t size, struct xfs_buf *bp);
> void xfs_symlink_local_to_remote(struct xfs_trans *tp, struct xfs_buf *bp,
> struct xfs_inode *ip, struct xfs_ifork *ifp);
> +xfs_failaddr_t xfs_symlink_shortform_verify(struct xfs_inode *ip);
>
> #endif /* __XFS_SHARED_H__ */
> diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
> index b6ef6cb..5d9642e 100644
> --- a/fs/xfs/libxfs/xfs_symlink_remote.c
> +++ b/fs/xfs/libxfs/xfs_symlink_remote.c
> @@ -210,3 +210,39 @@ xfs_symlink_local_to_remote(
> xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsymlink_hdr) +
> ifp->if_bytes - 1);
> }
> +
> +/* Verify the consistency of an inline symlink. */
> +xfs_failaddr_t
> +xfs_symlink_shortform_verify(
> + struct xfs_inode *ip)
> +{
> + char *sfp;
> + char *endp;
> + struct xfs_ifork *ifp;
> + int size;
> +
> + ASSERT(ip->i_d.di_format == XFS_DINODE_FMT_LOCAL);
> + ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
> + sfp = (char *)ifp->if_u1.if_data;
> + size = ifp->if_bytes;
> + endp = sfp + size;
> +
> + /* Zero length symlinks can exist while we're deleting a remote one. */
> + if (size == 0)
> + return NULL;
> +
> + /* No negative sizes or overly long symlink targets. */
> + if (size < 0 || size > XFS_SYMLINK_MAXLEN)
> + return __this_address;
> +
> + /* No NULLs in the target either. */
> + for (; sfp < endp; sfp++) {
> + if (*sfp == 0)
> + return __this_address;
> + }
if (memchr(sfp, 0, size - 1))
return __this_address;
Otherwise looks ok.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 09/13] xfs: refactor short form directory structure verifier function
2017-12-13 23:59 ` [PATCH 09/13] xfs: refactor short form directory structure verifier function Darrick J. Wong
@ 2017-12-19 5:45 ` Dave Chinner
0 siblings, 0 replies; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 5:45 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:59:07PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Change the short form directory structure verifier function to return
> the instruction pointer of a failing check or NULL if everything's ok.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_dir2_priv.h | 2 +-
> fs/xfs/libxfs/xfs_dir2_sf.c | 26 +++++++++++++-------------
> fs/xfs/libxfs/xfs_inode_fork.c | 5 ++---
> 3 files changed, 16 insertions(+), 17 deletions(-)
Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 10/13] xfs: provide a centralized method for verifying inline fork data
2017-12-13 23:59 ` [PATCH 10/13] xfs: provide a centralized method for verifying inline fork data Darrick J. Wong
@ 2017-12-19 6:06 ` Dave Chinner
2017-12-19 20:50 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 6:06 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:59:13PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Replace the current haphazard dir2 shortform verifier callsites with a
> centralized verifier function that can be called either with the default
> verifier functions or with a custom set. This helps us strengthen
> integrity checking while providing us with flexibility for repair tools.
>
> xfs_repair wants this to be able to supply its own verifier functions
> when trying to fix possibly corrupt metadata.
I was about to say "this looks a little over-engineered" then I read
the commit message.... :P
Ok, so it pulls the fork verication out of the libxfs inode read code,
and instead slots it into the kernel inode cache miss path just
after the inode has been read. Same effect - newly read inodes get
their forks verified before use. ANd they same thing happens in
xfs_iflush_int(), which is private to the kernel anyway....
> +
> +/* Default fork content verifiers. */
> +struct xfs_ifork_ops xfs_default_ifork_ops = {
> + .verify_attr = xfs_attr_shortform_verify,
> + .verify_dir = xfs_dir2_sf_verify,
> + .verify_symlink = xfs_symlink_shortform_verify,
> +};
> +
> +/* Verify the inline contents of the data fork of an inode. */
> +xfs_failaddr_t
> +xfs_ifork_verify_data(
> + struct xfs_inode *ip,
> + struct xfs_ifork_ops *ops)
> +{
> + xfs_ifork_verifier_t fn = NULL;
> +
> + /* Non-local data fork, we're done. */
> + if (ip->i_d.di_format != XFS_DINODE_FMT_LOCAL)
> + return NULL;
> +
> + /* Check the inline data fork if there is one. */
> + if (S_ISDIR(VFS_I(ip)->i_mode))
> + fn = ops->verify_dir;
> + else if (S_ISLNK(VFS_I(ip)->i_mode))
> + fn = ops->verify_symlink;
> +
> + if (fn)
> + return fn(ip);
> + return NULL;
> +}
Seems a little convoluted, especially if we grow more cases. This
seems simpler already:
switch(VFS_I(ip)->i_mode & S_IFMT) {
case S_IFDIR:
return ops->verify_dir(ip);
case S_IFLNK:
return ops->verify_symlink(ip);
default:
break;
}
return NULL;
> +
> +/* Verify the inline contents of the attr fork of an inode. */
> +xfs_failaddr_t
> +xfs_ifork_verify_attr(
> + struct xfs_inode *ip,
> + struct xfs_ifork_ops *ops)
> +{
> + xfs_ifork_verifier_t fn = NULL;
> +
> + /* There has to be an attr fork allocated if aformat is local. */
> + if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
> + if (XFS_IFORK_PTR(ip, XFS_ATTR_FORK))
> + fn = ops->verify_attr;
> + else
> + return __this_address;
> + }
> +
> + if (fn)
> + return fn(ip);
> + return NULL;
And this could be stright lined as:
if (ip->i_d.di_aformat != XFS_DINODE_FMT_LOCAL)
return NULL;
if (!XFS_IFORK_PTR(ip, XFS_ATTR_FORK))
return __this_address;
return ops->verify_attr(ip);
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/13] xfs: fail out of xfs_attr3_leaf_lookup_int if it looks corrupt
2017-12-13 23:59 ` [PATCH 11/13] xfs: fail out of xfs_attr3_leaf_lookup_int if it looks corrupt Darrick J. Wong
@ 2017-12-19 6:13 ` Dave Chinner
0 siblings, 0 replies; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 6:13 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:59:19PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> If the xattr leaf block looks corrupt, return -EFSCORRUPTED to userspace
> instead of ASSERTing on debug kernels or running off the end of the
> buffer on regular kernels.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_attr_leaf.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index a5033f0..e130952 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -2246,7 +2246,8 @@ xfs_attr3_leaf_lookup_int(
> leaf = bp->b_addr;
> xfs_attr3_leaf_hdr_from_disk(args->geo, &ichdr, leaf);
> entries = xfs_attr3_leaf_entryp(leaf);
> - ASSERT(ichdr.count < args->geo->blksize / 8);
> + if (ichdr.count >= args->geo->blksize / 8)
> + return -EFSCORRUPTED;
I was wondering exactly what the "/ 8" is for, but I think it's just
a rough calculation of maximum number of leaf entries taht can be in
the block. Doesn't matter right now - this is still better than
assert failures....
So, look fine,
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 12/13] xfs: create a new buf_ops pointer to verify structure metadata
2017-12-13 23:59 ` [PATCH 12/13] xfs: create a new buf_ops pointer to verify structure metadata Darrick J. Wong
@ 2017-12-19 6:22 ` Dave Chinner
2017-12-19 18:15 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 6:22 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:59:25PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Expose all metadata structure buffer verifier functions via buf_ops.
> These will be used by the online scrub mechanism to look for problems
> with buffers that are already sitting around in memory.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
....
> @@ -2468,7 +2478,7 @@ xfs_agf_read_verify(
> .verify_write = xfs_attr3_leaf_write_verify,
> + .verify_struct = xfs_attr3_leaf_verify,
> };
>
> int
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index d4d2902..1be995b 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -204,10 +204,42 @@ xfs_attr3_rmt_write_verify(
> ASSERT(len == 0);
> }
>
> +static xfs_failaddr_t
> +xfs_attr3_rmt_verify_struct(
> + struct xfs_buf *bp)
> +{
> + struct xfs_mount *mp = bp->b_target->bt_mount;
> + char *ptr;
> + void *failed_at;
> + int len;
> + xfs_daddr_t bno;
> + int blksize = mp->m_attr_geo->blksize;
> +
> + /* no verification of non-crc buffers */
> + if (!xfs_sb_version_hascrc(&mp->m_sb))
> + return NULL;
> +
> + ptr = bp->b_addr;
> + bno = bp->b_bn;
> + len = BBTOB(bp->b_length);
> + ASSERT(len >= blksize);
> +
> + while (len > 0) {
> + if ((failed_at = xfs_attr3_rmt_verify(mp, ptr, blksize, bno)))
> + return failed_at;
> + len -= blksize;
> + ptr += blksize;
> + bno += BTOBB(blksize);
> + }
> +
> + return NULL;
> +}
I'd much prefer to see this combined with
xfs_attr3_rmt_read_verify() rather than having another copy of this
iteration code. They really only vary by whether the CRC is checked
in the loop....
....
> +static xfs_failaddr_t
> +xfs_dquot_buf_verify_struct(
> + struct xfs_buf *bp)
> +{
> + struct xfs_mount *mp = bp->b_target->bt_mount;
> +
> + if (!xfs_dquot_buf_verify(mp, bp, 0))
> + return __this_address;
> + return NULL;
> +}
I can't remember what happened exactly with dquot buffers earlire in
the patchset, but why isn't it returning a failaddr like all the
other structure verifiers?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 13/13] xfs: scrub in-core metadata
2017-12-13 23:59 ` [PATCH 13/13] xfs: scrub in-core metadata Darrick J. Wong
@ 2017-12-19 6:23 ` Dave Chinner
0 siblings, 0 replies; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 6:23 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Dec 13, 2017 at 03:59:32PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Whenever we load a buffer, explicitly re-call the structure verifier to
> ensure that memory isn't corrupting things.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/scrub/agheader.c | 3 +++
> fs/xfs/scrub/btree.c | 4 ++++
> fs/xfs/scrub/common.c | 23 +++++++++++++++++++++++
> fs/xfs/scrub/common.h | 2 ++
> fs/xfs/scrub/dabtree.c | 22 ++++++++++++++++++++++
> fs/xfs/scrub/dir.c | 4 ++++
> 6 files changed, 58 insertions(+)
Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 12/13] xfs: create a new buf_ops pointer to verify structure metadata
2017-12-19 6:22 ` Dave Chinner
@ 2017-12-19 18:15 ` Darrick J. Wong
2017-12-19 20:53 ` Dave Chinner
0 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-19 18:15 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 05:22:05PM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:59:25PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Expose all metadata structure buffer verifier functions via buf_ops.
> > These will be used by the online scrub mechanism to look for problems
> > with buffers that are already sitting around in memory.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> ....
> > @@ -2468,7 +2478,7 @@ xfs_agf_read_verify(
> > .verify_write = xfs_attr3_leaf_write_verify,
> > + .verify_struct = xfs_attr3_leaf_verify,
> > };
> >
> > int
> > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> > index d4d2902..1be995b 100644
> > --- a/fs/xfs/libxfs/xfs_attr_remote.c
> > +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> > @@ -204,10 +204,42 @@ xfs_attr3_rmt_write_verify(
> > ASSERT(len == 0);
> > }
> >
> > +static xfs_failaddr_t
> > +xfs_attr3_rmt_verify_struct(
> > + struct xfs_buf *bp)
> > +{
> > + struct xfs_mount *mp = bp->b_target->bt_mount;
> > + char *ptr;
> > + void *failed_at;
> > + int len;
> > + xfs_daddr_t bno;
> > + int blksize = mp->m_attr_geo->blksize;
> > +
> > + /* no verification of non-crc buffers */
> > + if (!xfs_sb_version_hascrc(&mp->m_sb))
> > + return NULL;
> > +
> > + ptr = bp->b_addr;
> > + bno = bp->b_bn;
> > + len = BBTOB(bp->b_length);
> > + ASSERT(len >= blksize);
> > +
> > + while (len > 0) {
> > + if ((failed_at = xfs_attr3_rmt_verify(mp, ptr, blksize, bno)))
> > + return failed_at;
> > + len -= blksize;
> > + ptr += blksize;
> > + bno += BTOBB(blksize);
> > + }
> > +
> > + return NULL;
> > +}
>
> I'd much prefer to see this combined with
> xfs_attr3_rmt_read_verify() rather than having another copy of this
> iteration code. They really only vary by whether the CRC is checked
> in the loop....
>
> ....
>
> > +static xfs_failaddr_t
> > +xfs_dquot_buf_verify_struct(
> > + struct xfs_buf *bp)
> > +{
> > + struct xfs_mount *mp = bp->b_target->bt_mount;
> > +
> > + if (!xfs_dquot_buf_verify(mp, bp, 0))
> > + return __this_address;
> > + return NULL;
> > +}
>
> I can't remember what happened exactly with dquot buffers earlire in
> the patchset, but why isn't it returning a failaddr like all the
> other structure verifiers?
The dquot verifiers are sufficiently different from everything else
(verbose error reporting, some ability to zap garbage data) that I was
going to send that as a separate cleanup series.
I /think/ the solution is to disentangle xfs_dqcheck into a separate
check routine that returns xfs_failaddr_t like everything else (at a
cost of the removal of all the xfs_alert calls) so that the error
reports become "xfs: quota buffer XXX error at xfs_dqcheck+0x74" like
everything else. The caller becomes directly responsible for printing a
warning message (instead of XFS_QMOPT_DOWARN).
Then, the quota repair piece becomes a separate function which the
XFS_QMOPT_DQREPAIR callers can call directly.
Sound good?
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 03/13] xfs: have buffer verifier functions report failing address
2017-12-19 4:12 ` Dave Chinner
@ 2017-12-19 20:26 ` Darrick J. Wong
0 siblings, 0 replies; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-19 20:26 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 03:12:38PM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:58:31PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Modify each function that checks the contents of a metadata buffer to
> > return the instruction address of the failing test so that we can report
> > more precise failure errors to the log.
>
>
> > @@ -331,29 +332,29 @@ xfs_allocbt_verify(
> > level = be16_to_cpu(block->bb_level);
> > switch (block->bb_magic) {
> > case cpu_to_be32(XFS_ABTB_CRC_MAGIC):
> > - if (!xfs_btree_sblock_v5hdr_verify(bp))
> > - return false;
> > + if ((fa = xfs_btree_sblock_v5hdr_verify(bp)))
> > + return fa;
>
> I see a few of these.
>
> fa = xfs_btree_sblock_v5hdr_verify(bp);
> if (fa)
> return fa;
Fixed.
> > @@ -4609,7 +4609,7 @@ xfs_btree_sblock_v5hdr_verify(
> > * @bp: buffer containing the btree block
> > * @max_recs: maximum records allowed in this btree node
> > */
> > -bool
> > +xfs_failaddr_t
> > xfs_btree_sblock_verify(
> > struct xfs_buf *bp,
> > unsigned int max_recs)
> > @@ -4619,19 +4619,19 @@ xfs_btree_sblock_verify(
> >
> > /* numrecs verification */
> > if (be16_to_cpu(block->bb_numrecs) > max_recs)
> > - return false;
> > + return __this_address;
> >
> > /* sibling pointer verification */
> > if (!block->bb_u.s.bb_leftsib ||
> > (be32_to_cpu(block->bb_u.s.bb_leftsib) >= mp->m_sb.sb_agblocks &&
> > block->bb_u.s.bb_leftsib != cpu_to_be32(NULLAGBLOCK)))
> > - return false;
> > + return __this_address;
> > if (!block->bb_u.s.bb_rightsib ||
> > (be32_to_cpu(block->bb_u.s.bb_rightsib) >= mp->m_sb.sb_agblocks &&
> > block->bb_u.s.bb_rightsib != cpu_to_be32(NULLAGBLOCK)))
> > - return false;
> > + return __this_address;
>
> Out if scope for this patch, but like th e lblock verifying, I think
> these are candidates for xfs_verify_agbno()?
Yep. Added another patch to fix this.
> > diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> > index 27297a6..87565b6 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> > +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> > @@ -73,13 +73,13 @@ xfs_dir3_leaf1_check(
> > } else if (leafhdr.magic != XFS_DIR2_LEAF1_MAGIC)
> > return false;
> >
> > - return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf);
> > + return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf) == NULL;
> > }
>
> Hmmmm - shouldn't we make xfs_dir3_leaf_check() dump the failaddr
> so we know what debug check failed? This would improve the debug
> failure reporting in this code, and seeing as we now have the info it
> doesn't really make sense to throw it away without reporting it...
Fixed.
> > diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> > index 682e2bf..00ded0a 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_node.c
> > +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> > @@ -76,13 +76,13 @@ xfs_dir3_leafn_check(
> > } else if (leafhdr.magic != XFS_DIR2_LEAFN_MAGIC)
> > return false;
> >
> > - return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf);
> > + return xfs_dir3_leaf_check_int(dp->i_mount, dp, &leafhdr, leaf) == NULL;
> > }
> > #else
> > #define xfs_dir3_leaf_check(dp, bp)
> > #endif
>
> Same here.
Fixed.
> > @@ -93,21 +93,21 @@ xfs_dir3_free_verify(
> > struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
> >
> > if (hdr3->magic != cpu_to_be32(XFS_DIR3_FREE_MAGIC))
> > - return false;
> > + return __this_address;
> > if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
> > - return false;
> > + return __this_address;
> > if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
> > - return false;
> > + return __this_address;
> > if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
> > - return false;
> > + return __this_address;
> > } else {
> > if (hdr->magic != cpu_to_be32(XFS_DIR2_FREE_MAGIC))
> > - return false;
> > + return __this_address;
> > }
>
> /me wonders if there is opportunity for factoring this header check
> seeing as it's repeated a few times, just with different magic
> numbers...
>
> > diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> > index 45c68d0..c441efb 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> > +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> > @@ -41,7 +41,7 @@ extern int xfs_dir2_leaf_to_block(struct xfs_da_args *args,
> > #ifdef DEBUG
> > #define xfs_dir3_data_check(dp, bp) \
> > do { \
> > - if (!__xfs_dir3_data_check((dp), (bp))) { \
> > + if (__xfs_dir3_data_check((dp), (bp))) { \
> > XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, \
> > (bp)->b_target->bt_mount, (bp)->b_addr); \
> > } \
>
> This should capture the failaddr and dump it as part of the
> corruption error, I think. Otherwise we lose what part of the
> structure was considered corrupt from the error report.
Ok.
> > @@ -507,7 +507,7 @@ xfs_iread(
> > return error;
> >
> > /* even unallocated inodes are verified */
> > - if (!xfs_dinode_verify(mp, ip->i_ino, dip)) {
> > + if (xfs_dinode_verify(mp, ip->i_ino, dip)) {
> > xfs_alert(mp, "%s: validation failed for inode %lld",
> > __func__, ip->i_ino);
>
> Capture the failaddr in the error message?
Ok.
> > diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> > index a9c97a3..2317511 100644
> > --- a/fs/xfs/libxfs/xfs_inode_buf.h
> > +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> > @@ -82,7 +82,7 @@ void xfs_inobp_check(struct xfs_mount *, struct xfs_buf *);
> > #define xfs_inobp_check(mp, bp)
> > #endif /* DEBUG */
> >
> > -bool xfs_dinode_verify(struct xfs_mount *mp, xfs_ino_t ino,
> > - struct xfs_dinode *dip);
> > +void *xfs_dinode_verify(struct xfs_mount *mp, xfs_ino_t ino,
> > + struct xfs_dinode *dip);
>
> xfs_failaddr_t?
Fixed.
--D
> Cheers,
>
> Dave.
>
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 04/13] xfs: refactor verifier callers to print address of failing check
2017-12-15 3:09 ` Dave Chinner
@ 2017-12-19 20:29 ` Darrick J. Wong
0 siblings, 0 replies; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-19 20:29 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Fri, Dec 15, 2017 at 02:09:10PM +1100, Dave Chinner wrote:
> On Thu, Dec 14, 2017 at 04:04:09PM -0800, Darrick J. Wong wrote:
> > On Fri, Dec 15, 2017 at 09:03:26AM +1100, Dave Chinner wrote:
> > > On Wed, Dec 13, 2017 at 03:58:37PM -0800, Darrick J. Wong wrote:
> > > > */
> > > > void
> > > > xfs_verifier_error(
> > > > - struct xfs_buf *bp)
> > > > + struct xfs_buf *bp,
> > > > + xfs_failaddr_t fa)
> > > > {
> > > > struct xfs_mount *mp = bp->b_target->bt_mount;
> > > >
> > > > xfs_alert(mp, "Metadata %s detected at %pS, %s block 0x%llx",
> > > > bp->b_error == -EFSBADCRC ? "CRC error" : "corruption",
> > > > - __return_address, bp->b_ops->name, bp->b_bn);
> > > > + fa ? fa : __return_address, bp->b_ops->name,
> > > > + bp->b_bn);
> > > >
> > > > xfs_alert(mp, "Unmount and run xfs_repair");
> > >
> > > I'm also wondering if we should move the xfs_buf_ioerror() call
> > > inside this function, too, rather than coding multiple calls in the
> > > verifiers to set bp->b_error in each branch of the verifier that
> > > has an error...
> >
> > What, something like:
> >
> > if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF)) {
> > fa = __this_address;
> > error = -EFSBADCRC;
> > } else {
> > fa = xfs_agfl_verify(bp);
> > if (fa)
> > error = -EFSCORRUPTED;
> > }
> >
> > if (error)
> > xfs_verifier_error(bp, fa, error);
> >
>
> Yeah, seems like a better approach because we don't have to
> put the error on the buffer before calling xfs_verifier_error(). fa
> tells us exactly where the error wasdetected, so we don't need
> xfs_buf_ioerror() to tell us that...
Ok, but I'll do this as a separate patch.
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 06/13] xfs: move inode fork verifiers to xfs_dinode_verify
2017-12-19 5:16 ` Dave Chinner
@ 2017-12-19 20:34 ` Darrick J. Wong
2017-12-19 20:48 ` Dave Chinner
0 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-19 20:34 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 04:16:19PM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:58:49PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Consolidate the fork size and format verifiers to xfs_dinode_verify so
> > that we can reject bad inodes earlier and in a single place.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/libxfs/xfs_inode_buf.c | 72 ++++++++++++++++++++++++++++++++-
> > fs/xfs/libxfs/xfs_inode_fork.c | 86 ----------------------------------------
> > 2 files changed, 69 insertions(+), 89 deletions(-)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> > index 0e4c720..1392fe9 100644
> > --- a/fs/xfs/libxfs/xfs_inode_buf.c
> > +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> > @@ -390,6 +390,7 @@ xfs_dinode_verify(
> > uint16_t mode;
> > uint16_t flags;
> > uint64_t flags2;
> > + uint64_t di_size;
> >
> > if (dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC))
> > return __this_address;
> > @@ -408,7 +409,8 @@ xfs_dinode_verify(
> > }
> >
> > /* don't allow invalid i_size */
> > - if (be64_to_cpu(dip->di_size) & (1ULL << 63))
> > + di_size = be64_to_cpu(dip->di_size);
> > + if (di_size & (1ULL << 63))
> > return __this_address;
> >
> > mode = be16_to_cpu(dip->di_mode);
> > @@ -416,14 +418,74 @@ xfs_dinode_verify(
> > return __this_address;
> >
> > /* No zero-length symlinks/dirs. */
> > - if ((S_ISLNK(mode) || S_ISDIR(mode)) && dip->di_size == 0)
> > + if ((S_ISLNK(mode) || S_ISDIR(mode)) && di_size == 0)
> > return __this_address;
> >
> > + /* Fork checks carried over from xfs_iformat_fork */
> > + if (mode &&
> > + be32_to_cpu(dip->di_nextents) + be16_to_cpu(dip->di_anextents) >
> > + be64_to_cpu(dip->di_nblocks))
>
> Can you indent this last line so it doesn't look like a spearate
> logic check?
Done.
> if (mode &&
> be32_to_cpu(dip->di_nextents) + be16_to_cpu(dip->di_anextents) >
> be64_to_cpu(dip->di_nblocks))
> > + return __this_address;
> > +
> > + if (mode && dip->di_forkoff > mp->m_sb.sb_inodesize)
> > + return __this_address;
>
> Hold on, this check is completely bogus. di_forkoff is in units of 8 bytes,
> which inode size is in bytes. Also, di_forkoff is a u8, so it can't
> /ever/ be larger than the inode size which are >= 256 bytes. Yeah,
> though so:
>
> #define XFS_DFORK_BOFF(dip) ((int)((dip)->di_forkoff << 3))
>
> This check needs to be:
>
> if (mode && XFS_DFORK_BOFF(dip) > mp->m_sb.sb_inodesize)
> return __this_address;
>
> Otherwise looks good.
Oops, thank you for catching that!
--D
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 07/13] xfs: create structure verifier function for shortform xattrs
2017-12-19 5:23 ` Dave Chinner
@ 2017-12-19 20:41 ` Darrick J. Wong
2017-12-19 20:51 ` Dave Chinner
0 siblings, 1 reply; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-19 20:41 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 04:23:29PM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:58:55PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Create a function to perform structure verification for short form
> > extended attributes.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/libxfs/xfs_attr_leaf.c | 70 +++++++++++++++++++++++++++++++++++++++++
> > fs/xfs/libxfs/xfs_attr_leaf.h | 1 +
> > 2 files changed, 71 insertions(+)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> > index 1274872..a5033f0 100644
> > --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> > @@ -873,6 +873,76 @@ xfs_attr_shortform_allfit(
> > return xfs_attr_shortform_bytesfit(dp, bytes);
> > }
> >
> > +/* Verify the consistency of an inline attribute fork. */
> > +xfs_failaddr_t
> > +xfs_attr_shortform_verify(
> > + struct xfs_inode *ip)
> > +{
> > + struct xfs_attr_shortform *sfp;
> > + struct xfs_attr_sf_entry *sfep;
> > + struct xfs_attr_sf_entry *next_sfep;
> > + char *endp;
> > + struct xfs_ifork *ifp;
> > + int i;
> > + int size;
> > +
> > + ASSERT(ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL);
> > + ifp = XFS_IFORK_PTR(ip, XFS_ATTR_FORK);
> > + sfp = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
> > + size = ifp->if_bytes;
> > +
> > + /*
> > + * Give up if the attribute is way too short.
> > + */
> > + if (size < sizeof(struct xfs_attr_sf_hdr))
> > + return __this_address;
> > +
> > + endp = (char *)sfp + size;
> > +
> > + /* Check all reported entries */
> > + sfep = &sfp->list[0];
> > + for (i = 0; i < sfp->hdr.count; i++) {
> > + /*
> > + * struct xfs_attr_sf_entry has a variable length.
> > + * Check the fixed-offset parts of the structure are
> > + * within the data buffer.
> > + */
> > + if (((char *)sfep + sizeof(*sfep)) >= endp)
> > + return __this_address;
> > +
> > + /* Don't allow names with known bad length. */
> > + if (sfep->namelen == 0)
> > + return __this_address;
> > +
> > + /*
> > + * Check that the variable-length part of the structure is
> > + * within the data buffer. The next entry starts after the
> > + * name component, so nextentry is an acceptable test.
> > + */
> > + next_sfep = XFS_ATTR_SF_NEXTENTRY(sfep);
> > + if ((char *)next_sfep > endp)
> > + return __this_address;
> > +
> > + /*
> > + * Check for unknown flags. Short form doesn't support
> > + * the incomplete or local bits.
> > + */
> > + if (sfep->flags & ~(XFS_ATTR_ROOT | XFS_ATTR_SECURE))
> > + return __this_address;
> > +
> > + /* Check for invalid namespace combinations. */
> > + if ((sfep->flags & XFS_ATTR_ROOT) &&
> > + (sfep->flags & XFS_ATTR_SECURE))
> > + return __this_address;
>
> #define ATTR_SF_FLAGS (XFS_ATTR_ROOT | XFS_ATTR_SECURE)
XFS_ATTR_NSP_ONDISK_MASK?
>
> if (sfep->flags & ~ATTR_SF_FLAGS)
> return __this_address;
> if ((sfep->flags & ATTR_SF_FLAGS) == ATTR_SF_FLAGS)
> return __this_address;
Also, can we just use hweight8() > 1 here? We only allow one namespace
bit per attr.
--D
>
> Otherwise looks ok.
>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
>
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 08/13] xfs: create structure verifier function for short form symlinks
2017-12-19 5:27 ` Dave Chinner
@ 2017-12-19 20:45 ` Darrick J. Wong
0 siblings, 0 replies; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-19 20:45 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 04:27:58PM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:59:01PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Create a function to check the structure of short form symlink targets.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/libxfs/xfs_shared.h | 1 +
> > fs/xfs/libxfs/xfs_symlink_remote.c | 36 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 37 insertions(+)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h
> > index c6f4eb4..67ccb1a 100644
> > --- a/fs/xfs/libxfs/xfs_shared.h
> > +++ b/fs/xfs/libxfs/xfs_shared.h
> > @@ -143,5 +143,6 @@ bool xfs_symlink_hdr_ok(xfs_ino_t ino, uint32_t offset,
> > uint32_t size, struct xfs_buf *bp);
> > void xfs_symlink_local_to_remote(struct xfs_trans *tp, struct xfs_buf *bp,
> > struct xfs_inode *ip, struct xfs_ifork *ifp);
> > +xfs_failaddr_t xfs_symlink_shortform_verify(struct xfs_inode *ip);
> >
> > #endif /* __XFS_SHARED_H__ */
> > diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
> > index b6ef6cb..5d9642e 100644
> > --- a/fs/xfs/libxfs/xfs_symlink_remote.c
> > +++ b/fs/xfs/libxfs/xfs_symlink_remote.c
> > @@ -210,3 +210,39 @@ xfs_symlink_local_to_remote(
> > xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsymlink_hdr) +
> > ifp->if_bytes - 1);
> > }
> > +
> > +/* Verify the consistency of an inline symlink. */
> > +xfs_failaddr_t
> > +xfs_symlink_shortform_verify(
> > + struct xfs_inode *ip)
> > +{
> > + char *sfp;
> > + char *endp;
> > + struct xfs_ifork *ifp;
> > + int size;
> > +
> > + ASSERT(ip->i_d.di_format == XFS_DINODE_FMT_LOCAL);
> > + ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
> > + sfp = (char *)ifp->if_u1.if_data;
> > + size = ifp->if_bytes;
> > + endp = sfp + size;
> > +
> > + /* Zero length symlinks can exist while we're deleting a remote one. */
> > + if (size == 0)
> > + return NULL;
> > +
> > + /* No negative sizes or overly long symlink targets. */
> > + if (size < 0 || size > XFS_SYMLINK_MAXLEN)
> > + return __this_address;
> > +
> > + /* No NULLs in the target either. */
> > + for (; sfp < endp; sfp++) {
> > + if (*sfp == 0)
> > + return __this_address;
> > + }
>
> if (memchr(sfp, 0, size - 1))
> return __this_address;
Fixed.
--D
> Otherwise looks ok.
>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
>
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 06/13] xfs: move inode fork verifiers to xfs_dinode_verify
2017-12-19 20:34 ` Darrick J. Wong
@ 2017-12-19 20:48 ` Dave Chinner
0 siblings, 0 replies; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 20:48 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 12:34:59PM -0800, Darrick J. Wong wrote:
> On Tue, Dec 19, 2017 at 04:16:19PM +1100, Dave Chinner wrote:
> > On Wed, Dec 13, 2017 at 03:58:49PM -0800, Darrick J. Wong wrote:
> > > + if (mode && dip->di_forkoff > mp->m_sb.sb_inodesize)
> > > + return __this_address;
> >
> > Hold on, this check is completely bogus. di_forkoff is in units of 8 bytes,
> > which inode size is in bytes. Also, di_forkoff is a u8, so it can't
> > /ever/ be larger than the inode size which are >= 256 bytes. Yeah,
> > though so:
> >
> > #define XFS_DFORK_BOFF(dip) ((int)((dip)->di_forkoff << 3))
> >
> > This check needs to be:
> >
> > if (mode && XFS_DFORK_BOFF(dip) > mp->m_sb.sb_inodesize)
> > return __this_address;
> >
> > Otherwise looks good.
>
> Oops, thank you for catching that!
Wasn't your bug - the existing code has had this check for a long
time and nobody has noticed it was bogus until now :/
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 10/13] xfs: provide a centralized method for verifying inline fork data
2017-12-19 6:06 ` Dave Chinner
@ 2017-12-19 20:50 ` Darrick J. Wong
0 siblings, 0 replies; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-19 20:50 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 05:06:40PM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:59:13PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Replace the current haphazard dir2 shortform verifier callsites with a
> > centralized verifier function that can be called either with the default
> > verifier functions or with a custom set. This helps us strengthen
> > integrity checking while providing us with flexibility for repair tools.
> >
> > xfs_repair wants this to be able to supply its own verifier functions
> > when trying to fix possibly corrupt metadata.
>
> I was about to say "this looks a little over-engineered" then I read
> the commit message.... :P
>
> Ok, so it pulls the fork verication out of the libxfs inode read code,
> and instead slots it into the kernel inode cache miss path just
> after the inode has been read. Same effect - newly read inodes get
> their forks verified before use. ANd they same thing happens in
> xfs_iflush_int(), which is private to the kernel anyway....
<nod> It's a little over-engineered for the kernel; the real point of
all this is to check local format forks by default while enabling
xfs_repair to skip those checks if, say, it wants to fix an inode with
broken xattrs by zapping the attr fork.
> > +
> > +/* Default fork content verifiers. */
> > +struct xfs_ifork_ops xfs_default_ifork_ops = {
> > + .verify_attr = xfs_attr_shortform_verify,
> > + .verify_dir = xfs_dir2_sf_verify,
> > + .verify_symlink = xfs_symlink_shortform_verify,
> > +};
> > +
> > +/* Verify the inline contents of the data fork of an inode. */
> > +xfs_failaddr_t
> > +xfs_ifork_verify_data(
> > + struct xfs_inode *ip,
> > + struct xfs_ifork_ops *ops)
> > +{
> > + xfs_ifork_verifier_t fn = NULL;
> > +
> > + /* Non-local data fork, we're done. */
> > + if (ip->i_d.di_format != XFS_DINODE_FMT_LOCAL)
> > + return NULL;
> > +
> > + /* Check the inline data fork if there is one. */
> > + if (S_ISDIR(VFS_I(ip)->i_mode))
> > + fn = ops->verify_dir;
> > + else if (S_ISLNK(VFS_I(ip)->i_mode))
> > + fn = ops->verify_symlink;
> > +
> > + if (fn)
> > + return fn(ip);
> > + return NULL;
> > +}
>
> Seems a little convoluted, especially if we grow more cases. This
> seems simpler already:
>
> switch(VFS_I(ip)->i_mode & S_IFMT) {
> case S_IFDIR:
> return ops->verify_dir(ip);
> case S_IFLNK:
> return ops->verify_symlink(ip);
> default:
> break;
> }
> return NULL;
Fixed.
> > +
> > +/* Verify the inline contents of the attr fork of an inode. */
> > +xfs_failaddr_t
> > +xfs_ifork_verify_attr(
> > + struct xfs_inode *ip,
> > + struct xfs_ifork_ops *ops)
> > +{
> > + xfs_ifork_verifier_t fn = NULL;
> > +
> > + /* There has to be an attr fork allocated if aformat is local. */
> > + if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
> > + if (XFS_IFORK_PTR(ip, XFS_ATTR_FORK))
> > + fn = ops->verify_attr;
> > + else
> > + return __this_address;
> > + }
> > +
> > + if (fn)
> > + return fn(ip);
> > + return NULL;
>
> And this could be stright lined as:
>
> if (ip->i_d.di_aformat != XFS_DINODE_FMT_LOCAL)
> return NULL;
> if (!XFS_IFORK_PTR(ip, XFS_ATTR_FORK))
> return __this_address;
>
> return ops->verify_attr(ip);
Fixed.
--D
> -Dave.
>
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 07/13] xfs: create structure verifier function for shortform xattrs
2017-12-19 20:41 ` Darrick J. Wong
@ 2017-12-19 20:51 ` Dave Chinner
2017-12-19 21:04 ` Darrick J. Wong
0 siblings, 1 reply; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 20:51 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 12:41:47PM -0800, Darrick J. Wong wrote:
> On Tue, Dec 19, 2017 at 04:23:29PM +1100, Dave Chinner wrote:
> > On Wed, Dec 13, 2017 at 03:58:55PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > >
> > > Create a function to perform structure verification for short form
> > > extended attributes.
> > >
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > > + /* Check for invalid namespace combinations. */
> > > + if ((sfep->flags & XFS_ATTR_ROOT) &&
> > > + (sfep->flags & XFS_ATTR_SECURE))
> > > + return __this_address;
> >
> > #define ATTR_SF_FLAGS (XFS_ATTR_ROOT | XFS_ATTR_SECURE)
>
> XFS_ATTR_NSP_ONDISK_MASK?
Yeah, that works. I didn't go looking for an existing definition :/
> >
> > if (sfep->flags & ~ATTR_SF_FLAGS)
> > return __this_address;
> > if ((sfep->flags & ATTR_SF_FLAGS) == ATTR_SF_FLAGS)
> > return __this_address;
>
> Also, can we just use hweight8() > 1 here? We only allow one namespace
> bit per attr.
With a comment, yes, otherwise someone unfamiliar with what
hweight() does will have to go do lots of extra work to understand
the code.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 12/13] xfs: create a new buf_ops pointer to verify structure metadata
2017-12-19 18:15 ` Darrick J. Wong
@ 2017-12-19 20:53 ` Dave Chinner
0 siblings, 0 replies; 41+ messages in thread
From: Dave Chinner @ 2017-12-19 20:53 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Tue, Dec 19, 2017 at 10:15:29AM -0800, Darrick J. Wong wrote:
> On Tue, Dec 19, 2017 at 05:22:05PM +1100, Dave Chinner wrote:
> > On Wed, Dec 13, 2017 at 03:59:25PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > >
> > > Expose all metadata structure buffer verifier functions via buf_ops.
> > > These will be used by the online scrub mechanism to look for problems
> > > with buffers that are already sitting around in memory.
> > >
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > ....
> > > @@ -2468,7 +2478,7 @@ xfs_agf_read_verify(
> > > .verify_write = xfs_attr3_leaf_write_verify,
> > > + .verify_struct = xfs_attr3_leaf_verify,
> > > };
> > >
> > > int
> > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> > > index d4d2902..1be995b 100644
> > > --- a/fs/xfs/libxfs/xfs_attr_remote.c
> > > +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> > > @@ -204,10 +204,42 @@ xfs_attr3_rmt_write_verify(
> > > ASSERT(len == 0);
> > > }
> > >
> > > +static xfs_failaddr_t
> > > +xfs_attr3_rmt_verify_struct(
> > > + struct xfs_buf *bp)
> > > +{
> > > + struct xfs_mount *mp = bp->b_target->bt_mount;
> > > + char *ptr;
> > > + void *failed_at;
> > > + int len;
> > > + xfs_daddr_t bno;
> > > + int blksize = mp->m_attr_geo->blksize;
> > > +
> > > + /* no verification of non-crc buffers */
> > > + if (!xfs_sb_version_hascrc(&mp->m_sb))
> > > + return NULL;
> > > +
> > > + ptr = bp->b_addr;
> > > + bno = bp->b_bn;
> > > + len = BBTOB(bp->b_length);
> > > + ASSERT(len >= blksize);
> > > +
> > > + while (len > 0) {
> > > + if ((failed_at = xfs_attr3_rmt_verify(mp, ptr, blksize, bno)))
> > > + return failed_at;
> > > + len -= blksize;
> > > + ptr += blksize;
> > > + bno += BTOBB(blksize);
> > > + }
> > > +
> > > + return NULL;
> > > +}
> >
> > I'd much prefer to see this combined with
> > xfs_attr3_rmt_read_verify() rather than having another copy of this
> > iteration code. They really only vary by whether the CRC is checked
> > in the loop....
> >
> > ....
> >
> > > +static xfs_failaddr_t
> > > +xfs_dquot_buf_verify_struct(
> > > + struct xfs_buf *bp)
> > > +{
> > > + struct xfs_mount *mp = bp->b_target->bt_mount;
> > > +
> > > + if (!xfs_dquot_buf_verify(mp, bp, 0))
> > > + return __this_address;
> > > + return NULL;
> > > +}
> >
> > I can't remember what happened exactly with dquot buffers earlire in
> > the patchset, but why isn't it returning a failaddr like all the
> > other structure verifiers?
>
> The dquot verifiers are sufficiently different from everything else
> (verbose error reporting, some ability to zap garbage data) that I was
> going to send that as a separate cleanup series.
>
> I /think/ the solution is to disentangle xfs_dqcheck into a separate
> check routine that returns xfs_failaddr_t like everything else (at a
> cost of the removal of all the xfs_alert calls) so that the error
> reports become "xfs: quota buffer XXX error at xfs_dqcheck+0x74" like
> everything else. The caller becomes directly responsible for printing a
> warning message (instead of XFS_QMOPT_DOWARN).
>
> Then, the quota repair piece becomes a separate function which the
> XFS_QMOPT_DQREPAIR callers can call directly.
>
> Sound good?
Yes, seems like a reasonable cleanup to make. Making the dquot code
have fewer special snowflakes is always a good idea :P
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 07/13] xfs: create structure verifier function for shortform xattrs
2017-12-19 20:51 ` Dave Chinner
@ 2017-12-19 21:04 ` Darrick J. Wong
0 siblings, 0 replies; 41+ messages in thread
From: Darrick J. Wong @ 2017-12-19 21:04 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Wed, Dec 20, 2017 at 07:51:29AM +1100, Dave Chinner wrote:
> On Tue, Dec 19, 2017 at 12:41:47PM -0800, Darrick J. Wong wrote:
> > On Tue, Dec 19, 2017 at 04:23:29PM +1100, Dave Chinner wrote:
> > > On Wed, Dec 13, 2017 at 03:58:55PM -0800, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > >
> > > > Create a function to perform structure verification for short form
> > > > extended attributes.
> > > >
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > > + /* Check for invalid namespace combinations. */
> > > > + if ((sfep->flags & XFS_ATTR_ROOT) &&
> > > > + (sfep->flags & XFS_ATTR_SECURE))
> > > > + return __this_address;
> > >
> > > #define ATTR_SF_FLAGS (XFS_ATTR_ROOT | XFS_ATTR_SECURE)
> >
> > XFS_ATTR_NSP_ONDISK_MASK?
>
> Yeah, that works. I didn't go looking for an existing definition :/
>
> > >
> > > if (sfep->flags & ~ATTR_SF_FLAGS)
> > > return __this_address;
> > > if ((sfep->flags & ATTR_SF_FLAGS) == ATTR_SF_FLAGS)
> > > return __this_address;
> >
> > Also, can we just use hweight8() > 1 here? We only allow one namespace
> > bit per attr.
>
> With a comment, yes, otherwise someone unfamiliar with what
> hweight() does will have to go do lots of extra work to understand
> the code.
/*
* Check for invalid namespace combinations. We only allow
* one namespace flag per xattr, so we can just count the
* bits (i.e. hweight) here.
*/
--D
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2017-12-19 21:04 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-13 23:58 [PATCH 00/13] xfs: more and better verifiers Darrick J. Wong
2017-12-13 23:58 ` [PATCH 01/13] xfs: refactor long-format btree header verification routines Darrick J. Wong
2017-12-14 22:06 ` Dave Chinner
2017-12-15 0:12 ` Darrick J. Wong
2017-12-13 23:58 ` [PATCH 02/13] xfs: remove XFS_WANT_CORRUPTED_RETURN from dir3 data verifiers Darrick J. Wong
2017-12-19 3:50 ` Dave Chinner
2017-12-13 23:58 ` [PATCH 03/13] xfs: have buffer verifier functions report failing address Darrick J. Wong
2017-12-19 4:12 ` Dave Chinner
2017-12-19 20:26 ` Darrick J. Wong
2017-12-13 23:58 ` [PATCH 04/13] xfs: refactor verifier callers to print address of failing check Darrick J. Wong
2017-12-14 22:03 ` Dave Chinner
2017-12-15 0:04 ` Darrick J. Wong
2017-12-15 3:09 ` Dave Chinner
2017-12-19 20:29 ` Darrick J. Wong
2017-12-13 23:58 ` [PATCH 05/13] xfs: verify dinode header first Darrick J. Wong
2017-12-19 4:13 ` Dave Chinner
2017-12-13 23:58 ` [PATCH 06/13] xfs: move inode fork verifiers to xfs_dinode_verify Darrick J. Wong
2017-12-19 5:16 ` Dave Chinner
2017-12-19 20:34 ` Darrick J. Wong
2017-12-19 20:48 ` Dave Chinner
2017-12-13 23:58 ` [PATCH 07/13] xfs: create structure verifier function for shortform xattrs Darrick J. Wong
2017-12-19 5:23 ` Dave Chinner
2017-12-19 20:41 ` Darrick J. Wong
2017-12-19 20:51 ` Dave Chinner
2017-12-19 21:04 ` Darrick J. Wong
2017-12-13 23:59 ` [PATCH 08/13] xfs: create structure verifier function for short form symlinks Darrick J. Wong
2017-12-19 5:27 ` Dave Chinner
2017-12-19 20:45 ` Darrick J. Wong
2017-12-13 23:59 ` [PATCH 09/13] xfs: refactor short form directory structure verifier function Darrick J. Wong
2017-12-19 5:45 ` Dave Chinner
2017-12-13 23:59 ` [PATCH 10/13] xfs: provide a centralized method for verifying inline fork data Darrick J. Wong
2017-12-19 6:06 ` Dave Chinner
2017-12-19 20:50 ` Darrick J. Wong
2017-12-13 23:59 ` [PATCH 11/13] xfs: fail out of xfs_attr3_leaf_lookup_int if it looks corrupt Darrick J. Wong
2017-12-19 6:13 ` Dave Chinner
2017-12-13 23:59 ` [PATCH 12/13] xfs: create a new buf_ops pointer to verify structure metadata Darrick J. Wong
2017-12-19 6:22 ` Dave Chinner
2017-12-19 18:15 ` Darrick J. Wong
2017-12-19 20:53 ` Dave Chinner
2017-12-13 23:59 ` [PATCH 13/13] xfs: scrub in-core metadata Darrick J. Wong
2017-12-19 6:23 ` Dave Chinner
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.