linux-kernel - [PATCH v2] fs: avoid iterate_supers to trigger call for sync on filesystem during mount

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>] [day] [month] [year] [list]

Date:   Fri, 17 Mar 2017 16:21:52 +0530
From:   Vivek Trivedi <t.vivek@...sung.com>
To:     viro@...iv.linux.org.uk, jack@...e.cz,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     a.sahrawat@...sung.com, pankaj.m@...sung.com,
        Vivek Trivedi <t.vivek@...sung.com>
Subject: [PATCH v2] fs: avoid iterate_supers to trigger call for sync on
 filesystem during mount

It has been observed that apps may block in sys_sync for long time if
there is parallel mount request for large size storage block device in a
loaded environment.

For example, sys_sync is reported to be hunged when a large size disk
(e.g. 4TB/8TB disks) is connected. sys_mount may take time for reading
large amount of metedata - free space accounting by reading bitmap during
mount. The larger the volume, the larger the size of the bitmaps read
during mount.

During mount operation s_umount lock is taken by sys_mount. The lock is
not released till the mount is completed. The sync() process keeps on
waiting till s_umount is relased by sys_mount.

Scenario:
        Process1			Process2
        sys_sync()			sys_mount()
	iterate_supers			do_mount()
	...				vfs_kern_mount()
	...				mount_fs() (Filesystem_mount)
	...				mount_bdev()
	...				sget()
	...				alloc_super()
	...				down_write_nested(&s->s_umount,
	...					SINGLE_DEPTH_NESTING);
	...				  => LOCK HELD by MOUNT
	...				...
	...				...
	down_read(&sb->s_umount);	...
	  => WAITING FOR LOCK		...
	...				...
	...				fill_super()
	...				  => time TAKING (per filesystem)
	...				up_write(&sb->s_umount);
	...				  => LOCK RELEASE by mount process
	...
	... 
	  => STUCK TILL MOUNT is completed

Since, the superblock gets added to the list during the mount path
alloc_super, so the 'sb' is visible in the s_list. But the behaviour of
waiting to sync() a filesystem which is not active yet, seems ambigous
here.

To avoid this issue, may be we should consider about having to check only
the ACTIVE filesystem for doing operations from the superblock list.
Once mount i.e. fill_super is completed, MS_BORN is set on the superblock
flags. If MS_BORN flag is not set on superblock flags, it means mount is
not completed yet, there is no point in waiting for sys_sync in that
superblock.

Signed-off-by: Vivek Trivedi <t.vivek@...sung.com>
Reviewed-by: Amit Sahrawat <a.sahrawat@...sung.com>
Reviewed-by: Jan Kara <jack@...e.cz>
---

Changes in v2:
 - it is more consistent to use MS_BORN instead of MS_ACTIVE as it is
   already used in iterate_supers(), just under s_umount
 - apply same change in iterate_supers_type() to keep them consistent

 fs/super.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index b8b6a08..d9b96d6 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -587,6 +587,10 @@ void iterate_supers(void (*f)(struct super_block *, void *), void *arg)
 	list_for_each_entry(sb, &super_blocks, s_list) {
 		if (hlist_unhashed(&sb->s_instances))
 			continue;
+
+		if (!(sb->s_flags & MS_BORN))
+			continue;
+
 		sb->s_count++;
 		spin_unlock(&sb_lock);

@@ -621,6 +625,9 @@ void iterate_supers_type(struct file_system_type *type,

 	spin_lock(&sb_lock);
 	hlist_for_each_entry(sb, &type->fs_supers, s_instances) {
+		if (!(sb->s_flags & MS_BORN))
+			continue;
+
 		sb->s_count++;
 		spin_unlock(&sb_lock);

-- 
1.9.1