All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed

From: Brian Foster <bfoster@redhat.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 0/2] xfs: fix buffer use after free on unpin abort
Date: Tue, 11 May 2021 09:52:55 -0400	[thread overview]
Message-ID: <20210511135257.878743-1-bfoster@redhat.com> (raw)

Hi all,

Here's a proper v1 of the previously posted RFC to address a subtle
buffer use after free in the unpin abort sequence for buffer log items.
Dave had previously suggested that the underlying problem here is that
bli's are effectively used by the AIL unreferenced. While this makes a
lot of sense, this is a long standing design detail that subtly impacts
code related to log item processing, AIL processing, buffer I/O, as well
as potentially log recovery. In contrast, the immediate problem that
leads to the use after free is lack of a buffer hold in a context that
already explicitly acquires a hold for the problematic simulated I/O
failure sequence.

Given the significant cost/risk vs. benefit imbalance of a design
rework, I've opted to to make the minimal change to fix this bug and
defer broader rework to a standalone effort. Patch 1 basically reorders
the preexisting buffer hold to accommodate the flaw that the AIL does
not hold a reference to the bli (and thus does not maintain the
associated buffer hold). This preserves the existing isolation logic and
prevents the associated UAF. This survives an fstests run and is going
on 6k iterations of generic/019 (which previously reproduced the problem
in 2-3k iterations) without any explosions. Thoughts, reviews, flames
appreciated.

Brian

v1:
- Rework patch 1 to hold conditionally in the abort case and document
  the underlying design flaw.
- Add patch 2 to remove some unused code.
rfc: https://lore.kernel.org/linux-xfs/20210503121816.561340-1-bfoster@redhat.com/

Brian Foster (2):
  xfs: hold buffer across unpin and potential shutdown processing
  xfs: remove dead stale buf unpin handling code

 fs/xfs/xfs_buf_item.c | 57 +++++++++++++++++--------------------------
 1 file changed, 22 insertions(+), 35 deletions(-)

-- 
2.26.3

next             reply	other threads:[~2021-05-11 13:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-11 13:52 Brian Foster [this message]
2021-05-11 13:52 ` [PATCH 1/2] xfs: hold buffer across unpin and potential shutdown processing Brian Foster
2021-05-12  1:52   ` Darrick J. Wong
2021-05-12 12:22     ` Christoph Hellwig
2021-05-12 14:29       ` Brian Foster
2021-05-12 14:28     ` Brian Foster
2021-05-11 13:52 ` [PATCH 2/2] xfs: remove dead stale buf unpin handling code Brian Foster
2021-05-12  1:55   ` Darrick J. Wong
2021-05-12 12:00   ` Christoph Hellwig
2021-05-12 14:29     ` Brian Foster
2021-05-12 14:30   ` [PATCH v1.1 " Brian Foster

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=20210511135257.878743-1-bfoster@redhat.com \
    --to=bfoster@redhat.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

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.