[PATCH] fs/namespace: handle mount(MS_BIND|MS_REMOUNT) without locking sb->s_umount

From: Konstantin Khlebnikov
Date: Wed Mar 18 2020 - 10:18:04 EST


Writeback grabs sb->s_umount for read during I/O. This blocks bind-remount
for a long time. Bind-remount actually does not need sb->s_umount locked
for read or write because it does not alter superblock, only mnt_flags.
All mnt_flags are serialized by global mount_lock.

This patch moves locking into callers to handle remount atomically.
Also grab namespace_sem to synchronize with /proc/mounts and mountinfo.
Function do_change_type() uses the same locking combination.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx>
---
fs/namespace.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 85b5f7bea82e..c6c03be5cc4e 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -459,11 +459,11 @@ void mnt_drop_write_file(struct file *file)
}
EXPORT_SYMBOL(mnt_drop_write_file);

+/* mount_lock must be held */
static int mnt_make_readonly(struct mount *mnt)
{
int ret = 0;

- lock_mount_hash();
mnt->mnt.mnt_flags |= MNT_WRITE_HOLD;
/*
* After storing MNT_WRITE_HOLD, we'll read the counters. This store
@@ -497,15 +497,14 @@ static int mnt_make_readonly(struct mount *mnt)
*/
smp_wmb();
mnt->mnt.mnt_flags &= ~MNT_WRITE_HOLD;
- unlock_mount_hash();
+
return ret;
}

+/* mount_lock must be held */
static int __mnt_unmake_readonly(struct mount *mnt)
{
- lock_mount_hash();
mnt->mnt.mnt_flags &= ~MNT_READONLY;
- unlock_mount_hash();
return 0;
}

@@ -2440,6 +2439,7 @@ static bool can_change_locked_flags(struct mount *mnt, unsigned int mnt_flags)
return true;
}

+/* mount_lock must be held */
static int change_mount_ro_state(struct mount *mnt, unsigned int mnt_flags)
{
bool readonly_request = (mnt_flags & MNT_READONLY);
@@ -2454,16 +2454,14 @@ static int change_mount_ro_state(struct mount *mnt, unsigned int mnt_flags)
}

/*
- * Update the user-settable attributes on a mount. The caller must hold
- * sb->s_umount for writing.
+ * Update the user-settable attributes on a mount.
+ * mount_lock must be held.
*/
static void set_mount_attributes(struct mount *mnt, unsigned int mnt_flags)
{
- lock_mount_hash();
mnt_flags |= mnt->mnt.mnt_flags & ~MNT_USER_SETTABLE_MASK;
mnt->mnt.mnt_flags = mnt_flags;
touch_mnt_namespace(mnt->mnt_ns);
- unlock_mount_hash();
}

static void mnt_warn_timestamp_expiry(struct path *mountpoint, struct vfsmount *mnt)
@@ -2495,7 +2493,6 @@ static void mnt_warn_timestamp_expiry(struct path *mountpoint, struct vfsmount *
*/
static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags)
{
- struct super_block *sb = path->mnt->mnt_sb;
struct mount *mnt = real_mount(path->mnt);
int ret;

@@ -2508,11 +2505,13 @@ static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags)
if (!can_change_locked_flags(mnt, mnt_flags))
return -EPERM;

- down_write(&sb->s_umount);
+ namespace_lock();
+ lock_mount_hash();
ret = change_mount_ro_state(mnt, mnt_flags);
if (ret == 0)
set_mount_attributes(mnt, mnt_flags);
- up_write(&sb->s_umount);
+ unlock_mount_hash();
+ namespace_unlock();

mnt_warn_timestamp_expiry(path, &mnt->mnt);

@@ -2551,8 +2550,11 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags,
err = -EPERM;
if (ns_capable(sb->s_user_ns, CAP_SYS_ADMIN)) {
err = reconfigure_super(fc);
- if (!err)
+ if (!err) {
+ lock_mount_hash();
set_mount_attributes(mnt, mnt_flags);
+ unlock_mount_hash();
+ }
}
up_write(&sb->s_umount);
}