From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 1/4] xfs: detect self referencing btree sibling pointers
Date: Mon, 2 May 2022 18:20:15 +1000 [thread overview]
Message-ID: <20220502082018.1076561-2-david@fromorbit.com> (raw)
In-Reply-To: <20220502082018.1076561-1-david@fromorbit.com>
From: Dave Chinner <dchinner@redhat.com>
To catch the obvious graph cycle problem and hence potential endless
looping.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_btree.c | 137 ++++++++++++++++++++++++++++----------
1 file changed, 102 insertions(+), 35 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index a8c79e760d8a..991fae6f500a 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -51,6 +51,50 @@ xfs_btree_magic(
return magic;
}
+static xfs_failaddr_t
+xfs_btree_check_lblock_siblings(
+ struct xfs_mount *mp,
+ struct xfs_btree_cur *cur,
+ int level,
+ xfs_fsblock_t fsb,
+ xfs_fsblock_t sibling)
+{
+ if (sibling == NULLFSBLOCK)
+ return NULL;
+ if (sibling == fsb)
+ return __this_address;
+ if (level >= 0) {
+ if (!xfs_btree_check_lptr(cur, sibling, level + 1))
+ return __this_address;
+ } else if (!xfs_verify_fsbno(mp, sibling)) {
+ return __this_address;
+ }
+
+ return NULL;
+}
+
+static xfs_failaddr_t
+xfs_btree_check_sblock_siblings(
+ struct xfs_mount *mp,
+ struct xfs_btree_cur *cur,
+ int level,
+ xfs_agnumber_t agno,
+ xfs_agblock_t agbno,
+ xfs_agblock_t sibling)
+{
+ if (sibling == NULLAGBLOCK)
+ return NULL;
+ if (sibling == agbno)
+ return __this_address;
+ if (level >= 0) {
+ if (!xfs_btree_check_sptr(cur, sibling, level + 1))
+ return __this_address;
+ } else if (!xfs_verify_agbno(mp, agno, sibling)) {
+ return __this_address;
+ }
+ return NULL;
+}
+
/*
* Check a long btree block header. Return the address of the failing check,
* or NULL if everything is ok.
@@ -65,6 +109,8 @@ __xfs_btree_check_lblock(
struct xfs_mount *mp = cur->bc_mp;
xfs_btnum_t btnum = cur->bc_btnum;
int crc = xfs_has_crc(mp);
+ xfs_failaddr_t fa;
+ xfs_fsblock_t fsb = NULLFSBLOCK;
if (crc) {
if (!uuid_equal(&block->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid))
@@ -83,16 +129,16 @@ __xfs_btree_check_lblock(
if (be16_to_cpu(block->bb_numrecs) >
cur->bc_ops->get_maxrecs(cur, level))
return __this_address;
- if (block->bb_u.l.bb_leftsib != cpu_to_be64(NULLFSBLOCK) &&
- !xfs_btree_check_lptr(cur, be64_to_cpu(block->bb_u.l.bb_leftsib),
- level + 1))
- return __this_address;
- if (block->bb_u.l.bb_rightsib != cpu_to_be64(NULLFSBLOCK) &&
- !xfs_btree_check_lptr(cur, be64_to_cpu(block->bb_u.l.bb_rightsib),
- level + 1))
- return __this_address;
- return NULL;
+ if (bp)
+ fsb = XFS_DADDR_TO_FSB(mp, xfs_buf_daddr(bp));
+
+ fa = xfs_btree_check_lblock_siblings(mp, cur, level, fsb,
+ be64_to_cpu(block->bb_u.l.bb_leftsib));
+ if (!fa)
+ fa = xfs_btree_check_lblock_siblings(mp, cur, level, fsb,
+ be64_to_cpu(block->bb_u.l.bb_rightsib));
+ return fa;
}
/* Check a long btree block header. */
@@ -130,6 +176,9 @@ __xfs_btree_check_sblock(
struct xfs_mount *mp = cur->bc_mp;
xfs_btnum_t btnum = cur->bc_btnum;
int crc = xfs_has_crc(mp);
+ xfs_failaddr_t fa;
+ xfs_agblock_t agbno = NULLAGBLOCK;
+ xfs_agnumber_t agno = NULLAGNUMBER;
if (crc) {
if (!uuid_equal(&block->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid))
@@ -146,16 +195,18 @@ __xfs_btree_check_sblock(
if (be16_to_cpu(block->bb_numrecs) >
cur->bc_ops->get_maxrecs(cur, level))
return __this_address;
- if (block->bb_u.s.bb_leftsib != cpu_to_be32(NULLAGBLOCK) &&
- !xfs_btree_check_sptr(cur, be32_to_cpu(block->bb_u.s.bb_leftsib),
- level + 1))
- return __this_address;
- if (block->bb_u.s.bb_rightsib != cpu_to_be32(NULLAGBLOCK) &&
- !xfs_btree_check_sptr(cur, be32_to_cpu(block->bb_u.s.bb_rightsib),
- level + 1))
- return __this_address;
- return NULL;
+ if (bp) {
+ agbno = xfs_daddr_to_agbno(mp, xfs_buf_daddr(bp));
+ agno = xfs_daddr_to_agno(mp, xfs_buf_daddr(bp));
+ }
+
+ fa = xfs_btree_check_sblock_siblings(mp, cur, level, agno, agbno,
+ be32_to_cpu(block->bb_u.s.bb_leftsib));
+ if (!fa)
+ fa = xfs_btree_check_sblock_siblings(mp, cur, level, agno,
+ agbno, be32_to_cpu(block->bb_u.s.bb_rightsib));
+ return fa;
}
/* Check a short btree block header. */
@@ -4271,6 +4322,20 @@ xfs_btree_visit_block(
if (xfs_btree_ptr_is_null(cur, &rptr))
return -ENOENT;
+ /*
+ * We only visit blocks once in this walk, so we have to avoid the
+ * internal xfs_btree_lookup_get_block() optimisation where it will
+ * return the same block without checking if the right sibling points
+ * back to us and creates a cyclic reference in the btree.
+ */
+ if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
+ if (be64_to_cpu(rptr.l) ==
+ XFS_DADDR_TO_FSB(cur->bc_mp, xfs_buf_daddr(bp)))
+ return -EFSCORRUPTED;
+ } else if (be32_to_cpu(rptr.s) ==
+ xfs_daddr_to_agbno(cur->bc_mp, xfs_buf_daddr(bp))) {
+ return -EFSCORRUPTED;
+ }
return xfs_btree_lookup_get_block(cur, level, &rptr, &block);
}
@@ -4445,20 +4510,21 @@ xfs_btree_lblock_verify(
{
struct xfs_mount *mp = bp->b_mount;
struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
+ xfs_fsblock_t fsb;
+ xfs_failaddr_t fa;
/* numrecs verification */
if (be16_to_cpu(block->bb_numrecs) > max_recs)
return __this_address;
/* sibling pointer verification */
- if (block->bb_u.l.bb_leftsib != cpu_to_be64(NULLFSBLOCK) &&
- !xfs_verify_fsbno(mp, be64_to_cpu(block->bb_u.l.bb_leftsib)))
- return __this_address;
- if (block->bb_u.l.bb_rightsib != cpu_to_be64(NULLFSBLOCK) &&
- !xfs_verify_fsbno(mp, be64_to_cpu(block->bb_u.l.bb_rightsib)))
- return __this_address;
-
- return NULL;
+ fsb = XFS_DADDR_TO_FSB(mp, xfs_buf_daddr(bp));
+ fa = xfs_btree_check_lblock_siblings(mp, NULL, -1, fsb,
+ be64_to_cpu(block->bb_u.l.bb_leftsib));
+ if (!fa)
+ fa = xfs_btree_check_lblock_siblings(mp, NULL, -1, fsb,
+ be64_to_cpu(block->bb_u.l.bb_rightsib));
+ return fa;
}
/**
@@ -4499,7 +4565,9 @@ xfs_btree_sblock_verify(
{
struct xfs_mount *mp = bp->b_mount;
struct xfs_btree_block *block = XFS_BUF_TO_BLOCK(bp);
- xfs_agblock_t agno;
+ xfs_agnumber_t agno;
+ xfs_agblock_t agbno;
+ xfs_failaddr_t fa;
/* numrecs verification */
if (be16_to_cpu(block->bb_numrecs) > max_recs)
@@ -4507,14 +4575,13 @@ xfs_btree_sblock_verify(
/* sibling pointer verification */
agno = xfs_daddr_to_agno(mp, xfs_buf_daddr(bp));
- if (block->bb_u.s.bb_leftsib != cpu_to_be32(NULLAGBLOCK) &&
- !xfs_verify_agbno(mp, agno, be32_to_cpu(block->bb_u.s.bb_leftsib)))
- return __this_address;
- if (block->bb_u.s.bb_rightsib != cpu_to_be32(NULLAGBLOCK) &&
- !xfs_verify_agbno(mp, agno, be32_to_cpu(block->bb_u.s.bb_rightsib)))
- return __this_address;
-
- return NULL;
+ agbno = xfs_daddr_to_agbno(mp, xfs_buf_daddr(bp));
+ fa = xfs_btree_check_sblock_siblings(mp, NULL, -1, agno, agbno,
+ be32_to_cpu(block->bb_u.s.bb_leftsib));
+ if (!fa)
+ fa = xfs_btree_check_sblock_siblings(mp, NULL, -1, agno, agbno,
+ be32_to_cpu(block->bb_u.s.bb_rightsib));
+ return fa;
}
/*
--
2.35.1
next prev parent reply other threads:[~2022-05-02 8:20 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-02 8:20 [PATCH 0/4] xfs: fix random format verification issues Dave Chinner
2022-05-02 8:20 ` Dave Chinner [this message]
2022-05-03 14:53 ` [PATCH 1/4] xfs: detect self referencing btree sibling pointers Christoph Hellwig
2022-05-03 21:27 ` Dave Chinner
2022-05-03 22:53 ` Darrick J. Wong
2022-05-03 23:13 ` Dave Chinner
2022-05-06 9:22 ` [xfs] 32678f1513: aim7.jobs-per-min -5.6% regression kernel test robot
2022-05-06 21:29 ` Dave Chinner
2022-05-07 11:09 ` [LKP] " Carel Si
2022-05-09 0:03 ` Dave Chinner
2022-05-02 8:20 ` [PATCH 2/4] xfs: validate inode fork size against fork format Dave Chinner
2022-05-03 14:55 ` Christoph Hellwig
2022-05-03 22:55 ` Darrick J. Wong
2022-05-02 8:20 ` [PATCH 3/4] xfs: set XFS_FEAT_NLINK correctly Dave Chinner
2022-05-03 14:56 ` Christoph Hellwig
2022-05-03 22:55 ` Darrick J. Wong
2022-05-02 8:20 ` [PATCH 4/4] xfs: validate v5 feature fields Dave Chinner
2022-05-02 9:44 ` kernel test robot
2022-05-02 12:37 ` kernel test robot
2022-05-03 15:00 ` Christoph Hellwig
2022-05-03 21:26 ` Dave Chinner
2022-05-03 22:59 ` Darrick J. Wong
2022-05-03 23:18 ` Dave Chinner
2022-05-03 23:28 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220502082018.1076561-2-david@fromorbit.com \
--to=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox