All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Tinguely <tinguely@sgi.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 06/18] xfs: fix buffer lookup race on allocation failure
Date: Fri, 13 Apr 2012 13:32:27 -0500	[thread overview]
Message-ID: <4F88713B.7070805@sgi.com> (raw)
In-Reply-To: <1334319061-12968-7-git-send-email-david@fromorbit.com>

On 04/13/12 07:10, Dave Chinner wrote:
> From: Dave Chinner<dchinner@redhat.com>
>
> 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>
> ---



Much simpler idea than v1. Looks good.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

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

  reply	other threads:[~2012-04-13 18:32 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-13 12:10 [PATCH 0/18] xfs: current patch queue Dave Chinner
2012-04-13 12:10 ` [PATCH 01/18] xfs: Ensure inode reclaim can run during quotacheck Dave Chinner
2012-04-13 18:01   ` Mark Tinguely
2012-04-29 21:37   ` Christoph Hellwig
2012-04-13 12:10 ` [PATCH 02/18] xfs: pass shutdown method into xfs_trans_ail_delete_bulk Dave Chinner
2012-04-13 17:40   ` Mark Tinguely
2012-04-13 23:04     ` Dave Chinner
2012-04-13 23:38       ` [PATCH 02/18 V2] " Dave Chinner
2012-04-16 18:49         ` Mark Tinguely
2012-04-13 12:10 ` [PATCH 03/18] xfs: Do background CIL flushes via a workqueue Dave Chinner
2012-04-17 17:54   ` Mark Tinguely
2012-04-17 21:21   ` Ben Myers
2012-04-17 21:49     ` Ben Myers
2012-04-18  1:47       ` Dave Chinner
2012-04-30  1:24         ` Dave Chinner
2012-04-30  6:09           ` Dave Chinner
2012-04-18  1:58     ` Dave Chinner
2012-04-13 12:10 ` [PATCH 04/18] xfs: page type check in writeback only checks last buffer Dave Chinner
2012-04-16 16:15   ` Mark Tinguely
2012-04-29 21:39   ` Christoph Hellwig
2012-04-30  0:29     ` Dave Chinner
2012-04-13 12:10 ` [PATCH 05/18] xfs: Use preallocation for inodes with extsz hints Dave Chinner
2012-04-13 16:45   ` Mark Tinguely
2012-04-16 15:59   ` Mark Tinguely
2012-04-13 12:10 ` [PATCH 06/18] xfs: fix buffer lookup race on allocation failure Dave Chinner
2012-04-13 18:32   ` Mark Tinguely [this message]
2012-04-13 12:10 ` [PATCH 07/18] xfs: check for buffer errors before waiting Dave Chinner
2012-04-13 17:56   ` Mark Tinguely
2012-04-13 12:10 ` [PATCH 08/18] xfs: fix incorrect b_offset initialisation Dave Chinner
2012-04-13 12:10 ` [PATCH 09/18] xfs: use kmem_zone_zalloc for buffers Dave Chinner
2012-04-13 12:10 ` [PATCH 10/18] xfs: clean up buffer get/read call API Dave Chinner
2012-04-13 12:10 ` [PATCH 11/18] xfs: kill b_file_offset Dave Chinner
2012-04-13 12:10 ` [PATCH 12/18] xfs: use blocks for counting length of buffers Dave Chinner
2012-04-13 12:10 ` [PATCH 13/18] xfs: use blocks for storing the desired IO size Dave Chinner
2012-04-13 12:10 ` [PATCH 14/18] xfs: kill xfs_buf_btoc Dave Chinner
2012-04-13 12:10 ` [PATCH 15/18] xfs: kill XBF_LOCK Dave Chinner
2012-04-13 21:20   ` Mark Tinguely
2012-04-13 12:10 ` [PATCH 16/18] xfs: kill xfs_read_buf() Dave Chinner
2012-04-13 12:11 ` [PATCH 17/18] xfs: kill XBF_DONTBLOCK Dave Chinner
2012-04-16 14:34   ` Mark Tinguely
2012-04-13 12:11 ` [PATCH 18/18] xfs: use iolock on XFS_IOC_ALLOCSP calls Dave Chinner
2012-04-16 15:10   ` Mark Tinguely
2012-04-16 21:29 ` [PATCH 0/18] xfs: current patch queue Ben Myers
2012-04-17  4:12   ` Dave Chinner
2012-04-17 14:44     ` 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=4F88713B.7070805@sgi.com \
    --to=tinguely@sgi.com \
    --cc=david@fromorbit.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 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.