From: Lachlan McIlroy <lachlan@sgi.com>
To: xfs-dev <xfs-dev@sgi.com>, xfs-oss <xfs@oss.sgi.com>
Subject: [PATCH] Fix use-after-free with buffers
Date: Wed, 20 Aug 2008 15:21:48 +1000 [thread overview]
Message-ID: <48ABA9EC.5040902@sgi.com> (raw)
We have a use-after-free issue where log completions access buffers via the
buffer log item and the buffer has already been freed. Fix this by taking
a reference on the buffer when attaching the buffer log item and release
the hold when the buffer log item is detached and we no longer need the
buffer. Also create a new function xfs_buf_item_free() to combine some
common code and move an ASSERT in xfs_buf_rele() so that we can catch more
cases.
===========================================================================
fs/xfs/linux-2.6/xfs_buf.c
===========================================================================
--- a/fs/xfs/linux-2.6/xfs_buf.c 2008-08-20 15:08:23.000000000 +1000
+++ b/fs/xfs/linux-2.6/xfs_buf.c 2008-08-20 15:14:06.000000000 +1000
@@ -831,6 +831,8 @@ xfs_buf_rele(
XB_TRACE(bp, "rele", bp->b_relse);
+ ASSERT(atomic_read(&bp->b_hold) > 0);
+
if (unlikely(!hash)) {
ASSERT(!bp->b_relse);
if (atomic_dec_and_test(&bp->b_hold))
@@ -838,7 +840,6 @@ xfs_buf_rele(
return;
}
- ASSERT(atomic_read(&bp->b_hold) > 0);
if (atomic_dec_and_lock(&bp->b_hold, &hash->bh_lock)) {
if (bp->b_relse) {
atomic_inc(&bp->b_hold);
===========================================================================
fs/xfs/xfs_buf_item.c
===========================================================================
--- a/fs/xfs/xfs_buf_item.c 2008-08-20 15:08:23.000000000 +1000
+++ b/fs/xfs/xfs_buf_item.c 2008-08-20 15:14:06.000000000 +1000
@@ -732,6 +732,7 @@ xfs_buf_item_init(
bip->bli_item.li_ops = &xfs_buf_item_ops;
bip->bli_item.li_mountp = mp;
bip->bli_buf = bp;
+ xfs_buf_hold(bp);
bip->bli_format.blf_type = XFS_LI_BUF;
bip->bli_format.blf_blkno = (__int64_t)XFS_BUF_ADDR(bp);
bip->bli_format.blf_len = (ushort)BTOBB(XFS_BUF_COUNT(bp));
@@ -867,6 +868,23 @@ xfs_buf_item_dirty(
return (bip->bli_flags & XFS_BLI_DIRTY);
}
+void
+xfs_buf_item_free(
+ xfs_buf_log_item_t *bip)
+{
+#ifdef XFS_TRANS_DEBUG
+ kmem_free(bip->bli_orig);
+ bip->bli_orig = NULL;
+ kmem_free(bip->bli_logged);
+ bip->bli_logged = NULL;
+#endif /* XFS_TRANS_DEBUG */
+
+#ifdef XFS_BLI_TRACE
+ ktrace_free(bip->bli_trace);
+#endif
+ kmem_zone_free(xfs_buf_item_zone, bip);
+}
+
/*
* This is called when the buf log item is no longer needed. It should
* free the buf log item associated with the given buffer and clear
@@ -887,18 +905,8 @@ xfs_buf_item_relse(
(XFS_BUF_IODONE_FUNC(bp) != NULL)) {
XFS_BUF_CLR_IODONE_FUNC(bp);
}
-
-#ifdef XFS_TRANS_DEBUG
- kmem_free(bip->bli_orig);
- bip->bli_orig = NULL;
- kmem_free(bip->bli_logged);
- bip->bli_logged = NULL;
-#endif /* XFS_TRANS_DEBUG */
-
-#ifdef XFS_BLI_TRACE
- ktrace_free(bip->bli_trace);
-#endif
- kmem_zone_free(xfs_buf_item_zone, bip);
+ xfs_buf_rele(bp);
+ xfs_buf_item_free(bip);
}
@@ -1120,6 +1128,7 @@ xfs_buf_iodone(
ASSERT(bip->bli_buf == bp);
+ xfs_buf_rele(bp);
mp = bip->bli_item.li_mountp;
/*
@@ -1136,18 +1145,7 @@ xfs_buf_iodone(
* xfs_trans_delete_ail() drops the AIL lock.
*/
xfs_trans_delete_ail(mp, (xfs_log_item_t *)bip);
-
-#ifdef XFS_TRANS_DEBUG
- kmem_free(bip->bli_orig);
- bip->bli_orig = NULL;
- kmem_free(bip->bli_logged);
- bip->bli_logged = NULL;
-#endif /* XFS_TRANS_DEBUG */
-
-#ifdef XFS_BLI_TRACE
- ktrace_free(bip->bli_trace);
-#endif
- kmem_zone_free(xfs_buf_item_zone, bip);
+ xfs_buf_item_free(bip);
}
#if defined(XFS_BLI_TRACE)
next reply other threads:[~2008-08-20 5:13 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-20 5:21 Lachlan McIlroy [this message]
2008-08-20 12:50 ` [PATCH] Fix use-after-free with buffers Christoph Hellwig
2008-09-02 6:02 ` Lachlan McIlroy
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=48ABA9EC.5040902@sgi.com \
--to=lachlan@sgi.com \
--cc=xfs-dev@sgi.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