[PATCH] xfs: fix slab-use-after-free in xrep_newbt_free()

[PATCH] xfs: fix slab-use-after-free in xrep_newbt_free()

[vienna-0FFH]

syzbot reported a slab-use-after-free read in xfs_defer_item_unpause()
while online repair was rebuilding a bmap btree:

  BUG: KASAN: slab-use-after-free in xfs_defer_item_unpause+0x116/0x250
  Read of size 4 at addr ffff8880132c127c by task syz.0.0/5327
  Call Trace:
   xfs_defer_item_unpause
   xrep_newbt_free+0x3fe/0x5f0
   xrep_bmap_build_new_fork+0x5a9/0x740
   xrep_bmap
   xrep_attempt
   xfs_scrub_metadata

The buggy address belongs to the object at ffff8880132c1240,
which belongs to the cache xfs_defer_pending of size 64.

xrep_newbt_add_blocks() attaches a paused EFI to the repair transaction
for each space reservation so that the blocks are reclaimed if the
system goes down before the new btree is committed.  If a later
xrep_defer_finish() fails (e.g. metadata I/O errors while staging the
new btree), xfs_defer_finish_noroll() shuts the filesystem down and
xfs_defer_cancel_list() frees all pending deferred work items, which
includes the paused EFIs referenced by resv->autoreap.

The repair then unwinds through xrep_newbt_cancel() -> xrep_newbt_free().
That function notices the shutdown and jumps to the junkit label, which
calls xfs_alloc_commit_autoreap() for each leftover reservation.  That
unpauses the deferred item through the stale pointer, causing the
use-after-free.  The report confirms this: the object was allocated by
xfs_alloc_schedule_autoreap() from xrep_newbt_add_blocks(), and freed
by xfs_defer_cancel_list() from the failing xrep_defer_finish().

Beyond the use-after-free, unpausing the deferred item is pointless
once the filesystem has been shut down: the remaining deferred work
will be cancelled when the repair transaction is torn down, and any
EFI that was committed to the log before the failure is replayed by
log recovery on the next mount.

Fix this by only unpausing the autoreap when the filesystem is not shut
down.

Fixes: 6bb9ea8ecd2c ("xfs: log EFIs for all btree blocks being used to stage a btree")
Reported-by: syzbot+e7f24354da9f7915bc54@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e7f24354da9f7915bc54
Cc: stable@vger.kernel.org
Signed-off-by: vienna-0FFH <viennaoffh@gmail.com>
---
 fs/xfs/scrub/newbt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/scrub/newbt.c b/fs/xfs/scrub/newbt.c
index 584076b..83558fd 100644
--- a/fs/xfs/scrub/newbt.c
+++ b/fs/xfs/scrub/newbt.c
@@ -516,10 +516,15 @@ junkit:
 	/*
 	 * If we still have reservations attached to @newbt, cleanup must have
 	 * failed and the filesystem is about to go down.  Clean up the incore
-	 * reservations and try to commit to freeing the space we used.
+	 * reservations.  If the filesystem is already shut down, leave the
+	 * autoreaps alone: a failed deferred finish has freed the pending
+	 * work items they point to, and nothing will ever finish deferred
+	 * work on a dead filesystem.  Any EFI that made it to the log is
+	 * replayed by log recovery.
 	 */
 	list_for_each_entry_safe(resv, n, &xnr->resv_list, list) {
-		xfs_alloc_commit_autoreap(sc->tp, &resv->autoreap);
+		if (!xfs_is_shutdown(sc->mp))
+			xfs_alloc_commit_autoreap(sc->tp, &resv->autoreap);
 		list_del(&resv->list);
 		xfs_perag_put(resv->pag);
 		kfree(resv);
-- 
2.43.0