[PATCH v2] xfs: drain inodegc before quota teardown on mount failure

[PATCH v2] xfs: drain inodegc before quota teardown on mount failure

[Zhang Cen]

xfs_mountfs() starts background inodegc before mount completion. If a
later setup step fails after quota setup, the failure unwind can destroy
quota state while an inodegc worker is still able to resume
xfs_inactive().

There are two related windows. First, out_agresv can still call
xfs_qm_unmount_quotas() while m_qflags allow dqattach, so dqget can
still need quota inodes that are being released. Second, failures after
xfs_qm_newmount() can destroy mp->m_quotainfo before inodegc has been
drained, while xfs_qm_dqattach() can still follow m_qflags into the
quota cache.

Normal unmount and the quotacheck abort path already drain inodegc before
quota teardown. Do the same for mount failure. Drain inodegc at out_agresv
before dropping AG reservations and quota inodes. Add an out_qm_unmount
label above out_rtunmount so failures after xfs_qm_newmount() drain
inodegc before xfs_qm_unmount() destroys quotainfo.

This was reproduced in a KASAN QEMU setup that forced a late
xfs_mountfs() failure after quota setup and widened the inodegc/dqattach
window. The failure reported a general protection fault in
xfs_qm_dqget_cache_lookup() from the inodegc worker path:

  xfs_qm_dqget_cache_lookup()
  xfs_qm_dqget_inode()
  xfs_qm_dqattach_locked()
  xfs_qm_dqattach()
  xfs_free_eofblocks()
  xfs_inactive()
  xfs_inodegc_worker()

Fixes: ab23a7768739 ("xfs: per-cpu deferred inode inactivation queues")
Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
---
v2:
- Add an out_qm_unmount label above out_rtunmount, as Christoph suggested.
- Keep the out_agresv inodegc drain before xfs_qm_unmount_quotas().
- Describe the KASAN/QEMU validation setup and fix trailer spacing.

 fs/xfs/xfs_mount.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index b24195f570cd8..39ce56e866060 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1156,7 +1156,7 @@ xfs_mountfs(
 	xfs_fs_unreserve_ag_blocks(mp);
 	if (error) {
 		xfs_warn(mp, "log mount finish failed");
-		goto out_rtunmount;
+		goto out_qm_unmount;
 	}
 
 	/*
@@ -1174,7 +1174,7 @@ xfs_mountfs(
 	if (xfs_has_zoned(mp)) {
 		error = xfs_mount_zones(mp);
 		if (error)
-			goto out_rtunmount;
+			goto out_qm_unmount;
 	}
 
 	/*
@@ -1228,16 +1228,29 @@ xfs_mountfs(
 	return 0;
 
  out_agresv:
+	/*
+	 * Drain ordinary inodegc before we drop per-AG reservations or tear
+	 * down quota state.  Background inactivation can still attach dquots
+	 * here, and dqget can still need the quota inodes.
+	 */
+	xfs_inodegc_flush(mp);
 	xfs_fs_unreserve_ag_blocks(mp);
 	xfs_qm_unmount_quotas(mp);
 	if (xfs_has_zoned(mp))
 		xfs_unmount_zones(mp);
+ out_qm_unmount:
+	/*
+	 * xfs_qm_newmount() can bring quota inodes into the mount.  Drain
+	 * inodegc before destroying quotainfo because background inactivation
+	 * can still attach dquots.
+	 */
+	xfs_inodegc_flush(mp);
+	/* Clean out dquots that might be in memory after quotacheck. */
+	xfs_qm_unmount(mp);
  out_rtunmount:
 	xfs_rtunmount_inodes(mp);
  out_rele_rip:
 	xfs_irele(rip);
-	/* Clean out dquots that might be in memory after quotacheck. */
-	xfs_qm_unmount(mp);
  out_free_metadir:
 	if (mp->m_metadirip)
 		xfs_irele(mp->m_metadirip);

base-commit: eb3f4b7426cfd2b79d65b7d37155480b32259a11
-- 
2.43.0