[PATCH v2 4/5] gfs2: protect demote requests during superblock
"Jiacheng Xu" <[email protected]>
| Newsgroups | dev.linux.lists.gfs2,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
The demote_rq sysfs callback can obtain a glock while unmount is tearing down the glock hash and destroying sd_glock_wq. The callback may then queue work through the destroyed workqueue. Serialize the complete demote operation with s_umount and reject requests while the superblock is not active. trylock semantics avoid deadlocking with mount failure and unmount paths that remove sysfs. Signed-off-by: Jiacheng Xu <[email protected]> --- fs/gfs2/sys.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c index 94b23a6d7efe..99596b1ac08b 100644 --- a/fs/gfs2/sys.c +++ b/fs/gfs2/sys.c @@ -315,6 +315,7 @@ static ssize_t quota_refresh_group_store(struct gfs2_sbd *sdp, const char *buf, static ssize_t demote_rq_store(struct gfs2_sbd *sdp, const char *buf, size_t len) { + struct super_block *sb = sdp->sd_vfs; struct gfs2_glock *gl; const struct gfs2_glock_operations *glops; unsigned int glmode; @@ -348,14 +349,27 @@ static ssize_t demote_rq_store(struct gfs2_sbd *sdp, const char *buf, size_t len glops = gfs2_glops_list[gltype]; if (glops == NULL) return -EINVAL; + + if (!down_read_trylock(&sb->s_umount)) + 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); if (rv) - return rv; + goto out_unlock; gfs2_glock_cb(gl, glmode); gfs2_glock_put(gl); - return len; + rv = 0; + +out_unlock: + up_read(&sb->s_umount); + return rv ? rv : len; } -- 2.25.1 > -----原始邮件----- > 发件人: "Jiacheng Xu" <[email protected]> > 发送时间:2026-08-21 12:09:21 (星期五) > 收件人: "Andreas Gruenbacher" <[email protected]> > 抄送: [email protected], [email protected] > 主题: [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" <[email protected]> > > 发送时间:2026-08-19 15:01:07 (星期三) > > 收件人: "Andreas Gruenbacher" <[email protected]> > > 抄送: [email protected], [email protected] > > 主题: [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 <[email protected]> > > Tested-by: Jiacheng Xu <[email protected]> > > --- > > 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; > > }