[PATCH] gfs2: fix quota init duplicate scan

[PATCH] gfs2: fix quota init duplicate scan

[Jie Wang]

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.

Also save the current qc pointer before iterator advance, and clear that
saved slot on duplicate so the correct on-disk entry is zeroed.

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>
---
 fs/gfs2/quota.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 5290865f27f1..0191ba211670 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -275,6 +275,25 @@ static struct gfs2_quota_data *gfs2_qd_search_bucket(unsigned int hash,
 	return NULL;
 }
 
+/*
+ * 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;
+
+	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)
+			return qd;
+	}
+
+	return NULL;
+}
+
 
 static int qd_get(struct gfs2_sbd *sdp, struct kqid qid,
 		  struct gfs2_quota_data **qdp)
@@ -1435,6 +1454,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
 		for (y = 0; y < sdp->sd_qc_per_block && slot < sdp->sd_quota_slots;
 		     y++, slot++) {
 			struct gfs2_quota_data *old_qd, *qd;
+			struct gfs2_quota_change *dup_qc = qc;
 			s64 qc_change = be64_to_cpu(qc->qc_change);
 			u32 qc_flags = be32_to_cpu(qc->qc_flags);
 			enum quota_type qtype = (qc_flags & GFS2_QCF_USER) ?
@@ -1458,23 +1478,22 @@ 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);
+			spin_unlock_bucket(hash);
 			if (old_qd) {
 				fs_err(sdp, "Corruption found in quota_change%u"
 					    "file: duplicate identifier in "
 					    "slot %u\n",
 					    sdp->sd_jdesc->jd_jid, slot);
 
-				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);
 
 				/* zero out the duplicate slot */
 				lock_buffer(bh);
-				memset(qc, 0, sizeof(*qc));
+				memset(dup_qc, 0, sizeof(*dup_qc));
 				mark_buffer_dirty(bh);
 				unlock_buffer(bh);
 
@@ -1483,6 +1502,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
 			BUG_ON(test_and_set_bit(slot, sdp->sd_quota_bitmap));
 			list_add(&qd->qd_list, &sdp->sd_quota_list);
 			atomic_inc(&sdp->sd_quota_count);
+			spin_lock_bucket(hash);
 			hlist_bl_add_head_rcu(&qd->qd_hlist, &qd_hash_table[hash]);
 			spin_unlock_bucket(hash);
 			spin_unlock(&qd_lock);
-- 
2.34.1

Re: [PATCH] gfs2: fix quota init duplicate scan

[Andreas Gruenbacher]

Hello,

thank you for the patch.

On Mon, Apr 20, 2026 at 5:20 AM Jie Wang <jie.wang@intel.com> wrote:
> 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.

That should work ... but with that, can you at least implement
gfs2_qd_search_bucket() on top of gfs2_qd_search_bucket_noref()? Or
better yet, convert qd_hash_table into an rhashtable, like
gl_hash_table already is? That should allow a normal ref taking lookup
even on PREEMPT_RT.

> Also save the current qc pointer before iterator advance, and clear that
> saved slot on duplicate so the correct on-disk entry is zeroed.

This needs to go into a separate patch. Also, why not put the 'qc++'
into the third expression of the surrounding for loop?

> 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>
> ---
>  fs/gfs2/quota.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index 5290865f27f1..0191ba211670 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -275,6 +275,25 @@ static struct gfs2_quota_data *gfs2_qd_search_bucket(unsigned int hash,
>         return NULL;
>  }
>
> +/*
> + * 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;
> +
> +       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)
> +                       return qd;
> +       }
> +
> +       return NULL;
> +}
> +
>
>  static int qd_get(struct gfs2_sbd *sdp, struct kqid qid,
>                   struct gfs2_quota_data **qdp)
> @@ -1435,6 +1454,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
>                 for (y = 0; y < sdp->sd_qc_per_block && slot < sdp->sd_quota_slots;
>                      y++, slot++) {
>                         struct gfs2_quota_data *old_qd, *qd;
> +                       struct gfs2_quota_change *dup_qc = qc;
>                         s64 qc_change = be64_to_cpu(qc->qc_change);
>                         u32 qc_flags = be32_to_cpu(qc->qc_flags);
>                         enum quota_type qtype = (qc_flags & GFS2_QCF_USER) ?
> @@ -1458,23 +1478,22 @@ 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);
> +                       spin_unlock_bucket(hash);
>                         if (old_qd) {
>                                 fs_err(sdp, "Corruption found in quota_change%u"
>                                             "file: duplicate identifier in "
>                                             "slot %u\n",
>                                             sdp->sd_jdesc->jd_jid, slot);
>
> -                               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);
>
>                                 /* zero out the duplicate slot */
>                                 lock_buffer(bh);
> -                               memset(qc, 0, sizeof(*qc));
> +                               memset(dup_qc, 0, sizeof(*dup_qc));
>                                 mark_buffer_dirty(bh);
>                                 unlock_buffer(bh);
>
> @@ -1483,6 +1502,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
>                         BUG_ON(test_and_set_bit(slot, sdp->sd_quota_bitmap));
>                         list_add(&qd->qd_list, &sdp->sd_quota_list);
>                         atomic_inc(&sdp->sd_quota_count);
> +                       spin_lock_bucket(hash);
>                         hlist_bl_add_head_rcu(&qd->qd_hlist, &qd_hash_table[hash]);
>                         spin_unlock_bucket(hash);
>                         spin_unlock(&qd_lock);
> --
> 2.34.1
>

Thanks,
Andreas

[PATCH v2 0/2] gfs2: fix quota init duplicate scan on PREEMPT_RT

[Jie Wang]

This series addresses the PREEMPT_RT issue reported by syzbot in
gfs2_quota_init() duplicate-id handling.

On PREEMPT_RT, taking a lockref reference under the quota hash bucket
bitlock may sleep and trigger "sleeping function called from invalid
context".

Patch 1 fixes that path by using a no-ref lookup, and implements
gfs2_qd_search_bucket() on top of the no-ref helper.

Patch 2 keeps duplicate-slot zeroing correctness adjustment separate,
and expresses it by moving qc++ to the for-loop third expression.

Changes in v2:
- Split v1 into two patches as requested.
- Implement gfs2_qd_search_bucket() on top of
  gfs2_qd_search_bucket_noref().
- Move qc++ into the for-loop third expression.

The qd_hash_table -> rhashtable conversion is a larger refactor and is
kept out of this bugfix series.

Jie Wang (2):
  gfs2: fix quota init duplicate scan
  gfs2: move quota_init qc iterator increment

 fs/gfs2/quota.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] gfs2: fix quota init duplicate scan

[Jie Wang]

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>
---
 fs/gfs2/quota.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 5290865f27f1..df1cb99c3344 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -254,9 +254,13 @@ static struct gfs2_quota_data *qd_alloc(unsigned hash, struct gfs2_sbd *sdp, str
 	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,16 +1472,15 @@ 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);
+			spin_unlock_bucket(hash);
 			if (old_qd) {
 				fs_err(sdp, "Corruption found in quota_change%u"
 					    "file: duplicate identifier in "
 					    "slot %u\n",
 					    sdp->sd_jdesc->jd_jid, slot);
 
-				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);
@@ -1483,6 +1496,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
 			BUG_ON(test_and_set_bit(slot, sdp->sd_quota_bitmap));
 			list_add(&qd->qd_list, &sdp->sd_quota_list);
 			atomic_inc(&sdp->sd_quota_count);
+			spin_lock_bucket(hash);
 			hlist_bl_add_head_rcu(&qd->qd_hlist, &qd_hash_table[hash]);
 			spin_unlock_bucket(hash);
 			spin_unlock(&qd_lock);
-- 
2.34.1

Re: [PATCH v2 1/2] gfs2: fix quota init duplicate scan

[Andreas Gruenbacher]

Hello,

this is looking good except for one minor detail (see below).

On Tue, Apr 21, 2026 at 10:44 AM Jie Wang <jie.wang@intel.com> wrote:
> 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>
> ---
>  fs/gfs2/quota.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index 5290865f27f1..df1cb99c3344 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -254,9 +254,13 @@ static struct gfs2_quota_data *qd_alloc(unsigned hash, struct gfs2_sbd *sdp, str
>         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,16 +1472,15 @@ 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);
> +                       spin_unlock_bucket(hash);
>                         if (old_qd) {
>                                 fs_err(sdp, "Corruption found in quota_change%u"
>                                             "file: duplicate identifier in "
>                                             "slot %u\n",
>                                             sdp->sd_jdesc->jd_jid, slot);
>
> -                               spin_unlock_bucket(hash);

Why didn't you just leave this spin_unlock_bucket() call inside the if
statement?

>                                 spin_unlock(&qd_lock);
> -                               qd_put(old_qd);
>
>                                 gfs2_glock_put(qd->qd_gl);
>                                 kmem_cache_free(gfs2_quotad_cachep, qd);
> @@ -1483,6 +1496,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
>                         BUG_ON(test_and_set_bit(slot, sdp->sd_quota_bitmap));
>                         list_add(&qd->qd_list, &sdp->sd_quota_list);
>                         atomic_inc(&sdp->sd_quota_count);
> +                       spin_lock_bucket(hash);
>                         hlist_bl_add_head_rcu(&qd->qd_hlist, &qd_hash_table[hash]);
>                         spin_unlock_bucket(hash);
>                         spin_unlock(&qd_lock);
> --
> 2.34.1
>

Thanks,
Andreas

[PATCH v3] gfs2: fix quota init duplicate scan

[Jie Wang]

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>
---
v3:
- Keep spin_unlock_bucket(hash) in the if (old_qd) path.
- Drop the extra spin_lock_bucket(hash) before insertion.
 
 fs/gfs2/quota.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 5290865f27f1..934397248fe7 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -254,9 +254,13 @@ static struct gfs2_quota_data *qd_alloc(unsigned hash, struct gfs2_sbd *sdp, str
 	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);
-- 
2.34.1

Re: [PATCH v3] gfs2: fix quota init duplicate scan

[Andreas Gruenbacher]

On Thu, Apr 23, 2026 at 7:52 AM Jie Wang <jie.wang@intel.com> wrote:
> 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>
> ---
> v3:
> - Keep spin_unlock_bucket(hash) in the if (old_qd) path.
> - Drop the extra spin_lock_bucket(hash) before insertion.
>
>  fs/gfs2/quota.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index 5290865f27f1..934397248fe7 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -254,9 +254,13 @@ static struct gfs2_quota_data *qd_alloc(unsigned hash, struct gfs2_sbd *sdp, str
>         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);
> --
> 2.34.1
>

Thanks, I'll add this.

Andreas

[PATCH v2 2/2] gfs2: move quota_init qc iterator increment

[Jie Wang]

Move qc++ from the loop body into the for-loop increment
expression in gfs2_quota_init().

This keeps iterator progression explicit and avoids mixing pointer
advance with duplicate-slot handling in the loop body.

Signed-off-by: Jie Wang <jie.wang@intel.com>
---
 fs/gfs2/quota.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index df1cb99c3344..b9e13fe286be 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1447,7 +1447,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
 
 		qc = (struct gfs2_quota_change *)(bh->b_data + sizeof(struct gfs2_meta_header));
 		for (y = 0; y < sdp->sd_qc_per_block && slot < sdp->sd_quota_slots;
-		     y++, slot++) {
+		     y++, slot++, qc++) {
 			struct gfs2_quota_data *old_qd, *qd;
 			s64 qc_change = be64_to_cpu(qc->qc_change);
 			u32 qc_flags = be32_to_cpu(qc->qc_flags);
@@ -1455,7 +1455,6 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
 						USRQUOTA : GRPQUOTA;
 			struct kqid qc_id = make_kqid(&init_user_ns, qtype,
 						      be32_to_cpu(qc->qc_id));
-			qc++;
 			if (!qc_change)
 				continue;
 
-- 
2.34.1

Re: [PATCH v2 2/2] gfs2: move quota_init qc iterator increment

[Andreas Gruenbacher]

On Tue, Apr 21, 2026 at 10:44 AM Jie Wang <jie.wang@intel.com> wrote:
> Move qc++ from the loop body into the for-loop increment
> expression in gfs2_quota_init().
>
> This keeps iterator progression explicit and avoids mixing pointer
> advance with duplicate-slot handling in the loop body.
>
> Signed-off-by: Jie Wang <jie.wang@intel.com>
> ---
>  fs/gfs2/quota.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index df1cb99c3344..b9e13fe286be 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -1447,7 +1447,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
>
>                 qc = (struct gfs2_quota_change *)(bh->b_data + sizeof(struct gfs2_meta_header));
>                 for (y = 0; y < sdp->sd_qc_per_block && slot < sdp->sd_quota_slots;
> -                    y++, slot++) {
> +                    y++, slot++, qc++) {
>                         struct gfs2_quota_data *old_qd, *qd;
>                         s64 qc_change = be64_to_cpu(qc->qc_change);
>                         u32 qc_flags = be32_to_cpu(qc->qc_flags);
> @@ -1455,7 +1455,6 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
>                                                 USRQUOTA : GRPQUOTA;
>                         struct kqid qc_id = make_kqid(&init_user_ns, qtype,
>                                                       be32_to_cpu(qc->qc_id));
> -                       qc++;
>                         if (!qc_change)
>                                 continue;
>
> --
> 2.34.1
>

Thanks, I'll add this to for-next once -rc1 is out.

Andreas