Forwarded: [PATCH] gfs2: fix duplicate kmem_cache name on concurrent mounts

syzbot posted 1 patch 1 week, 2 days ago
fs/gfs2/ops_fstype.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
Forwarded: [PATCH] gfs2: fix duplicate kmem_cache name on concurrent mounts
Posted by syzbot 1 week, 2 days ago
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] gfs2: fix duplicate kmem_cache name on concurrent mounts
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master

When gfs2_fill_super() creates the per-filesystem bufdata cache, it
uses sd_fsname as part of the cache name:

  sdp->sd_bufdata = kmem_cache_create("gfs2-bufdata/<fsname>", ...);

At this point sd_fsname is set from sd_table_name (e.g. "syz:syz"),
which is the same for all concurrent mounts of the same device.
The unique suffix (.s for spectator, .N for journal ID) is only
added to sd_fsname AFTER wait_on_journal() completes, since the
journal ID is assigned by the locking subsystem during mount.

With multiple concurrent mounts of the same device (as triggered by
syzkaller with procs:5), multiple calls to gfs2_fill_super() all try
to create a cache named "gfs2-bufdata/syz:syz" before any of them
reaches the point where sd_fsname becomes unique, triggering:

  kmem_cache of name 'gfs2-bufdata/syz:syz' already exists
  WARNING: mm/slab_common.c:112

Fix this by moving the bufdata cache creation to after
wait_on_journal() completes and sd_fsname has been updated with its
unique suffix. Also move the fail_bufdata cleanup label accordingly
in the error unwind path.

Reported-by: syzbot+b441db1854c360b83221@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b441db1854c360b83221
Fixes: 767e4de3ffce ("gfs2: per-filesystem bufdata cache")
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/gfs2/ops_fstype.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index b44adb40635d..7f8e04c11494 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1198,17 +1198,9 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (!sdp->sd_delete_wq)
 		goto fail_glock_wq;
 
-	char *bufdata_name = kasprintf(GFP_KERNEL, "gfs2-bufdata/%s", sdp->sd_fsname);
-	sdp->sd_bufdata = kmem_cache_create(bufdata_name,
-					    sizeof(struct gfs2_bufdata),
-					    0, 0, NULL);
-	kfree(bufdata_name);
-	if (!sdp->sd_bufdata)
-		goto fail_delete_wq;
-
 	error = gfs2_sys_fs_add(sdp);
 	if (error)
-		goto fail_bufdata;
+		goto fail_delete_wq;
 
 	gfs2_create_debugfs_file(sdp);
 
@@ -1255,6 +1247,14 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 		snprintf(sdp->sd_fsname, sizeof(sdp->sd_fsname), "%s.%u",
 			 sdp->sd_table_name, sdp->sd_lockstruct.ls_jid);
 
+	char *bufdata_name = kasprintf(GFP_KERNEL, "gfs2-bufdata/%s", sdp->sd_fsname);
+	sdp->sd_bufdata = kmem_cache_create(bufdata_name,
+			                    sizeof(struct gfs2_bufdata),
+                                             0, 0, NULL);
+	kfree(bufdata_name);
+	if (!sdp->sd_bufdata)
+		goto fail_sb;
+
 	error = init_inodes(sdp, DO);
 	if (error)
 		goto fail_sb;
@@ -1304,6 +1304,7 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (sb->s_root)
 		dput(sb->s_root);
 	sb->s_root = NULL;
+	kmem_cache_destroy(sdp->sd_bufdata);
 fail_locking:
 	init_locking(sdp, &mount_gh, UNDO);
 fail_lm:
@@ -1313,8 +1314,6 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 fail_debug:
 	gfs2_delete_debugfs_file(sdp);
 	gfs2_sys_fs_del(sdp);
-fail_bufdata:
-	kmem_cache_destroy(sdp->sd_bufdata);
 fail_delete_wq:
 	destroy_workqueue(sdp->sd_delete_wq);
 fail_glock_wq:
-- 
2.43.0