[PATCH] gfs2: Fix NULL pointer dereference in quota refresh
"Jiacheng Xu" <[email protected]>
| Newsgroups | dev.linux.lists.gfs2,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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; }
repro.c
(text/plain, 742.3 KB) - not displayed