btrfs: fix mount failure due to remount races

[BUG]
The following reproducer can cause btrfs mount to fail:

  dev="/dev/test/scratch1"
  mnt1="/mnt/test"
  mnt2="/mnt/scratch"

  mkfs.btrfs -f $dev
  mount $dev $mnt1
  btrfs subvolume create $mnt1/subvol1
  btrfs subvolume create $mnt1/subvol2
  umount $mnt1

  mount $dev $mnt1 -o subvol=subvol1
  while mount -o remount,ro $mnt1; do mount -o remount,rw $mnt1; done &
  bg=$!

  while mount $dev $mnt2 -o subvol=subvol2; do umount $mnt2; done

  kill $bg
  wait
  umount -R $mnt1
  umount -R $mnt2

The script will fail with the following error:

  mount: /mnt/scratch: /dev/mapper/test-scratch1 already mounted on /mnt/test.
        dmesg(1) may have more information after failed mount system call.
  umount: /mnt/test: target is busy.
  umount: /mnt/scratch/: not mounted

And there is no kernel error message.

[CAUSE]
During the btrfs mount, to support mounting different subvolumes with
different RO/RW flags, we need to detect that and retry if needed:

  Retry with matching RO flags if the initial mount fail with -EBUSY.

The problem is, during that retry we do not hold any super block lock
(s_umount), this means there can be a remount process changing the RO
flags of the original fs super block.

If so, we can have an EBUSY error during retry.  And this time we treat
any failure as an error, without any retry and cause the above EBUSY
mount failure.

[FIX]
The current retry behavior is racy because we do not have a super block
thus no way to hold s_umount to prevent the race with remount.

Solve the root problem by allowing fc->sb_flags to mismatch from the
sb->s_flags at btrfs_get_tree_super().

Then at the re-entry point btrfs_get_tree_subvol(), manually check the
fc->s_flags against sb->s_flags, if it's a RO->RW mismatch, then
reconfigure with s_umount lock hold.

Reported-by: Enno Gotthold <egotthold@suse.com>
Reported-by: Fabian Vogt <fvogt@suse.com>
[ Special thanks for the reproducer and early analysis pointing to btrfs. ]
Fixes: f044b318675f ("btrfs: handle the ro->rw transition for mounting different subvolumes")
Link: https://bugzilla.suse.com/show_bug.cgi?id=1231836
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 3381389ab93a7c5589ecc438577291abc2a1e62e..387e4cf8b53df6965d5716fb6f6493eb866ea2a8 100644 (file)
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1885,18 +1885,21 @@ static int btrfs_get_tree_super(struct fs_context *fc)
 
        if (sb->s_root) {
                btrfs_close_devices(fs_devices);
-               if ((fc->sb_flags ^ sb->s_flags) & SB_RDONLY)
-                       ret = -EBUSY;
+               /*
+                * At this stage we may have RO flag mismatch between
+                * fc->sb_flags and sb->s_flags.  Caller should detect such
+                * mismatch and reconfigure with sb->s_umount rwsem held if
+                * needed.
+                */
        } else {
                snprintf(sb->s_id, sizeof(sb->s_id), "%pg", bdev);
                shrinker_debugfs_rename(sb->s_shrink, "sb-btrfs:%s", sb->s_id);
                btrfs_sb(sb)->bdev_holder = &btrfs_fs_type;
                ret = btrfs_fill_super(sb, fs_devices);
-       }
-
-       if (ret) {
-               deactivate_locked_super(sb);
-               return ret;
+               if (ret) {
+                       deactivate_locked_super(sb);
+                       return ret;
+               }
        }
 
        btrfs_clear_oneshot_options(fs_info);
@@ -1982,39 +1985,18 @@ error:
  * btrfs or not, setting the whole super block RO.  To make per-subvolume mounting
  * work with different options work we need to keep backward compatibility.
  */
-static struct vfsmount *btrfs_reconfigure_for_mount(struct fs_context *fc)
+static int btrfs_reconfigure_for_mount(struct fs_context *fc, struct vfsmount *mnt)
 {
-       struct vfsmount *mnt;
-       int ret;
-       const bool ro2rw = !(fc->sb_flags & SB_RDONLY);
-
-       /*
-        * We got an EBUSY because our SB_RDONLY flag didn't match the existing
-        * super block, so invert our setting here and retry the mount so we
-        * can get our vfsmount.
-        */
-       if (ro2rw)
-               fc->sb_flags |= SB_RDONLY;
-       else
-               fc->sb_flags &= ~SB_RDONLY;
-
-       mnt = fc_mount(fc);
-       if (IS_ERR(mnt))
-               return mnt;
+       int ret = 0;
 
-       if (!ro2rw)
-               return mnt;
+       if (fc->sb_flags & SB_RDONLY)
+               return ret;
 
-       /* We need to convert to rw, call reconfigure. */
-       fc->sb_flags &= ~SB_RDONLY;
        down_write(&mnt->mnt_sb->s_umount);
-       ret = btrfs_reconfigure(fc);
+       if (!(fc->sb_flags & SB_RDONLY) && (mnt->mnt_sb->s_flags & SB_RDONLY))
+               ret = btrfs_reconfigure(fc);
        up_write(&mnt->mnt_sb->s_umount);
-       if (ret) {
-               mntput(mnt);
-               return ERR_PTR(ret);
-       }
-       return mnt;
+       return ret;
 }
 
 static int btrfs_get_tree_subvol(struct fs_context *fc)
@@ -2024,6 +2006,7 @@ static int btrfs_get_tree_subvol(struct fs_context *fc)
        struct fs_context *dup_fc;
        struct dentry *dentry;
        struct vfsmount *mnt;
+       int ret = 0;
 
        /*
         * Setup a dummy root and fs_info for test/set super.  This is because
@@ -2066,11 +2049,16 @@ static int btrfs_get_tree_subvol(struct fs_context *fc)
        fc->security = NULL;
 
        mnt = fc_mount(dup_fc);
-       if (PTR_ERR_OR_ZERO(mnt) == -EBUSY)
-               mnt = btrfs_reconfigure_for_mount(dup_fc);
-       put_fs_context(dup_fc);
-       if (IS_ERR(mnt))
+       if (IS_ERR(mnt)) {
+               put_fs_context(dup_fc);
                return PTR_ERR(mnt);
+       }
+       ret = btrfs_reconfigure_for_mount(dup_fc, mnt);
+       put_fs_context(dup_fc);
+       if (ret) {
+               mntput(mnt);
+               return ret;
+       }
 
        /*
         * This free's ->subvol_name, because if it isn't set we have to