[0/2] xfs: fix buffer use after free on unpin abort

Message

Brian Foster May 11, 2021, 1:52 p.m. UTC

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/[e-mail]/

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(-)