[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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.