vfs: protect remounting superblock read-only

Currently remouting superblock read-only is racy in a major way.

With the per mount read-only infrastructure it is now possible to
prevent most races, which this patch attempts.

Before starting the remount read-only, iterate through all mounts
belonging to the superblock and if none of them have any pending
writes, set sb->s_readonly_remount.  This indicates that remount is in
progress and no further write requests are allowed.  If the remount
succeeds set MS_RDONLY and reset s_readonly_remount.

If the remounting is unsuccessful just reset s_readonly_remount.
This can result in transient EROFS errors, despite the fact the
remount failed.  Unfortunately hodling off writes is difficult as
remount itself may touch the filesystem (e.g. through load_nls())
which would deadlock.

A later patch deals with delayed writes due to nlink going to zero.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Tested-by: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

4 files changed, 61 insertions, 5 deletions

diff --git a/fs/internal.h b/fs/internal.h
index 2523a4029452..9962c59ba280 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -52,6 +52,7 @@ extern int finish_automount(struct vfsmount *, struct path *);
 
 extern void mnt_make_longterm(struct vfsmount *);
 extern void mnt_make_shortterm(struct vfsmount *);
+extern int sb_prepare_remount_readonly(struct super_block *);
 
 extern void __init mnt_init(void);
 
diff --git a/fs/namespace.c b/fs/namespace.c
index 145217b088d1..98ebc78b21ab 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -273,6 +273,15 @@ static unsigned int mnt_get_writers(struct mount *mnt)
 #endif
 }
 
+static int mnt_is_readonly(struct vfsmount *mnt)
+{
+        if (mnt->mnt_sb->s_readonly_remount)
+                return 1;
+        /* Order wrt setting s_flags/s_readonly_remount in do_remount() */
+        smp_rmb();
+        return __mnt_is_readonly(mnt);
+}
+
 /*
  * Most r/o checks on a fs are for operations that take
  * discrete amounts of time, like a write() or unlink().
@@ -312,7 +321,7 @@ int mnt_want_write(struct vfsmount *m)
          * MNT_WRITE_HOLD is cleared.
          */
         smp_rmb();
-        if (__mnt_is_readonly(m)) {
+        if (mnt_is_readonly(m)) {
                 mnt_dec_writers(mnt);
                 ret = -EROFS;
                 goto out;
@@ -435,6 +444,35 @@ static void __mnt_unmake_readonly(struct mount *mnt)
         br_write_unlock(vfsmount_lock);
 }
 
+int sb_prepare_remount_readonly(struct super_block *sb)
+{
+        struct mount *mnt;
+        int err = 0;
+
+        br_write_lock(vfsmount_lock);
+        list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) {
+                if (!(mnt->mnt.mnt_flags & MNT_READONLY)) {
+                        mnt->mnt.mnt_flags |= MNT_WRITE_HOLD;
+                        smp_mb();
+                        if (mnt_get_writers(mnt) > 0) {
+                                err = -EBUSY;
+                                break;
+                        }
+                }
+        }
+        if (!err) {
+                sb->s_readonly_remount = 1;
+                smp_wmb();
+        }
+        list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) {
+                if (mnt->mnt.mnt_flags & MNT_WRITE_HOLD)
+                        mnt->mnt.mnt_flags &= ~MNT_WRITE_HOLD;
+        }
+        br_write_unlock(vfsmount_lock);
+
+        return err;
+}
+
 static void free_vfsmnt(struct mount *mnt)
 {
         kfree(mnt->mnt_devname);
diff --git a/fs/super.c b/fs/super.c
index 993ca8f128d6..6acc02237e3e 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -723,23 +723,33 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
         /* If we are remounting RDONLY and current sb is read/write,
            make sure there are no rw files opened */
         if (remount_ro) {
-                if (force)
+                if (force) {
                         mark_files_ro(sb);
-                else if (!fs_may_remount_ro(sb))
-                        return -EBUSY;
+                } else {
+                        retval = sb_prepare_remount_readonly(sb);
+                        if (retval)
+                                return retval;
+
+                        retval = -EBUSY;
+                        if (!fs_may_remount_ro(sb))
+                                goto cancel_readonly;
+                }
         }
 
         if (sb->s_op->remount_fs) {
                 retval = sb->s_op->remount_fs(sb, &flags, data);
                 if (retval) {
                         if (!force)
-                                return retval;
+                                goto cancel_readonly;
                         /* If forced remount, go ahead despite any errors */
                         WARN(1, "forced remount of a %s fs returned %i\n",
                              sb->s_type->name, retval);
                 }
         }
         sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (flags & MS_RMT_MASK);
+        /* Needs to be ordered wrt mnt_is_readonly() */
+        smp_wmb();
+        sb->s_readonly_remount = 0;
 
         /*
          * Some filesystems modify their metadata via some other path than the
@@ -752,6 +762,10 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
         if (remount_ro && sb->s_bdev)
                 invalidate_bdev(sb->s_bdev);
         return 0;
+
+cancel_readonly:
+        sb->s_readonly_remount = 0;
+        return retval;
 }
 
 static void do_emergency_remount(struct work_struct *work)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 03385acd71e8..7b8a681b1ef4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1482,6 +1482,9 @@ struct super_block {
         int cleancache_poolid;
 
         struct shrinker s_shrink;       /* per-sb shrinker handle */
+
+        /* Being remounted read-only */
+        int s_readonly_remount;
 };
 
 /* superblock cache pruning functions */