From: eadaivs <eada...@sina.com> Hello Andreas,
On Mon, 30 Jan 2023 15:32:42 +0100 <agrue...@redhat.com> wrote: > I can see that there is a problem in the gfs2 quota code, but this > unfortunately doesn't look like an acceptable fix. A better approach > would be to use proper reference counting for gfs2_quota_data objects. > In this case, gfs2_quota_sync() is still holding a reference, so the > underlying object shouldn't be freed. > > Fixing this properly will require more than a handful of lines. Before clearing quota, wait for the qd synchronization operation to end. Add reference count in gfs2_quota_data and perform qd synchronization to control whether qd is in use. If so, temporarily clear quota. Link: https://lore.kernel.org/all/0000000000002b5e2405f14e8...@google.com Reported-and-tested-by: syzbot+3f6a670108ce43356...@syzkaller.appspotmail.com Signed-off-by: eadaivs <eada...@sina.com> --- fs/gfs2/incore.h | 1 + fs/gfs2/quota.c | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h index c26765080f28..61549bd95b32 100644 --- a/fs/gfs2/incore.h +++ b/fs/gfs2/incore.h @@ -463,6 +463,7 @@ struct gfs2_quota_data { u64 qd_sync_gen; unsigned long qd_last_warn; struct rcu_head qd_rcu; + unsigned int ref_cnt; }; enum { diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c index 1ed17226d9ed..dca4be078ce8 100644 --- a/fs/gfs2/quota.c +++ b/fs/gfs2/quota.c @@ -478,6 +478,7 @@ static int qd_fish(struct gfs2_sbd *sdp, struct gfs2_quota_data **qdp) qd_put(qd); return error; } + WRITE_ONCE(qd->ref_cnt, READ_ONCE(qd->ref_cnt) + 1); } *qdp = qd; @@ -493,6 +494,7 @@ static void qd_unlock(struct gfs2_quota_data *qd) bh_put(qd); slot_put(qd); qd_put(qd); + WRITE_ONCE(qd->ref_cnt, READ_ONCE(qd->ref_cnt) - 1); } static int qdsb_get(struct gfs2_sbd *sdp, struct kqid qid, @@ -1422,6 +1424,7 @@ int gfs2_quota_init(struct gfs2_sbd *sdp) qd->qd_change = qc_change; qd->qd_slot = slot; qd->qd_slot_count = 1; + WRITE_ONCE(qd->ref_cnt, 0); spin_lock(&qd_lock); BUG_ON(test_and_set_bit(slot, sdp->sd_quota_bitmap)); @@ -1454,11 +1457,13 @@ int gfs2_quota_init(struct gfs2_sbd *sdp) void gfs2_quota_cleanup(struct gfs2_sbd *sdp) { struct list_head *head = &sdp->sd_quota_list; - struct gfs2_quota_data *qd; + struct gfs2_quota_data *qd, *safe; +retry: spin_lock(&qd_lock); - while (!list_empty(head)) { - qd = list_last_entry(head, struct gfs2_quota_data, qd_list); + list_for_each_entry_safe(qd, safe, head, qd_list) { + if (READ_ONCE(qd->ref_cnt)) + continue; list_del(&qd->qd_list); @@ -1482,7 +1487,10 @@ void gfs2_quota_cleanup(struct gfs2_sbd *sdp) } spin_unlock(&qd_lock); - gfs2_assert_warn(sdp, !atomic_read(&sdp->sd_quota_count)); + if (atomic_read(&sdp->sd_quota_count)) { + schedule_timeout_uninterruptible(HZ); + goto retry; + } kvfree(sdp->sd_quota_bitmap); sdp->sd_quota_bitmap = NULL; -- 2.41.0
