Re: [PATCH v2 0/5] gfs2: protect sysfs callbacks from superblock teardown
From: Andreas Gruenbacher
Date: Thu Aug 27 2026 - 15:16:32 EST
Hello,
On Fri, Aug 21, 2026 at 6:09 AM Jiacheng Xu <stitch@xxxxxxxxxx> 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@xxxxxxxxxx/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