[PATCH v3 1/2] xfs: skip inode inactivation on a shut down mount

[PATCH v3 1/2] xfs: skip inode inactivation on a shut down mount

[Mikhail Lobanov]

XFS already declines to inactivate inodes on a shut down mount, but only
at queue time: xfs_inode_mark_reclaimable() calls
xfs_inode_needs_inactive(), which returns false when the mount is shut
down ("If the log isn't running, push inodes straight to reclaim"), and
then drops the dquots and marks the inode reclaimable directly.

An inode that was queued for background inactivation while the mount was
still live is not covered by that check: the inodegc worker still calls
xfs_inactive() on it even after the mount has been shut down in the
meantime.  Inactivation modifies persistent metadata and runs
transactions that cannot complete on a shut down mount, and it relies on
subsystems (e.g. quota) that a torn down, or never fully set up, mount
may not have available.

Honour the same invariant at gc time.  In xfs_inodegc_inactivate(), skip
xfs_inactive() when the mount is shut down and just make the inode
reclaimable.  As the inode then goes straight to reclaim, drop its dquots
in that case too - exactly as the straight-to-reclaim path in
xfs_inode_mark_reclaimable() already does - so the dquot references are
not leaked.  xfs_qm_dqdetach() is a no-op when no dquots are attached.

On its own this is a consistency fix with the existing queue-time
behaviour; it is also a prerequisite for shutting the mount down in the
xfs_mountfs() failure path in the following patch.

Fixes: ab23a7768739 ("xfs: per-cpu deferred inode inactivation queues")
Signed-off-by: Mikhail Lobanov <m.lobanov@rosa.ru>
---
v3: split out of the v2 single patch as a prep patch, and additionally
    drop the attached dquots in the shutdown branch to avoid leaking
    references when an inode queued while the mount was live is processed
    after a (normal) shutdown - spotted in review of v2.

v2: https://lore.kernel.org/linux-xfs/aiKA7vVQ_RxT_YOr@infradead.org/T/#t
v1: https://lore.kernel.org/linux-xfs/ah6BIsvEitNW5Edb@infradead.org/

 fs/xfs/xfs_icache.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 2040a9292ee6..1f725804be17 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1940,10 +1940,25 @@ static int
 xfs_inodegc_inactivate(
 	struct xfs_inode	*ip)
 {
-	int			error;
+	int			error = 0;

 	trace_xfs_inode_inactivating(ip);
-	error = xfs_inactive(ip);
+
+	/*
+	 * If the filesystem has been shut down - for example a mount that
+	 * failed after background inactivation was enabled - do not
+	 * inactivate the inode.  Inactivation modifies persistent metadata,
+	 * its transactions cannot complete on a shut down mount, and the
+	 * subsystems it relies on (e.g. quota, mp->m_quotainfo) may not be
+	 * set up.  Drop any attached dquots and make the inode reclaimable,
+	 * the same way xfs_inode_mark_reclaimable() does when it sends an
+	 * inode straight to reclaim.
+	 */
+	if (!xfs_is_shutdown(ip->i_mount))
+		error = xfs_inactive(ip);
+	else
+		/* Going straight to reclaim, so drop the dquots. */
+		xfs_qm_dqdetach(ip);
 	xfs_inodegc_set_reclaimable(ip);
 	return error;

--
2.43.0

[PATCH v3 2/2] xfs: shut down the filesystem on a failed mount

[Mikhail Lobanov]

A corrupt/crafted XFS image can make mount fail after background inode
inactivation has already been enabled.  xfs_mountfs() turns on inodegc
(xfs_inodegc_start()) right after log recovery, but the quota subsystem
(mp->m_quotainfo) is only allocated much later, in xfs_qm_newmount() /
xfs_qm_mount_quotas().  The quota accounting flags in mp->m_qflags are
parsed from the mount options before xfs_mountfs() even runs.

If the mount then aborts in between - e.g. xfs_rtmount_inodes() failing
with "failed to read RT inodes" - the unwind path flushes the inodegc
queue, which inactivates the inodes that are still queued, and
xfs_inactive() calls xfs_qm_dqattach().  That path trusts
XFS_IS_QUOTA_ON() (the flag is set) and dereferences the not yet
allocated mp->m_quotainfo:

  XFS (loop0): failed to read RT inodes
  Oops: general protection fault, probably for non-canonical address
        0xdffffc000000002a: 0000 [#1] PREEMPT SMP KASAN NOPTI
  KASAN: null-ptr-deref in range [0x0000000000000150-0x0000000000000157]
  Workqueue: xfs-inodegc/loop0 xfs_inodegc_worker
  RIP: 0010:__mutex_lock+0xfe/0x930
  Call Trace:
   xfs_qm_dqget_cache_lookup+0x63/0x7f0
   xfs_qm_dqget_inode+0x336/0x860
   xfs_qm_dqattach_one+0x232/0x4e0
   xfs_qm_dqattach_locked+0x2c6/0x470
   xfs_qm_dqattach+0x46/0x70
   xfs_inactive+0x988/0xe80
   xfs_inodegc_worker+0x27c/0x730

The NULL m_quotainfo deref is only one symptom.  The deeper problem is
that a failed mount should not be inactivating inodes at all: it must
not write to the (possibly corrupt, only partially set up) persistent
metadata of a filesystem we just refused to mount, and the subsystems
inactivation relies on may not be initialised.

Mark the filesystem shut down before flushing the inodegc queue in the
xfs_mountfs() failure path.  With the preceding patch a shut down mount
no longer inactivates the queued inodes: xfs_inodegc_inactivate() drops
them straight to reclaim instead.  They are still pulled down so reclaim
can free them (which is why the flush was added in commit ab23a7768739
("xfs: per-cpu deferred inode inactivation queues")), but without
touching the on-disk structures - matching that comment's own "pull down
all the state and flee" intent.

Use SHUTDOWN_META_IO_ERROR for the shutdown: it is the generic "cannot
safely touch metadata" reason already used elsewhere in this file and in
the xfs_ifree() failure path, and unlike SHUTDOWN_FORCE_UMOUNT it does
not log a misleading "User initiated shutdown received".  A failed mount
is not necessarily on-disk corruption (it can be a transient I/O or
resource error), so SHUTDOWN_CORRUPT_ONDISK would not be accurate either.

Found by fuzzing XFS with syzkaller (corrupt image mount); reproduced and
verified under QEMU/KASAN.

Fixes: ab23a7768739 ("xfs: per-cpu deferred inode inactivation queues")
Signed-off-by: Mikhail Lobanov <m.lobanov@rosa.ru>
---
v3: depend on the preceding "skip inode inactivation on a shut down
    mount" prep patch instead of doing both changes in one patch; use
    SHUTDOWN_META_IO_ERROR instead of SHUTDOWN_FORCE_UMOUNT (no
    misleading "User initiated shutdown" message, no message special
    casing); reflow the comment to stay within 80 columns.

v2: https://lore.kernel.org/linux-xfs/aiKA7vVQ_RxT_YOr@infradead.org/T/#t
v1: https://lore.kernel.org/linux-xfs/ah6BIsvEitNW5Edb@infradead.org/

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

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index b24195f570cd..37fb69165502 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1243,11 +1243,19 @@ xfs_mountfs(
 		xfs_irele(mp->m_metadirip);

 	/*
-	 * Inactivate all inodes that might still be in memory after a log
-	 * intent recovery failure so that reclaim can free them.  Metadata
-	 * inodes and the root directory shouldn't need inactivation, but the
-	 * mount failed for some reason, so pull down all the state and flee.
+	 * The mount has failed.  Mark the filesystem shut down so that any
+	 * inodes still queued for background inactivation are dropped
+	 * straight to reclaim instead of being inactivated: a failed mount
+	 * must not write to the (possibly corrupt, only partially set up)
+	 * persistent metadata, and parts of the mount it would need - e.g.
+	 * the quota subsystem (mp->m_quotainfo) - may never have been
+	 * initialised.
+	 *
+	 * Flush the queue so that those inodes are pulled down and reclaim
+	 * can free them; with the fs shut down xfs_inodegc_inactivate()
+	 * turns each one reclaimable without touching the on-disk structures.
 	 */
+	xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR);
 	xfs_inodegc_flush(mp);

 	/*
--
2.43.0

Re: [PATCH v3 2/2] xfs: shut down the filesystem on a failed mount

[kernel test robot]

Hi Mikhail,

kernel test robot noticed the following build warnings:

[auto build test WARNING on xfs-linux/for-next]
[also build test WARNING on linus/master v7.1-rc6 next-20260605]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mikhail-Lobanov/xfs-shut-down-the-filesystem-on-a-failed-mount/20260605-175001
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
patch link:    https://lore.kernel.org/r/20260605093222.8555-2-m.lobanov%40rosa.ru
patch subject: [PATCH v3 2/2] xfs: shut down the filesystem on a failed mount
config: i386-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20260605/202606051954.GRmELwA9-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260605/202606051954.GRmELwA9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606051954.GRmELwA9-lkp@intel.com/

All warnings (new ones prefixed by >>):

   fs/xfs/xfs_icache.c: In function 'xfs_inodegc_inactivate':
>> fs/xfs/xfs_icache.c:1961:36: warning: suggest braces around empty body in an 'else' statement [-Wempty-body]
    1961 |                 xfs_qm_dqdetach(ip);
         |                                    ^


vim +/else +1961 fs/xfs/xfs_icache.c

  1933	
  1934	/*
  1935	 * Free all speculative preallocations and possibly even the inode itself.
  1936	 * This is the last chance to make changes to an otherwise unreferenced file
  1937	 * before incore reclamation happens.
  1938	 */
  1939	static int
  1940	xfs_inodegc_inactivate(
  1941		struct xfs_inode	*ip)
  1942	{
  1943		int			error = 0;
  1944	
  1945		trace_xfs_inode_inactivating(ip);
  1946	
  1947		/*
  1948		 * If the filesystem has been shut down - for example a mount that
  1949		 * failed after background inactivation was enabled - do not
  1950		 * inactivate the inode.  Inactivation modifies persistent metadata,
  1951		 * its transactions cannot complete on a shut down mount, and the
  1952		 * subsystems it relies on (e.g. quota, mp->m_quotainfo) may not be
  1953		 * set up.  Drop any attached dquots and make the inode reclaimable,
  1954		 * the same way xfs_inode_mark_reclaimable() does when it sends an
  1955		 * inode straight to reclaim.
  1956		 */
  1957		if (!xfs_is_shutdown(ip->i_mount))
  1958			error = xfs_inactive(ip);
  1959		else
  1960			/* Going straight to reclaim, so drop the dquots. */
> 1961			xfs_qm_dqdetach(ip);
  1962		xfs_inodegc_set_reclaimable(ip);
  1963		return error;
  1964	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki