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;
> }

Hello,

On Fri, Aug 21, 2026 at 6:09 AM Jiacheng Xu <stitch@zju.edu.cn> wrote:
> 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.

thanks for these patches, they look useful.  I would like to suggest
some changes; please see the below patch.  Could you please apply those
to the individual patches and repost?

> 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

With the patch I've just posted to the gfs2 mailing list [*], dumping
the status file shouldn't require any additional locking, so the last
patch in qour queue can be dropped.

[*] https://lore.kernel.org/gfs2/20260827190632.686583-1-agruenba@redhat.com/T/#u

Thanks,
Andreas

--

 fs/gfs2/quota.c | 19 +-------------
 fs/gfs2/sys.c   | 70 ++++++++++++++++++++++++++-----------------------
 2 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index b1fb60b4cd10..dbfc21693dd5 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1387,36 +1387,19 @@ 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)
-		goto out_unlock;
+		return error;
 
 	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;
 }
 
diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index 345d0527675f..ff90ec2eb798 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -63,20 +63,30 @@ static ssize_t id_show(struct gfs2_sbd *sdp, char *buf)
 			MAJOR(sdp->sd_vfs->s_dev), MINOR(sdp->sd_vfs->s_dev));
 }
 
+static bool super_trylock_shared_active(struct super_block *sb)
+{
+	if (!down_read_trylock(&sb->s_umount))
+		return false;
+	if (sb->s_flags & SB_ACTIVE)
+		return true;
+	up_read(&sb->s_umount);
+	return false;
+}
+
+static void super_unlock_active(struct super_block *sb)
+{
+	up_read(&sb->s_umount);
+}
+
 static ssize_t status_show(struct gfs2_sbd *sdp, char *buf)
 {
 	struct super_block *sb = sdp->sd_vfs;
 	unsigned long f;
 	ssize_t s;
 
-	if (!down_read_trylock(&sb->s_umount))
+	if (!super_trylock_shared_active(sb))
 		return -EAGAIN;
 
-	if (!(sb->s_flags & SB_ACTIVE)) {
-		s = -EAGAIN;
-		goto out_unlock;
-	}
-
 	f = sdp->sd_flags;
 	s = sysfs_emit(buf,
 		     "Journal Checked:          %d\n"
@@ -136,8 +146,7 @@ static ssize_t status_show(struct gfs2_sbd *sdp, char *buf)
 		     atomic_read(&sdp->sd_log_thresh1),
 		     atomic_read(&sdp->sd_log_thresh2));
 
-out_unlock:
-	up_read(&sb->s_umount);
+	super_unlock_active(sb);
 	return s;
 }
 
@@ -236,19 +245,13 @@ static ssize_t statfs_sync_store(struct gfs2_sbd *sdp, const char *buf,
 	if (val != 1)
 		return -EINVAL;
 
-	if (!down_read_trylock(&sb->s_umount))
+	if (!super_trylock_shared_active(sb))
 		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;
+	super_unlock_active(sb);
+	return len;
 }
 
 static ssize_t quota_sync_store(struct gfs2_sbd *sdp, const char *buf,
@@ -267,24 +270,19 @@ static ssize_t quota_sync_store(struct gfs2_sbd *sdp, const char *buf,
 	if (val != 1)
 		return -EINVAL;
 
-	if (!down_read_trylock(&sb->s_umount))
+	if (!super_trylock_shared_active(sb))
 		return -EAGAIN;
 
-	if (!(sb->s_flags & SB_ACTIVE)) {
-		error = -EAGAIN;
-		goto out_unlock;
-	}
-
 	gfs2_quota_sync(sb, 0);
 
-out_unlock:
-	up_read(&sb->s_umount);
-	return error ? error : len;
+	super_unlock_active(sb);
+	return len;
 }
 
 static ssize_t quota_refresh_user_store(struct gfs2_sbd *sdp, const char *buf,
 					size_t len)
 {
+	struct super_block *sb = sdp->sd_vfs;
 	struct kqid qid;
 	int error;
 	u32 id;
@@ -300,13 +298,19 @@ static ssize_t quota_refresh_user_store(struct gfs2_sbd *sdp, const char *buf,
 	if (!qid_valid(qid))
 		return -EINVAL;
 
+	if (!super_trylock_shared_active(sb))
+		return -EAGAIN;
+
 	error = gfs2_quota_refresh(sdp, qid);
+
+	super_unlock_active(sb);
 	return error ? error : len;
 }
 
 static ssize_t quota_refresh_group_store(struct gfs2_sbd *sdp, const char *buf,
 					 size_t len)
 {
+	struct super_block *sb = sdp->sd_vfs;
 	struct kqid qid;
 	int error;
 	u32 id;
@@ -322,7 +326,12 @@ static ssize_t quota_refresh_group_store(struct gfs2_sbd *sdp, const char *buf,
 	if (!qid_valid(qid))
 		return -EINVAL;
 
+	if (!super_trylock_shared_active(sb))
+		return -EAGAIN;
+
 	error = gfs2_quota_refresh(sdp, qid);
+
+	super_unlock_active(sb);
 	return error ? error : len;
 }
 
@@ -363,14 +372,9 @@ static ssize_t demote_rq_store(struct gfs2_sbd *sdp, const char *buf, size_t len
 	if (glops == NULL)
 		return -EINVAL;
 
-	if (!down_read_trylock(&sb->s_umount))
+	if (!super_trylock_shared_active(sb))
 		return -EAGAIN;
 
-	if (!(sb->s_flags & SB_ACTIVE)) {
-		rv = -EAGAIN;
-		goto out_unlock;
-	}
-
 	if (!test_and_set_bit(SDF_DEMOTE, &sdp->sd_flags))
 		fs_info(sdp, "demote interface used\n");
 	rv = gfs2_glock_get(sdp, glnum, glops, NO_CREATE, &gl);
@@ -381,7 +385,7 @@ static ssize_t demote_rq_store(struct gfs2_sbd *sdp, const char *buf, size_t len
 	rv = 0;
 
 out_unlock:
-	up_read(&sb->s_umount);
+	super_unlock_active(sb);
 	return rv ? rv : len;
 }
 
-- 
2.55.0