gfs2: fix quota init duplicate scan

gfs2_quota_init() checks for duplicate quota_change IDs while holding
qd_lock and the quota hash bucket bitlock. That path used
gfs2_qd_search_bucket(), which takes a lockref reference via
lockref_get_not_dead().

On PREEMPT_RT this may sleep, which is not allowed under the bucket
bitlock, triggering "sleeping function called from invalid context".

Use a no-ref bucket lookup in this path, then continue duplicate
handling without taking a lockref there.

Refactor gfs2_qd_search_bucket() to build on top of the no-ref helper
so lookup traversal stays in one place.

This patch fixes a bug reported by syzbot.

Reported-by: syzbot+642d0561f78362d67d3f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=642d0561f78362d67d3f
Tested-by: syzbot+642d0561f78362d67d3f@syzkaller.appspotmail.com
Signed-off-by: Jie Wang <jie.wang@intel.com>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 5290865f27f149ec53663925e8501b44cf1da70a..934397248fe76c5839d4a0dbf4984055c2150e9d 100644 (file)
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -254,9 +254,13 @@ fail:
        return NULL;
 }
 
-static struct gfs2_quota_data *gfs2_qd_search_bucket(unsigned int hash,
-                                                    const struct gfs2_sbd *sdp,
-                                                    struct kqid qid)
+/*
+ * Lookup variant for callers which already hold qd_lock + bucket lock.
+ */
+static struct gfs2_quota_data *
+gfs2_qd_search_bucket_noref(unsigned int hash,
+                           const struct gfs2_sbd *sdp,
+                           struct kqid qid)
 {
        struct gfs2_quota_data *qd;
        struct hlist_bl_node *h;
@@ -264,12 +268,22 @@ static struct gfs2_quota_data *gfs2_qd_search_bucket(unsigned int hash,
        hlist_bl_for_each_entry_rcu(qd, h, &qd_hash_table[hash], qd_hlist) {
                if (!qid_eq(qd->qd_id, qid))
                        continue;
-               if (qd->qd_sbd != sdp)
-                       continue;
-               if (lockref_get_not_dead(&qd->qd_lockref)) {
-                       list_lru_del_obj(&gfs2_qd_lru, &qd->qd_lru);
+               if (qd->qd_sbd == sdp)
                        return qd;
-               }
+       }
+
+       return NULL;
+}
+
+static struct gfs2_quota_data *
+gfs2_qd_search_bucket(unsigned int hash, const struct gfs2_sbd *sdp, struct kqid qid)
+{
+       struct gfs2_quota_data *qd;
+
+       qd = gfs2_qd_search_bucket_noref(hash, sdp, qid);
+       if (qd && lockref_get_not_dead(&qd->qd_lockref)) {
+               list_lru_del_obj(&gfs2_qd_lru, &qd->qd_lru);
+               return qd;
        }
 
        return NULL;
@@ -1458,7 +1472,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
 
                        spin_lock(&qd_lock);
                        spin_lock_bucket(hash);
-                       old_qd = gfs2_qd_search_bucket(hash, sdp, qc_id);
+                       old_qd = gfs2_qd_search_bucket_noref(hash, sdp, qc_id);
                        if (old_qd) {
                                fs_err(sdp, "Corruption found in quota_change%u"
                                            "file: duplicate identifier in "
@@ -1467,7 +1481,6 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
 
                                spin_unlock_bucket(hash);
                                spin_unlock(&qd_lock);
-                               qd_put(old_qd);
 
                                gfs2_glock_put(qd->qd_gl);
                                kmem_cache_free(gfs2_quotad_cachep, qd);