public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Brian Foster <bfoster@redhat.com>
Cc: xfs@oss.sgi.com
Subject: [PATCH] Re: XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length), file: fs/xfs/xfs_trans_buf.c, line: 568
Date: Mon, 26 Aug 2013 14:13:30 +1000	[thread overview]
Message-ID: <20130826041330.GU6023@dastard> (raw)
In-Reply-To: <52165830.8050006@redhat.com>

On Thu, Aug 22, 2013 at 02:28:00PM -0400, Brian Foster wrote:
> Hi all,
> 
> I hit an assert on a debug kernel while beating on some finobt work and
> eventually reproduced it on unmodified/TOT xfs/xfsprogs as of today. I
> hit it through a couple different paths, first while running fsstress on
> a CRC enabled filesystem (with otherwise default mkfs options):
> 
> (These tests are running on a 4p, 4GB VM against a 100GB virtio disk,
> hosted on a single spindle desktop box).
> 
> crc=1
> fsstress -z -fsymlink=1 -n99999999 -p4 -d /mnt/test
> 
> XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length),

Directory buffer overrun.

>  [<ffffffffa031d549>] xfs_trans_log_buf+0x89/0x1b0 [xfs]
>  [<ffffffffa02e7c1c>] xfs_da3_node_add+0x11c/0x210 [xfs]
>  [<ffffffffa02ea703>] xfs_da3_node_split+0xc3/0x230 [xfs]
>  [<ffffffffa02eaa18>] xfs_da3_split+0x1a8/0x410 [xfs]
>  [<ffffffffa02f743f>] xfs_dir2_node_addname+0x47f/0xde0 [xfs]

During a split.

Easily reproduced with "seq 200000 | xargs touch" as Michael Semon
reported last week.

The fix demonstrates my concerns about modifying directory code -
the CRC changes missed a *fundamental* directory format definition,
and we've only just tripped over it....

> rm -rf /mnt/test
> 
> XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length),

Directory buffer overrun.

>  [<ffffffffa032b549>] xfs_trans_log_buf+0x89/0x1b0 [xfs]
>  [<ffffffffa02f61ff>] xfs_da3_node_unbalance+0xef/0x1d0 [xfs]
>  [<ffffffffa02f98b0>] xfs_da3_join+0x240/0x290 [xfs]
>  [<ffffffffa030659b>] xfs_dir2_node_removename+0x69b/0x8b0 [xfs]

During a merge. Not sure why that is happening on a v4 filesystem.
V5 filesystem, yes, due to the above bug but v4 should not be
affected.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

xfs: fix calculation of the number of node entries in a dir3 node

From: Dave Chinner <dchinner@redhat.com>

The calculation doesn't take into account the size of the dir v3
header, so overestimates the hash entries in a node. This causes
directory buffer overruns when splitting and merging nodes.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_da_btree.h | 11 +++++++++--
 fs/xfs/xfs_dir2.c     | 16 ++++++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_da_btree.h b/fs/xfs/xfs_da_btree.h
index 8cdc77b..b1f2679 100644
--- a/fs/xfs/xfs_da_btree.h
+++ b/fs/xfs/xfs_da_btree.h
@@ -133,12 +133,19 @@ extern void xfs_da3_node_hdr_to_disk(struct xfs_da_intnode *to,
 				     struct xfs_da3_icnode_hdr *from);
 
 static inline int
-xfs_da3_node_hdr_size(struct xfs_da_intnode *dap)
+__xfs_da3_node_hdr_size(bool v3)
 {
-	if (dap->hdr.info.magic == cpu_to_be16(XFS_DA3_NODE_MAGIC))
+	if (v3)
 		return sizeof(struct xfs_da3_node_hdr);
 	return sizeof(struct xfs_da_node_hdr);
 }
+static inline int
+xfs_da3_node_hdr_size(struct xfs_da_intnode *dap)
+{
+	bool	v3 = dap->hdr.info.magic == cpu_to_be16(XFS_DA3_NODE_MAGIC);
+
+	return __xfs_da3_node_hdr_size(v3);
+}
 
 static inline struct xfs_da_node_entry *
 xfs_da3_node_tree_p(struct xfs_da_intnode *dap)
diff --git a/fs/xfs/xfs_dir2.c b/fs/xfs/xfs_dir2.c
index d3ff96c..edf203a 100644
--- a/fs/xfs/xfs_dir2.c
+++ b/fs/xfs/xfs_dir2.c
@@ -90,6 +90,9 @@ void
 xfs_dir_mount(
 	xfs_mount_t	*mp)
 {
+	int	nodehdr_size;
+
+
 	ASSERT(xfs_sb_version_hasdirv2(&mp->m_sb));
 	ASSERT((1 << (mp->m_sb.sb_blocklog + mp->m_sb.sb_dirblklog)) <=
 	       XFS_MAX_BLOCKSIZE);
@@ -98,12 +101,13 @@ xfs_dir_mount(
 	mp->m_dirdatablk = xfs_dir2_db_to_da(mp, XFS_DIR2_DATA_FIRSTDB(mp));
 	mp->m_dirleafblk = xfs_dir2_db_to_da(mp, XFS_DIR2_LEAF_FIRSTDB(mp));
 	mp->m_dirfreeblk = xfs_dir2_db_to_da(mp, XFS_DIR2_FREE_FIRSTDB(mp));
-	mp->m_attr_node_ents =
-		(mp->m_sb.sb_blocksize - (uint)sizeof(xfs_da_node_hdr_t)) /
-		(uint)sizeof(xfs_da_node_entry_t);
-	mp->m_dir_node_ents =
-		(mp->m_dirblksize - (uint)sizeof(xfs_da_node_hdr_t)) /
-		(uint)sizeof(xfs_da_node_entry_t);
+
+	nodehdr_size = __xfs_da3_node_hdr_size(xfs_sb_version_hascrc(&mp->m_sb));
+	mp->m_attr_node_ents = (mp->m_sb.sb_blocksize - nodehdr_size) /
+				(uint)sizeof(xfs_da_node_entry_t);
+	mp->m_dir_node_ents = (mp->m_dirblksize - nodehdr_size) /
+				(uint)sizeof(xfs_da_node_entry_t);
+
 	mp->m_dir_magicpct = (mp->m_dirblksize * 37) / 100;
 	if (xfs_sb_version_hasasciici(&mp->m_sb))
 		mp->m_dirnameops = &xfs_ascii_ci_nameops;

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  parent reply	other threads:[~2013-08-26  4:13 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-22 18:28 XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length), file: fs/xfs/xfs_trans_buf.c, line: 568 Brian Foster
2013-08-23 13:18 ` Mark Tinguely
2013-08-23 13:30   ` Brian Foster
2013-08-23 13:57     ` Mark Tinguely
2013-08-23 16:49       ` Brian Foster
2013-08-23 18:01         ` Mark Tinguely
2013-08-26  4:13 ` Dave Chinner [this message]
2013-08-26 13:36   ` [PATCH] " Brian Foster
2013-08-26 15:00     ` Mark Tinguely
2013-08-26 21:04       ` Dave Chinner
2013-08-26 21:19         ` Mark Tinguely
2013-08-27 13:04           ` Mark Tinguely
2013-08-26 20:26   ` Michael L. Semon
2013-08-29 12:37   ` Mark Tinguely
2013-08-30 14:56     ` Ben Myers
2013-09-12 23:51 ` Mark Tinguely
2013-09-16 15:44   ` Christoph Hellwig
2013-09-16 17:30     ` Mark Tinguely
2013-09-16 17:41     ` Michael L. Semon

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=20130826041330.GU6023@dastard \
    --to=david@fromorbit.com \
    --cc=bfoster@redhat.com \
    --cc=xfs@oss.sgi.com \
    /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