Btrfs: remove all subvol options before mounting top-level

Currently, setup_root_args() substitutes 's/subvol=[^,]*/subvolid=0/'.
But, this means that if the user passes both a subvol and subvolid for
some reason, we won't actually mount the top-level when we recursively
mount. For example, consider:

mkfs.btrfs -f /dev/sdb
mount /dev/sdb /mnt
btrfs subvol create /mnt/subvol1 # subvolid=257
btrfs subvol create /mnt/subvol2 # subvolid=258
umount /mnt
mount -osubvol=/subvol1,subvolid=258 /dev/sdb /mnt

In the final mount, subvol=/subvol1,subvolid=258 becomes
subvolid=0,subvolid=258, and the last option takes precedence, so we
mount subvol2 and try to look up subvol1 inside of it, which fails.

So, instead, do a thorough scan through the argument list and remove any
subvol= and subvolid= options, then append subvolid=0 to the end. This
implicitly makes subvol= take precedence over subvolid=, but we're about
to add a stricter check for that. This also makes setup_root_args() more
generic, which we'll need soon.

Reviewed-by: David Sterba <dsterba@suse.cz>
Signed-off-by: Omar Sandoval <osandov@osandov.com>
Signed-off-by: Chris Mason <clm@fb.com>

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 93f2edf..1c57c0d 100644 (file)
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1133,52 +1133,36 @@ static inline int is_subvolume_inode(struct inode *inode)
 }
 
 /*
- * This will strip out the subvol=%s argument for an argument string and add
- * subvolid=0 to make sure we get the actual tree root for path walking to the
- * subvol we want.
+ * This will add subvolid=0 to the argument string while removing any subvol=
+ * and subvolid= arguments to make sure we get the top-level root for path
+ * walking to the subvol we want.
  */
 static char *setup_root_args(char *args)
 {
-       unsigned len = strlen(args) + 2 + 1;
-       char *src, *dst, *buf;
+       char *buf, *dst, *sep;
 
-       /*
-        * We need the same args as before, but with this substitution:
-        * s!subvol=[^,]+!subvolid=0!
-        *
-        * Since the replacement string is up to 2 bytes longer than the
-        * original, allocate strlen(args) + 2 + 1 bytes.
-        */
-
-       src = strstr(args, "subvol=");
-       /* This shouldn't happen, but just in case.. */
-       if (!src)
-               return NULL;
+       if (!args)
+               return kstrdup("subvolid=0", GFP_NOFS);
 
-       buf = dst = kmalloc(len, GFP_NOFS);
+       /* The worst case is that we add ",subvolid=0" to the end. */
+       buf = dst = kmalloc(strlen(args) + strlen(",subvolid=0") + 1, GFP_NOFS);
        if (!buf)
                return NULL;
 
-       /*
-        * If the subvol= arg is not at the start of the string,
-        * copy whatever precedes it into buf.
-        */
-       if (src != args) {
-               *src++ = '\0';
-               strcpy(buf, args);
-               dst += strlen(args);
+       while (1) {
+               sep = strchrnul(args, ',');
+               if (!strstarts(args, "subvol=") &&
+                   !strstarts(args, "subvolid=")) {
+                       memcpy(dst, args, sep - args);
+                       dst += sep - args;
+                       *dst++ = ',';
+               }
+               if (*sep)
+                       args = sep + 1;
+               else
+                       break;
        }
-
        strcpy(dst, "subvolid=0");
-       dst += strlen("subvolid=0");
-
-       /*
-        * If there is a "," after the original subvol=... string,
-        * copy that suffix into our buffer.  Otherwise, we're done.
-        */
-       src = strchr(src, ',');
-       if (src)
-               strcpy(dst, src);
 
        return buf;
 }