linux-kernel - [PATCH] 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:   Tue, 07 Feb 2017 17:35:33 +0530
From:   Vivek Trivedi <t.vivek@...sung.com>
To:     viro@...iv.linux.org.uk, 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] 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 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 (as per filesystem)
        .                                                       up_write(&sb->s_umount); => LOCK RELEASE by mount process
        .
        . STUCKED 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.
'sb' is valid and referencable as long as part of the s_list and MS_ACTIVE is
set after successful mount and cleared during the umount path from
generic_shutdown_super.

Signed-off-by: Vivek Trivedi <t.vivek@...sung.com>
Reviewed-by: Amit Sahrawat <a.sahrawat@...sung.com>
---
 fs/super.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index b8b6a08..ef3f57c 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_root && !(sb->s_flags & MS_ACTIVE))
+			continue;
+
 		sb->s_count++;
 		spin_unlock(&sb_lock);

-- 
1.9.1