[PATCH] xfs: skip dquot attach when m_quotainfo is not set up

[Mikhail Lobanov]

XFS quota accounting flags (XFS_*QUOTA_ACCT in mp->m_qflags) are parsed
from the mount options before xfs_mountfs() runs, but the quota
subsystem itself (mp->m_quotainfo) is only allocated later, in
xfs_qm_mount_quotas().  Background inode inactivation (inodegc) is
enabled earlier still, in xfs_mountfs() right after log recovery.

As a result there is a window during mount where XFS_IS_QUOTA_ON() is
already true while mp->m_quotainfo is still NULL.  The same inconsistency
exists during teardown, when m_quotainfo is freed before the quota flags
are cleared.  If a background xfs_inodegc_worker inactivates an inode in
that window -- easily triggered by mounting a crafted/corrupt image where
the mount aborts after inodegc has been enabled (e.g. xfs_rtmount_inodes()
failing with "failed to read RT inodes") -- xfs_inactive() calls
xfs_qm_dqattach(), which trusts XFS_IS_QUOTA_ON() and goes on to
dereference the NULL mp->m_quotainfo in xfs_qm_dqget_inode():

  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

mutex_lock() faults on &qi->qi_tree_lock (offset 0x150) with qi == NULL.

When mp->m_quotainfo is NULL there are no in-core dquots to attach, so
the attach must simply be skipped.  Guard xfs_qm_need_dqattach() -- the
single gate every xfs_qm_dqattach()/xfs_qm_dqattach_locked() caller
passes through before reaching xfs_qm_dqget_inode() -- with an explicit
mp->m_quotainfo check.  Once quotas are fully set up m_quotainfo stays
non-NULL until quotas are turned off (which also clears the quota flags),
so the new check is a no-op for a normally mounted quota filesystem;
quota accounting and enforcement are unaffected.

The existing xfs_inodegc_flush() added by commit 0c7273e494dd ("xfs:
quotacheck failure can race with background inode inactivation") only
covers tearing down quotas after a failed quotacheck and does not help
here, because in this case quotacheck is never reached.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller. 

Fixes: ab23a7768739 ("xfs: per-cpu deferred inode inactivation queues")
Signed-off-by: Mikhail Lobanov <m.lobanov@xxxxxxx>
---
 fs/xfs/xfs_qm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index ee2530fabeeb..6a4e8d6fb806 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -310,6 +310,19 @@ xfs_qm_need_dqattach(

 	if (!XFS_IS_QUOTA_ON(mp))
 		return false;
+	/*
+	 * The quota accounting flags in m_qflags are set from the mount options
+	 * before xfs_mountfs() runs, but the quota subsystem (mp->m_quotainfo)
+	 * is only initialised later in xfs_qm_mount_quotas(); it is also torn
+	 * down (and m_quotainfo set to NULL) before those flags are cleared when
+	 * quotas are turned off.  During those windows a background inodegc
+	 * worker inactivating an inode can reach here with XFS_IS_QUOTA_ON()
+	 * true but no quotainfo to look dquots up in, which would lead to a NULL
+	 * pointer dereference of mp->m_quotainfo in xfs_qm_dqget_inode().  There
+	 * are no dquots to attach in that state, so skip the attach.
+	 */
+	if (!mp->m_quotainfo)
+		return false;
 	if (!XFS_NOT_DQATTACHED(mp, ip))
 		return false;
 	if (xfs_is_quota_inode(&mp->m_sb, ip->i_ino))
--
2.43.0