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

Hello,

thank you for the patch.

On Mon, Apr 20, 2026 at 5:20 AM Jie Wang <[email protected]> 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: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=642d0561f78362d67d3f
> Tested-by: [email protected]
> Signed-off-by: Jie Wang <[email protected]>
> ---
>  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