Unionfs: cleanup/consolidate branch-mode parsing code

Also a bug fix: disallow unrecognized branch modes at mount time, instead of
defaulting to "rw".

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index ffb0da1c4df24cd29228a61ce515c7fdb7baded5..22aa6e6b0053edaf98a1434f887481680b793813 100644 (file)
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -256,30 +256,21 @@ static int is_branch_overlap(struct dentry *dent1, struct dentry *dent2)
 }
 
 /*
- * Parse branch mode helper function
+ * Parse "ro" or "rw" options, but default to "rw" if no mode options was
+ * specified.  Fill the mode bits in @perms.  If encounter an unknown
+ * string, return -EINVAL.  Otherwise return 0.
  */
-int __parse_branch_mode(const char *name)
+int parse_branch_mode(const char *name, int *perms)
 {
-       if (!name)
+       if (!name || !strcmp(name, "rw")) {
+               *perms = MAY_READ | MAY_WRITE;
                return 0;
-       if (!strcmp(name, "ro"))
-               return MAY_READ;
-       if (!strcmp(name, "rw"))
-               return (MAY_READ | MAY_WRITE);
-       return 0;
-}
-
-/*
- * Parse "ro" or "rw" options, but default to "rw" of no mode options
- * was specified.
- */
-int parse_branch_mode(const char *name)
-{
-       int perms = __parse_branch_mode(name);
-
-       if (perms == 0)
-               perms = MAY_READ | MAY_WRITE;
-       return perms;
+       }
+       if (!strcmp(name, "ro")) {
+               *perms = MAY_READ;
+               return 0;
+       }
+       return -EINVAL;
 }
 
 /*
@@ -350,8 +341,17 @@ static int parse_dirs_option(struct super_block *sb, struct unionfs_dentry_info
                if (mode)
                        *mode++ = '\0';
 
-               perms = parse_branch_mode(mode);
+               err = parse_branch_mode(mode, &perms);
+               if (err) {
+                       printk(KERN_ERR "unionfs: invalid mode \"%s\" for "
+                              "branch %d\n", mode, bindex);
+                       goto out;
+               }
+               /* ensure that leftmost branch is writeable */
                if (!bindex && !(perms & MAY_WRITE)) {
+                       printk(KERN_ERR "unionfs: leftmost branch cannot be "
+                              "read-only (use \"-o ro\" to create a "
+                              "read-only union)\n");
                        err = -EINVAL;
                        goto out;
                }

diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 88f77d753068e21320a0510f6beb0c58df79812b..d9cf2a74377b5127853b713c0b7b2e8e30d7c9ad 100644 (file)
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -181,8 +181,8 @@ static noinline int do_remount_mode_option(char *optarg, int cur_branches,
                goto out;
        }
        *modename++ = '\0';
-       perms = __parse_branch_mode(modename);
-       if (perms == 0) {
+       err = parse_branch_mode(modename, &perms);
+       if (err) {
                printk(KERN_ERR "unionfs: invalid mode \"%s\" for \"%s\"\n",
                       modename, optarg);
                goto out;
@@ -350,13 +350,17 @@ found_insertion_point:
                modename = strchr(new_branch, '=');
        if (modename)
                *modename++ = '\0';
-       perms = parse_branch_mode(modename);
-
        if (!new_branch || !*new_branch) {
                printk(KERN_ERR "unionfs: null new branch\n");
                err = -EINVAL;
                goto out;
        }
+       err = parse_branch_mode(modename, &perms);
+       if (err) {
+               printk(KERN_ERR "unionfs: invalid mode \"%s\" for "
+                      "branch \"%s\"\n", modename, new_branch);
+               goto out;
+       }
        err = path_lookup(new_branch, LOOKUP_FOLLOW, &nd);
        if (err) {
                printk(KERN_ERR "unionfs: error accessing "

diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index d158671677fae8ffe9f64e67c04529388f4300e5..1617482ac0e256cd4bd69844bfd01d868e7121ce 100644 (file)
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -481,8 +481,7 @@ static inline int is_robranch(const struct dentry *dentry)
  */
 extern char *alloc_whname(const char *name, int len);
 extern int check_branch(struct nameidata *nd);
-extern int __parse_branch_mode(const char *name);
-extern int parse_branch_mode(const char *name);
+extern int parse_branch_mode(const char *name, int *perms);
 
 /*
  * These two functions are here because it is kind of daft to copy and paste