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

[PATCH v4 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>
---
v4: brace both branches of the if/else.  Without CONFIG_XFS_QUOTA
    xfs_qm_dqdetach() expands to nothing, leaving the else with an empty
    body, which trips -Wempty-body on a W=1 build (i386-allnoconfig,
    reported by the kernel test robot).

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 | 20 ++++++++++++++++++--
 1 file changed, 18 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,26 @@ 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 v4 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>
---
v4: no change to this patch; resent as part of the series.  v4 of patch
    1/2 braces the if/else to silence a -Wempty-body W=1 warning.

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 v4 2/2] xfs: shut down the filesystem on a failed mount

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

[Christoph Hellwig]

On Sun, Jun 07, 2026 at 09:30:25PM +0300, Mikhail Lobanov wrote:
> v4: brace both branches of the if/else.  Without CONFIG_XFS_QUOTA
>     xfs_qm_dqdetach() expands to nothing, leaving the else with an empty
>     body, which trips -Wempty-body on a W=1 build (i386-allnoconfig,
>     reported by the kernel test robot).

We should really turn these stubs from macros into inlines, but..

> @@ -1940,10 +1940,26 @@ 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);
> +	}

It might be easier to just move the shutdown check and xfs_qm_dqdetach
cal into the beginning of xfs_inactive as:

	if (xfs_is_shutdown(mp)
		goto out;

after initializing mp at declaration time at the top of the function.

But this is also still missing my other comment:  xfs_qm_dqdetach
will still cause a NULL pointer dereference if quotas haven't been
set up yet or torn down already.  So we'll probably want another
patch to exist early in xfs_qm_dqdetach for that case.

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

[Mikhail Lobanov]

Hi Christoph,

> we'll probably want another patch to exit early in xfs_qm_dqdetach for
> that case

Before I send that, I went looking for a path that actually reaches it and
couldn't find one in the current tree - so I wanted to check whether you
had a specific case in mind or meant it as hardening.

xfs_qm_dqdetach() only touches the quota subsystem (the final
xfs_qm_dqrele() puts the dquot back on qi->qi_lru) when a dquot is actually
attached, and a dquot can only be attached if m_quotainfo was non-NULL at
attach time.  That leaves two windows:

  - Before setup (failed/partial mount): m_quotainfo is NULL the whole
    time, so nothing ever attaches a dquot - the existing "no dquots
    attached" early return covers it.  (That's the window the v5 series
    fixes, but it faults in dqattach, not dqdetach.)

  - After teardown: every site that frees m_quotainfo reaches it only once
    the attached dquots are already gone - unmount detaches the quota
    inodes and reclaims the rest before xfs_qm_destroy_quotainfo(), and the
    quotacheck-failure path flushes inodegc before dqpurge (0c7273e494dd).
    Runtime quota off doesn't free m_quotainfo at all.

So I don't see a reachable NULL deref today.  I'm happy to still send the
guard as belt-and-suspenders - an early "if (!ip->i_mount->m_quotainfo)
return;", symmetric with the attach-side quota checks, with no Fixes: tag
since there's no regression to point at - if you'd like it for robustness.
Or if you have a path in mind that I've missed, point me at it and I'll fix
the actual teardown ordering instead.

Thanks,
Mikhail

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

[Christoph Hellwig]

On Wed, Jun 10, 2026 at 10:25:38PM +0300, Mikhail Lobanov wrote:
> Hi Christoph,
> 
> > we'll probably want another patch to exit early in xfs_qm_dqdetach for
> > that case
> 
> Before I send that, I went looking for a path that actually reaches it and
> couldn't find one in the current tree - so I wanted to check whether you
> had a specific case in mind or meant it as hardening.
> 
> xfs_qm_dqdetach() only touches the quota subsystem (the final
> xfs_qm_dqrele() puts the dquot back on qi->qi_lru) when a dquot is actually
> attached, and a dquot can only be attached if m_quotainfo was non-NULL at
> attach time.  That leaves two windows:

You're right.  I did rememer review tools comments about this, but
they do look bogus.