public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Mark Tinguely <tinguely@sgi.com>
To: stable@vger.kernel.org
Cc: xfs@oss.sgi.com
Subject: [3.0-stable PATCH 23/36] xfs: fix buffer lookup race on allocation failure
Date: Mon, 03 Dec 2012 17:42:31 -0600	[thread overview]
Message-ID: <20121203144310.928951136@sgi.com> (raw)
In-Reply-To: 20121203144208.143464631@sgi.com

[-- Attachment #1: 081 --]
[-- Type: text/plain, Size: 2410 bytes --]

From: Dave Chinner <dchinner@redhat.com>

Upstream commit: fe2429b0966a7ec42b5fe3bf96f0f10de0a3b536

When memory allocation fails to add the page array or tht epages to
a buffer during xfs_buf_get(), the buffer is left in the cache in a
partially initialised state. There is enough state left for the next
lookup on that buffer to find the buffer, and for the buffer to then
be used without finishing the initialisation.  As a result, when an
attempt to do IO on the buffer occurs, it fails with EIO because
there are no pages attached to the buffer.

We cannot remove the buffer from the cache immediately and free it,
because there may already be a racing lookup that is blocked on the
buffer lock. Hence the moment we unlock the buffer to then free it,
the other user is woken and we have a use-after-free situation.

To avoid this race condition altogether, allocate the pages for the
buffer before we insert it into the cache.  This then means that we
don't have an allocation  failure case to deal after the buffer is
already present in the cache, and hence avoid the problem
altogether.  In most cases we won't have racing inserts for the same
buffer, and so won't increase the memory pressure allocation before
insertion may entail.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Mark Tinguely <tinguely@sgi.com>
Signed-off-by: Ben Myers <bpm@sgi.com>
---
 fs/xfs/linux-2.6/xfs_buf.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Index: b/fs/xfs/linux-2.6/xfs_buf.c
===================================================================
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -551,18 +551,20 @@ xfs_buf_get(
 	if (unlikely(!new_bp))
 		return NULL;
 
+	error = xfs_buf_allocate_memory(new_bp, flags);
+	if (error) {
+		kmem_zone_free(xfs_buf_zone, new_bp);
+		return NULL;
+	}
+
 	bp = _xfs_buf_find(target, ioff, isize, flags, new_bp);
 	if (!bp) {
-		kmem_zone_free(xfs_buf_zone, new_bp);
+		xfs_buf_free(new_bp);
 		return NULL;
 	}
 
-	if (bp == new_bp) {
-		error = xfs_buf_allocate_memory(bp, flags);
-		if (error)
-			goto no_buffer;
-	} else
-		kmem_zone_free(xfs_buf_zone, new_bp);
+	if (bp != new_bp)
+		xfs_buf_free(new_bp);
 
 	/*
 	 * Now we have a workable buffer, fill in the block number so


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

  parent reply	other threads:[~2012-12-03 23:40 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-03 23:42 [3.0-stable PATCH 00/36] Proposed 3.0-stable bug patches Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 01/36] xfs: fix possible overflow in xfs_ioc_trim() Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 02/36] xfs: fix allocation length overflow in xfs_bmapi_write() Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 03/36] xfs: mark the xfssyncd workqueue as non-reentrant Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 04/36] xfs: xfs_trans_add_item() - dont assign in ASSERT() when compare is intended Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 05/36] xfs: only take the ILOCK in xfs_reclaim_inode() Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 06/36] xfs: fallback to vmalloc for large buffers in xfs_attrmulti_attr_get Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 07/36] xfs: fallback to vmalloc for large buffers in xfs_getbmap Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 08/36] xfs: fix deadlock in xfs_rtfree_extent Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 09/36] xfs: Fix open flag handling in open_by_handle code Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 10/36] xfs: Account log unmount transaction correctly Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 11/36] xfs: fix fstrim offset calculations Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 12/36] xfs: dont fill statvfs with project quota for a directory Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 13/36] xfs: Ensure inode reclaim can run during quotacheck Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 14/36] xfs: use shared ilock mode for direct IO writes by default Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 15/36] xfs: punch all delalloc blocks beyond EOF on write failure Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 16/36] xfs: page type check in writeback only checks last buffer Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 17/36] xfs: punch new delalloc blocks out of failed writes inside EOF Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 18/36] xfs: dont assert on delalloc regions beyond EOF Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 19/36] xfs: limit specualtive delalloc to maxioffset Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 20/36] xfs: Use preallocation for inodes with extsz hints Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 21/36] xfs: Dont allocate new buffers on every call to _xfs_buf_find Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 22/36] xfs: clean up buffer allocation Mark Tinguely
2012-12-03 23:42 ` Mark Tinguely [this message]
2012-12-03 23:42 ` [3.0-stable PATCH 24/36] xfs: use iolock on XFS_IOC_ALLOCSP calls Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 25/36] xfs: Properly exclude IO type flags from buffer flags Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 26/36] xfs: flush outstanding buffers on log mount failure Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 27/36] xfs: protect xfs_sync_worker with s_umount semaphore Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 28/36] xfs: fix memory reclaim deadlock on agi buffer Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 29/36] xfs: xfs_vm_writepage clear iomap_valid when Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 30/36] xfs: fix allocbt cursor leak in xfs_alloc_ag_vextent_near Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 31/36] xfs: shutdown xfs_sync_worker before the log Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 32/36] xfs: really fix the cursor leak in xfs_alloc_ag_vextent_near Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 33/36] xfs: check for stale inode before acquiring iflock on push Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 34/36] xfs: stop the sync worker before xfs_unmountfs Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 35/36] xfs: zero allocation_args on the kernel stack Mark Tinguely
2012-12-03 23:42 ` [3.0-stable PATCH 36/36] xfs: only update the last_sync_lsn when a transaction completes Mark Tinguely
2012-12-04 21:44 ` [3.0-stable PATCH 00/36] Proposed 3.0-stable bug patches Ben Myers
2012-12-05 21:45 ` Dave Chinner
2012-12-06 17:27   ` Mark Tinguely
2012-12-07 10:06     ` Dave Chinner
2012-12-07 21:15       ` Ben Myers
2012-12-08 12:06         ` Christoph Hellwig
2012-12-08 19:12         ` Greg KH
2012-12-10  0:24         ` Dave Chinner
2012-12-10 22:03           ` Ben Myers

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=20121203144310.928951136@sgi.com \
    --to=tinguely@sgi.com \
    --cc=stable@vger.kernel.org \
    --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