[PATCH v2 2/5] gfs2: protect statfs sync sysfs callback

[Jiacheng Xu]

The statfs_sync sysfs file remains accessible while gfs2_put_super()
releases sd_statfs_inode during unmount. A concurrent write can then
enter gfs2_statfs_sync() and dereference the released inode.

Serialize the sysfs callback with the superblock lifetime. Use a
non-blocking read lock so mount failure and unmount paths can remove
the sysfs files while holding s_umount for writing, and reject access
before the superblock becomes active.

Signed-off-by: Jiacheng Xu <stitch@xxxxxxxxxx>
---
 fs/gfs2/sys.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index ea2c7b9e4a77..0e2d65982c29 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -210,6 +210,7 @@ static ssize_t withdraw_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
 static ssize_t statfs_sync_store(struct gfs2_sbd *sdp, const char *buf,
                                 size_t len)
 {
+       struct super_block *sb = sdp->sd_vfs;
        int error, val;
 
        if (!capable(CAP_SYS_ADMIN))
@@ -222,8 +223,19 @@ static ssize_t statfs_sync_store(struct gfs2_sbd *sdp, const char *buf,
        if (val != 1)
                return -EINVAL;
 
-       gfs2_statfs_sync(sdp->sd_vfs, 0);
-       return len;
+       if (!down_read_trylock(&sb->s_umount))
+               return -EAGAIN;
+
+       if (!(sb->s_flags & SB_ACTIVE)) {
+               error = -EAGAIN;
+               goto out_unlock;
+       }
+
+       gfs2_statfs_sync(sb, 0);
+
+out_unlock:
+       up_read(&sb->s_umount);
+       return error ? error : len;
 }
 
 static ssize_t quota_sync_store(struct gfs2_sbd *sdp, const char *buf,
-- 
2.25.1


> -----原始邮件-----
> 发件人: "Jiacheng Xu" <stitch@xxxxxxxxxx>
> 发送时间:2026-08-21 12:09:21 (星期五)
> 收件人: "Andreas Gruenbacher" <agruenba@xxxxxxxxxx>
> 抄送: gfs2@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
> 主题: [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@xxxxxxxxxx>
> > 发送时间:2026-08-19 15:01:07 (星期三)
> > 收件人: "Andreas Gruenbacher" <agruenba@xxxxxxxxxx>
> > 抄送: gfs2@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
> > 主题: [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@xxxxxxxxxx>
> > Tested-by: Jiacheng Xu <stitch@xxxxxxxxxx>
> > ---
> > 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;
> > }