mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Jiacheng Xu" <stitch@zju.edu.cn>
To: "Andreas Gruenbacher" <agruenba@redhat.com>
Cc: gfs2@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH v2 1/5] gfs2: protect quota refresh from superblock teardown
Date: Fri, 21 Aug 2026 12:11:53 +0800 (GMT+08:00)	[thread overview]
Message-ID: <42b8a85f.162c4.1a02284a6da.Coremail.stitch@zju.edu.cn> (raw)
In-Reply-To: <6a91508.162bd.1a0228251e8.Coremail.stitch@zju.edu.cn>

The quota_refresh sysfs attributes become visible before fill_super()
has completed, so the quota inode can still be uninitialized when a
userspace write reaches gfs2_quota_refresh(). The same callback can
also race with superblock teardown.

Serialize the callback with the superblock lifetime using a read-side
s_umount lock. Use down_read_trylock() so the mount failure path can
remove the sysfs files while holding the write side of the lock without
deadlocking, and reject accesses before SB_ACTIVE is set.

Return -EAGAIN when the filesystem is not available for the callback.

Tested-by: Jiacheng Xu <stitch@zju.edu.cn>
Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
---
 fs/gfs2/quota.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 001c8b39ca55..d065b51950d7 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1384,19 +1384,36 @@ int gfs2_quota_sync(struct super_block *sb, int type)
 
 int gfs2_quota_refresh(struct gfs2_sbd *sdp, struct kqid qid)
 {
+       struct super_block *sb = sdp->sd_vfs;
        struct gfs2_quota_data *qd;
        struct gfs2_holder q_gh;
        int error;
 
+       /*
+        * The sysfs files are created before fill_super completes. Avoid
+        * blocking on s_umount because the mount failure path removes the
+        * sysfs files while holding it for writing.
+        */
+       if (!down_read_trylock(&sb->s_umount))
+               return -EAGAIN;
+
+       if (!(sb->s_flags & SB_ACTIVE)) {
+               error = -EAGAIN;
+               goto out_unlock;
+       }
+
        error = qd_get(sdp, qid, &qd);
        if (error)
-               return error;
+               goto out_unlock;
 
        error = do_glock(qd, FORCE, &q_gh);
        if (!error)
                gfs2_glock_dq_uninit(&q_gh);
 
        qd_put(qd);
+
+out_unlock:
+       up_read(&sb->s_umount);
        return error;
 }
 
-- 
2.25.1


> -----原始邮件-----
> 发件人: "Jiacheng Xu" <stitch@zju.edu.cn>
> 发送时间:2026-08-21 12:09:21 (星期五)
> 收件人: "Andreas Gruenbacher" <agruenba@redhat.com>
> 抄送: gfs2@lists.linux.dev, linux-kernel@vger.kernel.org
> 主题: [PATCH v2 0/5] gfs2: protect sysfs callbacks from superblock teardown
> 
> The GFS2 sysfs files become visible before fill_super() completes and
> remain present until after filesystem resources have been released.
> Consequently, callbacks that access quota, statfs, glock or journal
> state can race with mount failure rollback or unmount teardown.
> 
> The quota refresh fix was originally sent as a standalone [PATCH]. This
> version folds it into a complete series and adds the corresponding
> lifetime protection for the other affected sysfs callbacks.
> 
> All callbacks use down_read_trylock() on s_umount and verify SB_ACTIVE.
> Returning -EAGAIN avoids deadlock when mount failure or unmount holds
> the write side of s_umount while removing the sysfs files.
> 
> Changes in v2:
> - Folded the original quota refresh fix into a five-patch series.
> - Added statfs_sync, quota_sync, demote_rq and status fixes.
> 
> Jiacheng Xu (5):
>   gfs2: protect quota refresh from superblock teardown
>   gfs2: protect statfs sync sysfs callback
>   gfs2: protect quota sync sysfs callback
>   gfs2: protect demote requests during superblock teardown
>   gfs2: protect status sysfs reads during teardown
> 
>  fs/gfs2/quota.c | 19 ++++++++++++++-
>  fs/gfs2/sys.c   | 65 +++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 76 insertions(+), 8 deletions(-)
> 
> 
> base-commit: 0f23d56f17fdfc7db69d51f64c8b91bbab947aa9
> -- 
> 2.25.1
> 
> > -----原始邮件-----
> > 发件人: "Jiacheng Xu" <stitch@zju.edu.cn>
> > 发送时间:2026-08-19 15:01:07 (星期三)
> > 收件人: "Andreas Gruenbacher" <agruenba@redhat.com>
> > 抄送: gfs2@lists.linux.dev, linux-kernel@vger.kernel.org
> > 主题: [PATCH] gfs2: Fix NULL pointer dereference in quota refresh
> > 
> > The GFS2 sysfs files are registered before init_inodes() completes.
> > Consequently, the quota_refresh_user and quota_refresh_group sysfs
> > attributes can be accessed while sdp->sd_quota_inode has not been
> > initialized yet.
> > 
> > A concurrent write to quota_refresh_user may then call do_glock(), which
> > dereferences sdp->sd_quota_inode. This can result in a NULL pointer dereference 
> > in do_glock(). The same callback can also race with superblock
> > teardown and access data after the filesystem has started to shut down.
> > 
> > Moving sysfs registration after init_inodes() would avoid the initialization
> > window, but is not suitable because the lock manager may need the GFS2
> > sysfs files during the remaining mount sequence.
> > 
> > Serialize gfs2_quota_refresh() with the superblock lifetime instead.
> > Acquire s_umount for reading before accessing quota data. Mount failure and
> > unmount paths hold s_umount for writing, so this prevents the callback from
> > running while the superblock is being initialized or destroyed.
> > 
> > Use down_read_trylock() instead of down_read() because the mount failure
> > path may already hold s_umount for writing while removing the sysfs files.
> > Returning -EAGAIN allows the sysfs write to fail without introducing a
> > deadlock.
> > 
> > Also verify SB_ACTIVE after acquiring the read lock, since the sysfs
> > attributes become visible before the superblock is fully active.
> > 
> > The reproducer of the issue is attached. After applying this patch, 
> > the reproducer no longer triggers the kernel crash. 
> > 
> > Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
> > Tested-by: Jiacheng Xu <stitch@zju.edu.cn>
> > ---
> > fs/gfs2/quota.c | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> > index 001c8b39ca55..d50534ed9379 100644
> > --- a/fs/gfs2/quota.c
> > +++ b/fs/gfs2/quota.c
> > @@ -1384,19 +1384,36 @@ int gfs2_quota_sync(struct super_block *sb, int type)
> > 
> > int gfs2_quota_refresh(struct gfs2_sbd *sdp, struct kqid qid)
> > {
> > +     struct super_block *sb = sdp->sd_vfs;
> >       struct gfs2_quota_data *qd;
> >       struct gfs2_holder q_gh;
> >       int error;
> > 
> > +     /*
> > +      * The sysfs files are created before fill_super completes. Avoid
> > +      * blocking on s_umount because the mount failure path removes the
> > +      * sysfs files while holding it for writing.
> > +      */
> > +     if (!down_read_trylock(&sb->s_umount))
> > +             return -EAGAIN;
> > +
> > +     if (!(sb->s_flags & SB_ACTIVE)) {
> > +             error = -EAGAIN;
> > +             goto out_unlock;
> > +     }
> > +
> >       error = qd_get(sdp, qid, &qd);
> >       if (error)
> > -             return error;
> > +             goto out_unlock;
> > 
> >       error = do_glock(qd, FORCE, &q_gh);
> >       if (!error)
> >             gfs2_glock_dq_uninit(&q_gh);
> > 
> >       qd_put(qd);
> > 
> > +out_unlock:
> > +     up_read(&sb->s_umount);
> >       return error;
> > }

  reply	other threads:[~2026-08-21  4:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-19  7:01 [PATCH] gfs2: Fix NULL pointer dereference in quota refresh Jiacheng Xu
2026-08-21  4:09 ` [PATCH v2 0/5] gfs2: protect sysfs callbacks from superblock teardown Jiacheng Xu
2026-08-21  4:11   ` Jiacheng Xu [this message]
2026-08-21  4:14   ` [PATCH v2 2/5] gfs2: protect statfs sync sysfs callback Jiacheng Xu
2026-08-21  4:14   ` [PATCH v2 3/5] gfs2: protect quota " Jiacheng Xu
2026-08-21  4:15   ` [PATCH v2 4/5] gfs2: protect demote requests during superblock Jiacheng Xu
2026-08-21  4:16   ` [PATCH v2 5/5] gfs2: protect status sysfs reads during teardown Jiacheng Xu
2026-08-27 19:16 ` [PATCH v2 0/5] gfs2: protect sysfs callbacks from superblock teardown Andreas Gruenbacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=42b8a85f.162c4.1a02284a6da.Coremail.stitch@zju.edu.cn \
    --to=stitch@zju.edu.cn \
    --cc=agruenba@redhat.com \
    --cc=gfs2@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®