ovl: print error message for invalid mount options

Overlayfs should print an error message if an incorrect mount option is caught like other filesystems.

After this patch, improper option input could be clearly known.

Reported-by Fabian Sturm <[email protected]>

Signed-off-by hujianyang <[email protected]>

---
 fs/overlayfs/super.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

fs/overlayfs/super.c +0 -0

index b90952f..ab3c8cb 100644

fs/overlayfs/super.c +0 -0

 			return -EINVAL;
 		}
 	}

After importing multi-lower layer support, users could mount a r/o partition as the left most lowerdir instead of using it as upperdir. And a r/o upperdir may cause an error like

overlayfs: failed to create directory ./workdir/work

during mount.

This patch check the *s_flags* of upper fs and return an error if it is a r/o partition. The checking of *upper_mnt->mnt_sb->s_flags* can be removed now.

This patch also remove

	/* FIXME: workdir is not needed for a R/O mount */

from ovl_fill_super() because:

1) for upper fs r/o case
Setting a r/o partition as upper is prevented, no need to care about workdir in this case.

2) for "mount overlay -o ro" with a r/w upper fs case Users could remount overlayfs to r/w in this case, so workdir should not be omitted.

Signed-off-by hujianyang <[email protected]>

---
fs/overlayfs/super.c | 24 +++++++++++++++++++-----

1 files changed, 19 insertions(+), 5 deletions(-)

fs/overlayfs/super.c +1 -2

index edbb3eb..0e7a477 100644
@@ -529,8 +529,7 @@ static int ovl_remount(struct super_block *sb, int
 *flags, char *data)
 {
 	struct ovl_fs *ufs = sb->s_fs_info;
 
-	if (!(*flags & MS_RDONLY) &&
-	    (!ufs->upper_mnt || (ufs->upper_mnt->mnt_sb->s_flags &
 MS_RDONLY)))
+	if (!(*flags & MS_RDONLY) && (!ufs->upper_mnt))
 		return -EROFS;

	return 0;

fs/overlayfs/super.c +9 -0

@@ -619,6 +618,15 @@ static int ovl_parse_opt(char *opt, struct
 ovl_config *config)
 			return -EINVAL;
 		}
 	}
+
+	/* Workdir is useless in non-upper mount */
+	if (!config->upperdir && config->workdir) {
+		pr_info("overlayfs: option \"workdir=%s\" is useless in a non-upper
 mount, ignore\n",
+			config->workdir);
+		kfree(config->workdir);
+		config->workdir = NULL;
+	}
+
 	return 0;

}

fs/overlayfs/super.c +0 -1

@@ -838,7 +846,6 @@ static int ovl_fill_super(struct super_block *sb,
 void *data, int silent)
 
 	sb->s_stack_depth = 0;
 	if (ufs->config.upperdir) {
-		/* FIXME: workdir is not needed for a R/O mount */
 		if (!ufs->config.workdir) {
 			pr_err("overlayfs: missing 'workdir'\n");

			goto out_free_config;

fs/overlayfs/super.c +7 -0

@@ -848,6 +855,13 @@ static int ovl_fill_super(struct super_block *sb,
 void *data, int silent)
 		if (err)
 			goto out_free_config;
 
+		/* Upper fs should not be r/o */
+		if (upperpath.mnt->mnt_sb->s_flags & MS_RDONLY) {
+			pr_err("overlayfs: upper fs is r/o, try multi-lower layers
 mount\n");
+			err = -EINVAL;
+			goto out_put_upperpath;
+		}
+
 		err = ovl_mount_dir(ufs->config.workdir, &workpath);

		if (err)
			goto out_put_upperpath;

fs/overlayfs/super.c +2 -2

@@ -939,8 +953,8 @@ static int ovl_fill_super(struct super_block *sb,
 void *data, int silent)
 		ufs->numlower++;
 	}
 
-	/* If the upper fs is r/o or nonexistent, we mark overlayfs r/o too
 */
-	if (!ufs->upper_mnt || (ufs->upper_mnt->mnt_sb->s_flags & MS_RDONLY))
+	/* If the upper fs is nonexistent, we mark overlayfs r/o too */
+	if (!ufs->upper_mnt)
 		sb->s_flags |= MS_RDONLY;

	sb->s_d_op = &ovl_dentry_operations;

Quoted 102 lines · hujianyang

After importing multi-lower layer support, users could mount a r/o partition as the left most lowerdir instead of using it as upperdir. And a r/o upperdir may cause an error like

overlayfs: failed to create directory ./workdir/work

during mount.

This patch check the *s_flags* of upper fs and return an error if it is a r/o partition. The checking of *upper_mnt->mnt_sb->s_flags* can be removed now.

This patch also remove

	/* FIXME: workdir is not needed for a R/O mount */

from ovl_fill_super() because:

1) for upper fs r/o case
Setting a r/o partition as upper is prevented, no need to care about workdir in this case.

2) for "mount overlay -o ro" with a r/w upper fs case Users could remount overlayfs to r/w in this case, so workdir should not be omitted.

Signed-off-by hujianyang <[email protected]>

---
fs/overlayfs/super.c | 24 +++++++++++++++++++-----

1 files changed, 19 insertions(+), 5 deletions(-)

fs/overlayfs/super.c +1 -2

index edbb3eb..0e7a477 100644
@@ -529,8 +529,7 @@ static int ovl_remount(struct super_block *sb, int
 *flags, char *data)
 {
 	struct ovl_fs *ufs = sb->s_fs_info;
 
-	if (!(*flags & MS_RDONLY) &&
-	    (!ufs->upper_mnt || (ufs->upper_mnt->mnt_sb->s_flags &
 MS_RDONLY)))
+	if (!(*flags & MS_RDONLY) && (!ufs->upper_mnt))
 		return -EROFS;

	return 0;

fs/overlayfs/super.c +9 -0

@@ -619,6 +618,15 @@ static int ovl_parse_opt(char *opt, struct
 ovl_config *config)
 			return -EINVAL;
 		}
 	}
+
+	/* Workdir is useless in non-upper mount */
+	if (!config->upperdir && config->workdir) {
+		pr_info("overlayfs: option \"workdir=%s\" is useless in a non-upper
 mount, ignore\n",
+			config->workdir);
+		kfree(config->workdir);
+		config->workdir = NULL;
+	}
+
 	return 0;

}

fs/overlayfs/super.c +0 -1

@@ -838,7 +846,6 @@ static int ovl_fill_super(struct super_block *sb,
 void *data, int silent)
 
 	sb->s_stack_depth = 0;
 	if (ufs->config.upperdir) {
-		/* FIXME: workdir is not needed for a R/O mount */
 		if (!ufs->config.workdir) {
 			pr_err("overlayfs: missing 'workdir'\n");

			goto out_free_config;

fs/overlayfs/super.c +7 -0

@@ -848,6 +855,13 @@ static int ovl_fill_super(struct super_block *sb,
 void *data, int silent)
 		if (err)
 			goto out_free_config;
 
+		/* Upper fs should not be r/o */
+		if (upperpath.mnt->mnt_sb->s_flags & MS_RDONLY) {
+			pr_err("overlayfs: upper fs is r/o, try multi-lower layers
 mount\n");
+			err = -EINVAL;
+			goto out_put_upperpath;
+		}
+
 		err = ovl_mount_dir(ufs->config.workdir, &workpath);

		if (err)
			goto out_put_upperpath;

fs/overlayfs/super.c +2 -2

@@ -939,8 +953,8 @@ static int ovl_fill_super(struct super_block *sb,
 void *data, int silent)
 		ufs->numlower++;
 	}
 
-	/* If the upper fs is r/o or nonexistent, we mark overlayfs r/o too
 */
-	if (!ufs->upper_mnt || (ufs->upper_mnt->mnt_sb->s_flags & MS_RDONLY))
+	/* If the upper fs is nonexistent, we mark overlayfs r/o too */
+	if (!ufs->upper_mnt)
 		sb->s_flags |= MS_RDONLY;

Quoted 1 line

	sb->s_d_op = &ovl_dentry_operations;

It works fine to me.
And I think it is better than my implementation : )

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Just one question. If each r/o layer does not require a workdir, why would a stack of r/o layers require one - and hence the requirement that the top layer must be r/w?

Does it has to do with why workdir was introduced in the first place?

Sorry but I couldn't find information about why workdir was introduced. I suppose it was to support some functions that older versions can't.

Alex

Quoted 9 lines · Seunghun Lee

Quoted 7 lines · hujianyang

After importing multi-lower layer support, users could mount a r/o partition as the left most lowerdir instead of using it as upperdir. And a r/o upperdir may cause an error like

overlayfs: failed to create directory ./workdir/work

during mount.