linux-kernel - [PATCH] fs: Add to super_blocks list after SB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 13 Jul 2018 19:09:03 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:     viro@...iv.linux.org.uk
Cc:     syzbot <syzbot+2349f5067b1772c1d8a5@...kaller.appspotmail.com>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        syzkaller-bugs@...glegroups.com
Subject: [PATCH] fs: Add to super_blocks list after SB_BORN is set.

More simple version. Is this assumption correct?

>From 52f10183f2b921cd946dd4c83d9b1d99bc48914e Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@...ove.SKAURA.ne.jp>
Date: Fri, 13 Jul 2018 12:15:54 +0900
Subject: [PATCH] fs: Add to super_blocks list after SB_BORN is set.

syzbot is reporting hung tasks at iterate_supers() [1]. This is because
iterate_supers() is calling down_read(&sb->s_umount) on all superblocks
found by traversing super_blocks list while sget_userns() adds "newly
created superblock to super_blocks list with ->s_umount held by
alloc_super() for write" before "mount_fs() sets SB_BORN flag after
returning from type->mount()".

If type->mount() took long time after returning from sget() for some
reason, e.g. sync() request will be blocked on !SB_BORN superblocks.
Also, since security_sb_kern_mount() which is called after SB_BORN is
set might involve GFP_KERNEL memory allocation, we can't guarantee that
up_write(&sb->s_umount) is called as soon as SB_BORN was set.

Therefore, this patch makes sure that newly created superblock is added
to super_blocks list immediately before calling up_write(&sb->s_umount).

[1] https://syzkaller.appspot.com/bug?id=3c0c173ff55822aacb81ce7ae27a6676fba29a5c

Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SKAURA.ne.jp>
---
 fs/super.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 50728d9..8b2c8cc 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -236,6 +236,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
 	s->s_flags = flags;
 	if (s->s_user_ns != &init_user_ns)
 		s->s_iflags |= SB_I_NODEV;
+	INIT_LIST_HEAD(&s->s_list);
 	INIT_HLIST_NODE(&s->s_instances);
 	INIT_HLIST_BL_HEAD(&s->s_roots);
 	mutex_init(&s->s_sync_lock);
@@ -527,8 +528,6 @@ struct super_block *sget_userns(struct file_system_type *type,
 	}
 	s->s_type = type;
 	strlcpy(s->s_id, type->name, sizeof(s->s_id));
-	list_add_tail(&s->s_list, &super_blocks);
-	hlist_add_head(&s->s_instances, &type->fs_supers);
 	spin_unlock(&sb_lock);
 	get_filesystem(type);
 	register_shrinker_prepared(&s->s_shrink);
@@ -1305,6 +1304,12 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data)
 	WARN((sb->s_maxbytes < 0), "%s set sb->s_maxbytes to "
 		"negative value (%lld)\n", type->name, sb->s_maxbytes);

+	if (list_empty(&sb->s_list)) {
+		spin_lock(&sb_lock);
+		list_add_tail(&sb->s_list, &super_blocks);
+		hlist_add_head(&sb->s_instances, &type->fs_supers);
+		spin_unlock(&sb_lock);
+	}
 	up_write(&sb->s_umount);
 	free_secdata(secdata);
 	return root;
-- 
2.7.4