[PATCH] gfs2: Fix premature disposal of quota data with pending changes

[PATCH] gfs2: Fix premature disposal of quota data with pending changes

[Shuai Liu]

Quota data structures with a pending quota change (QDF_CHANGE set) hold
a reference to the quota data structure, which is dropped when the change
slot becomes unused in do_qc().  That reference is taken by __qd_hold()
when a change slot becomes used, but commit bb73ae8ff394 ("gfs2: Fix
initial quota data refcount") stopped taking it for the quota data
structures created by gfs2_quota_init() at mount time.

As a result, when gfs2_quota_sync() or gfs2_quota_unlock() syncs a parked
quota data structure with a pending change, do_qc() drops the reference
taken by qd_grab_sync() instead of the reference that should have been
held for the pending change.  This leaves the quota data structure with
a reference count of zero on the LRU list while the sync path is still
using it, so the shrinker can dispose of it before the sync path is done
with it, resulting in:

  gfs2: fsid=...: warning: assertion "!qd->qd_slot_ref" failed -
        function = gfs2_qd_dispose, file = fs/gfs2/quota.c, line = 130
  kernel BUG at fs/gfs2/quota.c:331!
  ...
  Call Trace:
   qd_put
   gfs2_quota_sync
   gfs2_quotad / gfs2_sync_fs

Fix this by taking the reference held for the pending quota change in
gfs2_quota_init() as well, and by dropping that reference (along with
the associated quota slot reference) in gfs2_quota_cleanup().

Reported-by: syzbot+0423714c06c369318794@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0423714c06c369318794
Fixes: bb73ae8ff394 ("gfs2: Fix initial quota data refcount")
Signed-off-by: Shuai Liu <847807867@qq.com>
---
The bug was located and the fix drafted with the help of an AI coding
assistant (GLM via the Trae IDE); the analysis, testing and submission
were reviewed and are certified by the human submitter.
---
 fs/gfs2/quota.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index b2984fb7b35c3..f8ac274b99db8 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1470,7 +1470,13 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
 			if (qd == NULL)
 				goto fail_brelse;
 
-			qd->qd_lockref.count = 0;
+			/*
+			 * The pending quota change holds a reference to the
+			 * quota data structure, just like when a change slot
+			 * becomes used in do_qc().  It is dropped when the
+			 * change slot becomes unused (in do_qc()) or in
+			 * gfs2_quota_cleanup().
+			 */
 			set_bit(QDF_CHANGE, &qd->qd_flags);
 			qd->qd_change = qc_change;
 			qd->qd_slot = slot;
@@ -1542,6 +1548,16 @@ void gfs2_quota_cleanup(struct gfs2_sbd *sdp)
 	spin_lock(&qd_lock);
 	list_for_each_entry(qd, &sdp->sd_quota_list, qd_list) {
 		spin_lock(&qd->qd_lockref.lock);
+		if (test_and_clear_bit(QDF_CHANGE, &qd->qd_flags)) {
+			/*
+			 * Drop the references held for the pending quota
+			 * change.  The change remains in the quota change
+			 * file and will be picked up again by
+			 * gfs2_quota_init() on the next mount.
+			 */
+			qd->qd_lockref.count--;
+			slot_put(qd);
+		}
 		if (qd->qd_lockref.count != 0) {
 			spin_unlock(&qd->qd_lockref.lock);
 			continue;
-- 
2.43.0