[PATCH v3 0/3] ovl: port to new mount api & updated layer parsing

[PATCH v3 0/3] ovl: port to new mount api & updated layer parsing

[Christian Brauner]

Hey everyone,

/* v3 */
(1) Add a patch that makes sure that struct ovl_layer starts with a
    pointer to struct vfsmount as ovl_free_fs() relies on this during
    cleanup.
(2) Rebase on top of overlayfs-next brought in substantial rework due to
    the data layer work. I had to redo a bunch of the parsing and adapt
    it to the new changes.

This ports overlayfs to the new mount api and modifies layer parsing to
allow for additive layer specification via fsconfig().

This passes xfstests including the new data layer lookup work. Here's a
500 layer overlayfs mount including a couple of data only layers:

1094 28 0:48 / /mnt/test rw,relatime shared:214 - overlay none rw,lowerdir=/mnt/ovl-lower1:/mnt/ovl-lower2:/mnt/ovl-lower3:/mnt/ovl-lower4:/mnt/ovl-lower5:/mnt/ovl-lower6:/mnt/ovl-lower7:/mnt/ovl-lower8:/mnt/ovl-lower9:/mnt/ovl-lower10:/mnt/ovl-lower11:/mnt/ovl-lower12:/mnt/ovl-lower13:/mnt/ovl-lower14:/mnt/ovl-lower15:/mnt/ovl-lower16:/mnt/ovl-lower17:/mnt/ovl-lower18:/mnt/ovl-lower19:/mnt/ovl-lower20:/mnt/ovl-lower21:/mnt/ovl-lower22:/mnt/ovl-lower23:/mnt/ovl-lower24:/mnt/ovl-lower25:/mnt/ovl-lower26:/mnt/ovl-lower27:/mnt/ovl-lower28:/mnt/ovl-lower29:/mnt/ovl-lower30:/mnt/ovl-lower31:/mnt/ovl-lower32:/mnt/ovl-lower33:/mnt/ovl-lower34:/mnt/ovl-lower35:/mnt/ovl-lower36:/mnt/ovl-lower37:/mnt/ovl-lower38:/mnt/ovl-lower39:/mnt/ovl-lower40:/mnt/ovl-lower41:/mnt/ovl-lower42:/mnt/ovl-lower43:/mnt/ovl-lower44:/mnt/ovl-lower45:/mnt/ovl-lower46:/mnt/ovl-lower47:/mnt/ovl-lower48:/mnt/ovl-lower49:/mnt/ovl-lower50:/mnt/ovl-lower51:/mnt/ovl-lower52:/mnt/ovl-lower53:/mnt/ovl-lower54:/mnt/ovl-lower
 55:/mnt/ovl-lower56:/mnt/ovl-lower57:/mnt/ovl-lower58:/mnt/ovl-lower59:/mnt/ovl-lower60:/mnt/ovl-lower61:/mnt/ovl-lower62:/mnt/ovl-lower63:/mnt/ovl-lower64:/mnt/ovl-lower65:/mnt/ovl-lower66:/mnt/ovl-lower67:/mnt/ovl-lower68:/mnt/ovl-lower69:/mnt/ovl-lower70:/mnt/ovl-lower71:/mnt/ovl-lower72:/mnt/ovl-lower73:/mnt/ovl-lower74:/mnt/ovl-lower75:/mnt/ovl-lower76:/mnt/ovl-lower77:/mnt/ovl-lower78:/mnt/ovl-lower79:/mnt/ovl-lower80:/mnt/ovl-lower81:/mnt/ovl-lower82:/mnt/ovl-lower83:/mnt/ovl-lower84:/mnt/ovl-lower85:/mnt/ovl-lower86:/mnt/ovl-lower87:/mnt/ovl-lower88:/mnt/ovl-lower89:/mnt/ovl-lower90:/mnt/ovl-lower91:/mnt/ovl-lower92:/mnt/ovl-lower93:/mnt/ovl-lower94:/mnt/ovl-lower95:/mnt/ovl-lower96:/mnt/ovl-lower97:/mnt/ovl-lower98:/mnt/ovl-lower99:/mnt/ovl-lower100:/mnt/ovl-lower101:/mnt/ovl-lower102:/mnt/ovl-lower103:/mnt/ovl-lower104:/mnt/ovl-lower105:/mnt/ovl-lower106:/mnt/ovl-lower107:/mnt/ovl-lower108:/mnt/ovl-lower109:/mnt/ovl-lower110:/mnt/ovl-lower111:/mnt/ovl-lower112:/mnt/ovl-low
 er113:/mnt/ovl-lower114:/mnt/ovl-lower115:/mnt/ovl-lower116:/mnt/ovl-lower117:/mnt/ovl-lower118:/mnt/ovl-lower119:/mnt/ovl-lower120:/mnt/ovl-lower121:/mnt/ovl-lower122:/mnt/ovl-lower123:/mnt/ovl-lower124:/mnt/ovl-lower125:/mnt/ovl-lower126:/mnt/ovl-lower127:/mnt/ovl-lower128:/mnt/ovl-lower129:/mnt/ovl-lower130:/mnt/ovl-lower131:/mnt/ovl-lower132:/mnt/ovl-lower133:/mnt/ovl-lower134:/mnt/ovl-lower135:/mnt/ovl-lower136:/mnt/ovl-lower137:/mnt/ovl-lower138:/mnt/ovl-lower139:/mnt/ovl-lower140:/mnt/ovl-lower141:/mnt/ovl-lower142:/mnt/ovl-lower143:/mnt/ovl-lower144:/mnt/ovl-lower145:/mnt/ovl-lower146:/mnt/ovl-lower147:/mnt/ovl-lower148:/mnt/ovl-lower149:/mnt/ovl-lower150:/mnt/ovl-lower151:/mnt/ovl-lower152:/mnt/ovl-lower153:/mnt/ovl-lower154:/mnt/ovl-lower155:/mnt/ovl-lower156:/mnt/ovl-lower157:/mnt/ovl-lower158:/mnt/ovl-lower159:/mnt/ovl-lower160:/mnt/ovl-lower161:/mnt/ovl-lower162:/mnt/ovl-lower163:/mnt/ovl-lower164:/mnt/ovl-lower165:/mnt/ovl-lower166:/mnt/ovl-lower167:/mnt/ovl-lower168:/
 mnt/ovl-lower169:/mnt/ovl-lower170:/mnt/ovl-lower171:/mnt/ovl-lower172:/mnt/ovl-lower173:/mnt/ovl-lower174:/mnt/ovl-lower175:/mnt/ovl-lower176:/mnt/ovl-lower177:/mnt/ovl-lower178:/mnt/ovl-lower179:/mnt/ovl-lower180:/mnt/ovl-lower181:/mnt/ovl-lower182:/mnt/ovl-lower183:/mnt/ovl-lower184:/mnt/ovl-lower185:/mnt/ovl-lower186:/mnt/ovl-lower187:/mnt/ovl-lower188:/mnt/ovl-lower189:/mnt/ovl-lower190:/mnt/ovl-lower191:/mnt/ovl-lower192:/mnt/ovl-lower193:/mnt/ovl-lower194:/mnt/ovl-lower195:/mnt/ovl-lower196:/mnt/ovl-lower197:/mnt/ovl-lower198:/mnt/ovl-lower199:/mnt/ovl-lower200:/mnt/ovl-lower201:/mnt/ovl-lower202:/mnt/ovl-lower203:/mnt/ovl-lower204:/mnt/ovl-lower205:/mnt/ovl-lower206:/mnt/ovl-lower207:/mnt/ovl-lower208:/mnt/ovl-lower209:/mnt/ovl-lower210:/mnt/ovl-lower211:/mnt/ovl-lower212:/mnt/ovl-lower213:/mnt/ovl-lower214:/mnt/ovl-lower215:/mnt/ovl-lower216:/mnt/ovl-lower217:/mnt/ovl-lower218:/mnt/ovl-lower219:/mnt/ovl-lower220:/mnt/ovl-lower221:/mnt/ovl-lower222:/mnt/ovl-lower223:/mnt/ovl
 -lower224:/mnt/ovl-lower225:/mnt/ovl-lower226:/mnt/ovl-lower227:/mnt/ovl-lower228:/mnt/ovl-lower229:/mnt/ovl-lower230:/mnt/ovl-lower231:/mnt/ovl-lower232:/mnt/ovl-lower233:/mnt/ovl-lower234:/mnt/ovl-lower235:/mnt/ovl-lower236:/mnt/ovl-lower237:/mnt/ovl-lower238:/mnt/ovl-lower239:/mnt/ovl-lower240:/mnt/ovl-lower241:/mnt/ovl-lower242:/mnt/ovl-lower243:/mnt/ovl-lower244:/mnt/ovl-lower245:/mnt/ovl-lower246:/mnt/ovl-lower247:/mnt/ovl-lower248:/mnt/ovl-lower249:/mnt/ovl-lower250:/mnt/ovl-lower251:/mnt/ovl-lower252:/mnt/ovl-lower253:/mnt/ovl-lower254:/mnt/ovl-lower255:/mnt/ovl-lower256:/mnt/ovl-lower257:/mnt/ovl-lower258:/mnt/ovl-lower259:/mnt/ovl-lower260:/mnt/ovl-lower261:/mnt/ovl-lower262:/mnt/ovl-lower263:/mnt/ovl-lower264:/mnt/ovl-lower265:/mnt/ovl-lower266:/mnt/ovl-lower267:/mnt/ovl-lower268:/mnt/ovl-lower269:/mnt/ovl-lower270:/mnt/ovl-lower271:/mnt/ovl-lower272:/mnt/ovl-lower273:/mnt/ovl-lower274:/mnt/ovl-lower275:/mnt/ovl-lower276:/mnt/ovl-lower277:/mnt/ovl-lower278:/mnt/ovl-lower2
 79:/mnt/ovl-lower280:/mnt/ovl-lower281:/mnt/ovl-lower282:/mnt/ovl-lower283:/mnt/ovl-lower284:/mnt/ovl-lower285:/mnt/ovl-lower286:/mnt/ovl-lower287:/mnt/ovl-lower288:/mnt/ovl-lower289:/mnt/ovl-lower290:/mnt/ovl-lower291:/mnt/ovl-lower292:/mnt/ovl-lower293:/mnt/ovl-lower294:/mnt/ovl-lower295:/mnt/ovl-lower296:/mnt/ovl-lower297:/mnt/ovl-lower298:/mnt/ovl-lower299:/mnt/ovl-lower300:/mnt/ovl-lower301:/mnt/ovl-lower302:/mnt/ovl-lower303:/mnt/ovl-lower304:/mnt/ovl-lower305:/mnt/ovl-lower306:/mnt/ovl-lower307:/mnt/ovl-lower308:/mnt/ovl-lower309:/mnt/ovl-lower310:/mnt/ovl-lower311:/mnt/ovl-lower312:/mnt/ovl-lower313:/mnt/ovl-lower314:/mnt/ovl-lower315:/mnt/ovl-lower316:/mnt/ovl-lower317:/mnt/ovl-lower318:/mnt/ovl-lower319:/mnt/ovl-lower320:/mnt/ovl-lower321:/mnt/ovl-lower322:/mnt/ovl-lower323:/mnt/ovl-lower324:/mnt/ovl-lower325:/mnt/ovl-lower326:/mnt/ovl-lower327:/mnt/ovl-lower328:/mnt/ovl-lower329:/mnt/ovl-lower330:/mnt/ovl-lower331:/mnt/ovl-lower332:/mnt/ovl-lower333:/mnt/ovl-lower334:/mnt
 /ovl-lower335:/mnt/ovl-lower336:/mnt/ovl-lower337:/mnt/ovl-lower338:/mnt/ovl-lower339:/mnt/ovl-lower340:/mnt/ovl-lower341:/mnt/ovl-lower342:/mnt/ovl-lower343:/mnt/ovl-lower344:/mnt/ovl-lower345:/mnt/ovl-lower346:/mnt/ovl-lower347:/mnt/ovl-lower348:/mnt/ovl-lower349:/mnt/ovl-lower350:/mnt/ovl-lower351:/mnt/ovl-lower352:/mnt/ovl-lower353:/mnt/ovl-lower354:/mnt/ovl-lower355:/mnt/ovl-lower356:/mnt/ovl-lower357:/mnt/ovl-lower358:/mnt/ovl-lower359:/mnt/ovl-lower360:/mnt/ovl-lower361:/mnt/ovl-lower362:/mnt/ovl-lower363:/mnt/ovl-lower364:/mnt/ovl-lower365:/mnt/ovl-lower366:/mnt/ovl-lower367:/mnt/ovl-lower368:/mnt/ovl-lower369:/mnt/ovl-lower370:/mnt/ovl-lower371:/mnt/ovl-lower372:/mnt/ovl-lower373:/mnt/ovl-lower374:/mnt/ovl-lower375:/mnt/ovl-lower376:/mnt/ovl-lower377:/mnt/ovl-lower378:/mnt/ovl-lower379:/mnt/ovl-lower380:/mnt/ovl-lower381:/mnt/ovl-lower382:/mnt/ovl-lower383:/mnt/ovl-lower384:/mnt/ovl-lower385:/mnt/ovl-lower386:/mnt/ovl-lower387:/mnt/ovl-lower388:/mnt/ovl-lower389:/mnt/ovl-lo
 wer390:/mnt/ovl-lower391:/mnt/ovl-lower392:/mnt/ovl-lower393:/mnt/ovl-lower394:/mnt/ovl-lower395:/mnt/ovl-lower396:/mnt/ovl-lower397:/mnt/ovl-lower398:/mnt/ovl-lower399:/mnt/ovl-lower400:/mnt/ovl-lower401:/mnt/ovl-lower402:/mnt/ovl-lower403:/mnt/ovl-lower404:/mnt/ovl-lower405:/mnt/ovl-lower406:/mnt/ovl-lower407:/mnt/ovl-lower408:/mnt/ovl-lower409:/mnt/ovl-lower410:/mnt/ovl-lower411:/mnt/ovl-lower412:/mnt/ovl-lower413:/mnt/ovl-lower414:/mnt/ovl-lower415:/mnt/ovl-lower416:/mnt/ovl-lower417:/mnt/ovl-lower418:/mnt/ovl-lower419:/mnt/ovl-lower420:/mnt/ovl-lower421:/mnt/ovl-lower422:/mnt/ovl-lower423:/mnt/ovl-lower424:/mnt/ovl-lower425:/mnt/ovl-lower426:/mnt/ovl-lower427:/mnt/ovl-lower428:/mnt/ovl-lower429:/mnt/ovl-lower430:/mnt/ovl-lower431:/mnt/ovl-lower432:/mnt/ovl-lower433:/mnt/ovl-lower434:/mnt/ovl-lower435:/mnt/ovl-lower436:/mnt/ovl-lower437:/mnt/ovl-lower438:/mnt/ovl-lower439:/mnt/ovl-lower440:/mnt/ovl-lower441:/mnt/ovl-lower442:/mnt/ovl-lower443:/mnt/ovl-lower444:/mnt/ovl-lower445:
 /mnt/ovl-lower446:/mnt/ovl-lower447:/mnt/ovl-lower448:/mnt/ovl-lower449:/mnt/ovl-lower450:/mnt/ovl-lower451:/mnt/ovl-lower452:/mnt/ovl-lower453:/mnt/ovl-lower454:/mnt/ovl-lower455:/mnt/ovl-lower456:/mnt/ovl-lower457:/mnt/ovl-lower458:/mnt/ovl-lower459:/mnt/ovl-lower460:/mnt/ovl-lower461:/mnt/ovl-lower462:/mnt/ovl-lower463:/mnt/ovl-lower464:/mnt/ovl-lower465:/mnt/ovl-lower466:/mnt/ovl-lower467:/mnt/ovl-lower468:/mnt/ovl-lower469:/mnt/ovl-lower470:/mnt/ovl-lower471:/mnt/ovl-lower472:/mnt/ovl-lower473:/mnt/ovl-lower474:/mnt/ovl-lower475:/mnt/ovl-lower476:/mnt/ovl-lower477:/mnt/ovl-lower478:/mnt/ovl-lower479:/mnt/ovl-lower480:/mnt/ovl-lower481:/mnt/ovl-lower482:/mnt/ovl-lower483:/mnt/ovl-lower484:/mnt/ovl-lower485:/mnt/ovl-lower486:/mnt/ovl-lower487:/mnt/ovl-lower488:/mnt/ovl-lower489:/mnt/ovl-lower490:/mnt/ovl-lower491:/mnt/ovl-lower492:/mnt/ovl-lower493:/mnt/ovl-lower494:/mnt/ovl-lower495:/mnt/ovl-lower496::/mnt/ovl-lower497::/mnt/ovl-lower498::/mnt/ovl-lower499::/mnt/ovl-lower500,upp
 erdir=/mnt/ovl-upper,workdir=/mnt/ovl-work,redirect_dir=on,index=on,metacopy=on

Christian

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Changes in v3:
- Fix potential NULL deref in ovl_init_fs_context().
- Don't create one long mount option string for lowerdir. Just use
  seq_printf() to create it on demand.
- Link to v2: https://lore.kernel.org/r/20230605-fs-overlayfs-mount_api-v2-0-3da91c97e0c0@kernel.org

Changes in v2:
- Split into two patches. First patch ports to new mount api without changing
  layer parsing. Second patch implements new layer parsing.
- Move layer parsing into a separate file.
- Link to v1: https://lore.kernel.org/r/20230605-fs-overlayfs-mount_api-v1-1-a8d78c3fbeaf@kernel.org

---



---
base-commit: f777c8266c1230fa44de7261b61a13d787b27f44
change-id: 20230605-fs-overlayfs-mount_api-20ea8b04eff4

[PATCH v3 1/3] ovl: check type and offset of struct vfsmount in ovl_entry

[Christian Brauner]

Porting overlayfs to the new amount api I started experiencing random
crashes that couldn't be explained easily. So after much debugging and
reasoning it became clear that struct ovl_entry requires the point to
struct vfsmount to be the first member and of type struct vfsmount.

During the port I added a new member at the beginning of struct
ovl_entry which broke all over the place in the form of random crashes
and cache corruptions. While there's a comment in ovl_free_fs() to the
effect of "Hack! Reuse ofs->layers as a vfsmount array before freeing
it" there's no such comment on struct ovl_entry which makes this easy to
trip over.

Add a comment and two static asserts for both the offset and the type of
pointer in struct ovl_entry.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/overlayfs/ovl_entry.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index c6c7d09b494e..e5207c4bf5b8 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -32,6 +32,7 @@ struct ovl_sb {
 };
 
 struct ovl_layer {
+	/* ovl_free_fs() relies on @mnt being the first member! */
 	struct vfsmount *mnt;
 	/* Trap in ovl inode cache */
 	struct inode *trap;
@@ -42,6 +43,14 @@ struct ovl_layer {
 	int fsid;
 };
 
+/*
+ * ovl_free_fs() relies on @mnt being the first member when unmounting
+ * the private mounts created for each layer. Let's check both the
+ * offset and type.
+ */
+static_assert(offsetof(struct ovl_layer, mnt) == 0);
+static_assert(__same_type(typeof_member(struct ovl_layer, mnt), struct vfsmount *));
+
 struct ovl_path {
 	const struct ovl_layer *layer;
 	struct dentry *dentry;

-- 
2.34.1

[PATCH v3 2/3] ovl: port to new mount api

[Christian Brauner]

We recently ported util-linux to the new mount api. Now the mount(8)
tool will by default use the new mount api. While trying hard to fall
back to the old mount api gracefully there are still cases where we run
into issues that are difficult to handle nicely.

Now with mount(8) and libmount supporting the new mount api I expect an
increase in the number of bug reports and issues we're going to see with
filesystems that don't yet support the new mount api. So it's time we
rectify this.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/overlayfs/ovl_entry.h |   2 +-
 fs/overlayfs/super.c     | 557 ++++++++++++++++++++++++++---------------------
 2 files changed, 305 insertions(+), 254 deletions(-)

diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index e5207c4bf5b8..c72433c06006 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -12,7 +12,7 @@ struct ovl_config {
 	bool default_permissions;
 	bool redirect_dir;
 	bool redirect_follow;
-	const char *redirect_mode;
+	unsigned redirect_mode;
 	bool index;
 	bool uuid;
 	bool nfs_export;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index d9be5d318e1b..3392dc5d2082 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -16,6 +16,8 @@
 #include <linux/posix_acl_xattr.h>
 #include <linux/exportfs.h>
 #include <linux/file.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include "overlayfs.h"
 
 MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>");
@@ -59,6 +61,79 @@ module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
 MODULE_PARM_DESC(metacopy,
 		 "Default to on or off for the metadata only copy up feature");
 
+enum {
+	Opt_lowerdir,
+	Opt_upperdir,
+	Opt_workdir,
+	Opt_default_permissions,
+	Opt_redirect_dir,
+	Opt_index,
+	Opt_uuid,
+	Opt_nfs_export,
+	Opt_userxattr,
+	Opt_xino,
+	Opt_metacopy,
+	Opt_volatile,
+};
+
+static const struct constant_table ovl_parameter_bool[] = {
+	{ "on",		true  },
+	{ "off",	false },
+	{}
+};
+
+static const struct constant_table ovl_parameter_xino[] = {
+	{ "on",		OVL_XINO_ON   },
+	{ "off",	OVL_XINO_OFF  },
+	{ "auto",	OVL_XINO_AUTO },
+	{}
+};
+
+enum {
+	OVL_REDIRECT_DIR_ON,
+	OVL_REDIRECT_DIR_OFF,
+	OVL_REDIRECT_DIR_FOLLOW,
+	OVL_REDIRECT_DIR_NOFOLLOW,
+};
+
+static const struct constant_table ovl_parameter_redirect_dir[] = {
+	{ "on",		OVL_REDIRECT_DIR_ON       },
+	{ "off",	OVL_REDIRECT_DIR_OFF      },
+	{ "follow",	OVL_REDIRECT_DIR_FOLLOW   },
+	{ "nofollow",	OVL_REDIRECT_DIR_NOFOLLOW },
+	{}
+};
+
+static const char *ovl_redirect_mode(struct ovl_config *config)
+{
+	return ovl_parameter_redirect_dir[config->redirect_mode].name;
+}
+
+static const struct fs_parameter_spec ovl_parameter_spec[] = {
+	fsparam_string("lowerdir",          Opt_lowerdir),
+	fsparam_string("upperdir",          Opt_upperdir),
+	fsparam_string("workdir",           Opt_workdir),
+	fsparam_flag("default_permissions", Opt_default_permissions),
+	fsparam_enum("redirect_dir",        Opt_redirect_dir, ovl_parameter_redirect_dir),
+	fsparam_enum("index",               Opt_index, ovl_parameter_bool),
+	fsparam_enum("uuid",                Opt_uuid, ovl_parameter_bool),
+	fsparam_enum("nfs_export",          Opt_nfs_export, ovl_parameter_bool),
+	fsparam_flag("userxattr",           Opt_userxattr),
+	fsparam_enum("xino",                Opt_xino, ovl_parameter_xino),
+	fsparam_enum("metacopy",            Opt_metacopy, ovl_parameter_bool),
+	fsparam_flag("volatile",            Opt_volatile),
+	{}
+};
+
+#define OVL_METACOPY_SET	BIT(0)
+#define OVL_REDIRECT_SET	BIT(1)
+#define OVL_NFS_EXPORT_SET	BIT(2)
+#define OVL_INDEX_SET		BIT(3)
+
+struct ovl_fs_context {
+	u8 set;
+};
+
 static struct dentry *ovl_d_real(struct dentry *dentry,
 				 const struct inode *inode)
 {
@@ -243,7 +318,6 @@ static void ovl_free_fs(struct ovl_fs *ofs)
 	kfree(ofs->config.lowerdir);
 	kfree(ofs->config.upperdir);
 	kfree(ofs->config.workdir);
-	kfree(ofs->config.redirect_mode);
 	if (ofs->creator_cred)
 		put_cred(ofs->creator_cred);
 	kfree(ofs);
@@ -253,7 +327,8 @@ static void ovl_put_super(struct super_block *sb)
 {
 	struct ovl_fs *ofs = sb->s_fs_info;
 
-	ovl_free_fs(ofs);
+	if (ofs)
+		ovl_free_fs(ofs);
 }
 
 /* Sync real dirty inodes in upper filesystem (if it exists) */
@@ -357,6 +432,7 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 {
 	struct super_block *sb = dentry->d_sb;
 	struct ovl_fs *ofs = sb->s_fs_info;
+	const char *redirect_mode;
 
 	seq_show_option(m, "lowerdir", ofs->config.lowerdir);
 	if (ofs->config.upperdir) {
@@ -365,8 +441,9 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 	}
 	if (ofs->config.default_permissions)
 		seq_puts(m, ",default_permissions");
-	if (strcmp(ofs->config.redirect_mode, ovl_redirect_mode_def()) != 0)
-		seq_printf(m, ",redirect_dir=%s", ofs->config.redirect_mode);
+	redirect_mode = ovl_redirect_mode(&ofs->config);
+	if (strcmp(redirect_mode, ovl_redirect_mode_def()) != 0)
+		seq_printf(m, ",redirect_dir=%s", redirect_mode);
 	if (ofs->config.index != ovl_index_def)
 		seq_printf(m, ",index=%s", ofs->config.index ? "on" : "off");
 	if (!ofs->config.uuid)
@@ -386,27 +463,6 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 	return 0;
 }
 
-static int ovl_remount(struct super_block *sb, int *flags, char *data)
-{
-	struct ovl_fs *ofs = sb->s_fs_info;
-	struct super_block *upper_sb;
-	int ret = 0;
-
-	if (!(*flags & SB_RDONLY) && ovl_force_readonly(ofs))
-		return -EROFS;
-
-	if (*flags & SB_RDONLY && !sb_rdonly(sb)) {
-		upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
-		if (ovl_should_sync(ofs)) {
-			down_read(&upper_sb->s_umount);
-			ret = sync_filesystem(upper_sb);
-			up_read(&upper_sb->s_umount);
-		}
-	}
-
-	return ret;
-}
-
 static const struct super_operations ovl_super_operations = {
 	.alloc_inode	= ovl_alloc_inode,
 	.free_inode	= ovl_free_inode,
@@ -416,216 +472,42 @@ static const struct super_operations ovl_super_operations = {
 	.sync_fs	= ovl_sync_fs,
 	.statfs		= ovl_statfs,
 	.show_options	= ovl_show_options,
-	.remount_fs	= ovl_remount,
-};
-
-enum {
-	OPT_LOWERDIR,
-	OPT_UPPERDIR,
-	OPT_WORKDIR,
-	OPT_DEFAULT_PERMISSIONS,
-	OPT_REDIRECT_DIR,
-	OPT_INDEX_ON,
-	OPT_INDEX_OFF,
-	OPT_UUID_ON,
-	OPT_UUID_OFF,
-	OPT_NFS_EXPORT_ON,
-	OPT_USERXATTR,
-	OPT_NFS_EXPORT_OFF,
-	OPT_XINO_ON,
-	OPT_XINO_OFF,
-	OPT_XINO_AUTO,
-	OPT_METACOPY_ON,
-	OPT_METACOPY_OFF,
-	OPT_VOLATILE,
-	OPT_ERR,
-};
-
-static const match_table_t ovl_tokens = {
-	{OPT_LOWERDIR,			"lowerdir=%s"},
-	{OPT_UPPERDIR,			"upperdir=%s"},
-	{OPT_WORKDIR,			"workdir=%s"},
-	{OPT_DEFAULT_PERMISSIONS,	"default_permissions"},
-	{OPT_REDIRECT_DIR,		"redirect_dir=%s"},
-	{OPT_INDEX_ON,			"index=on"},
-	{OPT_INDEX_OFF,			"index=off"},
-	{OPT_USERXATTR,			"userxattr"},
-	{OPT_UUID_ON,			"uuid=on"},
-	{OPT_UUID_OFF,			"uuid=off"},
-	{OPT_NFS_EXPORT_ON,		"nfs_export=on"},
-	{OPT_NFS_EXPORT_OFF,		"nfs_export=off"},
-	{OPT_XINO_ON,			"xino=on"},
-	{OPT_XINO_OFF,			"xino=off"},
-	{OPT_XINO_AUTO,			"xino=auto"},
-	{OPT_METACOPY_ON,		"metacopy=on"},
-	{OPT_METACOPY_OFF,		"metacopy=off"},
-	{OPT_VOLATILE,			"volatile"},
-	{OPT_ERR,			NULL}
 };
 
-static char *ovl_next_opt(char **s)
+static int ovl_set_redirect_mode(struct ovl_config *config)
 {
-	char *sbegin = *s;
-	char *p;
-
-	if (sbegin == NULL)
-		return NULL;
-
-	for (p = sbegin; *p; p++) {
-		if (*p == '\\') {
-			p++;
-			if (!*p)
-				break;
-		} else if (*p == ',') {
-			*p = '\0';
-			*s = p + 1;
-			return sbegin;
-		}
-	}
-	*s = NULL;
-	return sbegin;
-}
-
-static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
-{
-	if (strcmp(mode, "on") == 0) {
+	switch (config->redirect_mode) {
+	case OVL_REDIRECT_DIR_ON:
 		config->redirect_dir = true;
 		/*
 		 * Does not make sense to have redirect creation without
 		 * redirect following.
 		 */
 		config->redirect_follow = true;
-	} else if (strcmp(mode, "follow") == 0) {
+		return 0;
+	case OVL_REDIRECT_DIR_FOLLOW:
 		config->redirect_follow = true;
-	} else if (strcmp(mode, "off") == 0) {
+		return 0;
+	case OVL_REDIRECT_DIR_OFF:
 		if (ovl_redirect_always_follow)
 			config->redirect_follow = true;
-	} else if (strcmp(mode, "nofollow") != 0) {
-		pr_err("bad mount option \"redirect_dir=%s\"\n",
-		       mode);
-		return -EINVAL;
+		return 0;
+	case OVL_REDIRECT_DIR_NOFOLLOW:
+		return 0;
 	}
 
-	return 0;
+	pr_err("invalid \"redirect_dir\" mount option\n");
+	return -EINVAL;
 }
 
-static int ovl_parse_opt(char *opt, struct ovl_config *config)
+static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
+				struct ovl_config *config)
 {
-	char *p;
 	int err;
-	bool metacopy_opt = false, redirect_opt = false;
-	bool nfs_export_opt = false, index_opt = false;
-
-	config->redirect_mode = kstrdup(ovl_redirect_mode_def(), GFP_KERNEL);
-	if (!config->redirect_mode)
-		return -ENOMEM;
-
-	while ((p = ovl_next_opt(&opt)) != NULL) {
-		int token;
-		substring_t args[MAX_OPT_ARGS];
-
-		if (!*p)
-			continue;
-
-		token = match_token(p, ovl_tokens, args);
-		switch (token) {
-		case OPT_UPPERDIR:
-			kfree(config->upperdir);
-			config->upperdir = match_strdup(&args[0]);
-			if (!config->upperdir)
-				return -ENOMEM;
-			break;
-
-		case OPT_LOWERDIR:
-			kfree(config->lowerdir);
-			config->lowerdir = match_strdup(&args[0]);
-			if (!config->lowerdir)
-				return -ENOMEM;
-			break;
-
-		case OPT_WORKDIR:
-			kfree(config->workdir);
-			config->workdir = match_strdup(&args[0]);
-			if (!config->workdir)
-				return -ENOMEM;
-			break;
-
-		case OPT_DEFAULT_PERMISSIONS:
-			config->default_permissions = true;
-			break;
-
-		case OPT_REDIRECT_DIR:
-			kfree(config->redirect_mode);
-			config->redirect_mode = match_strdup(&args[0]);
-			if (!config->redirect_mode)
-				return -ENOMEM;
-			redirect_opt = true;
-			break;
-
-		case OPT_INDEX_ON:
-			config->index = true;
-			index_opt = true;
-			break;
-
-		case OPT_INDEX_OFF:
-			config->index = false;
-			index_opt = true;
-			break;
-
-		case OPT_UUID_ON:
-			config->uuid = true;
-			break;
-
-		case OPT_UUID_OFF:
-			config->uuid = false;
-			break;
-
-		case OPT_NFS_EXPORT_ON:
-			config->nfs_export = true;
-			nfs_export_opt = true;
-			break;
-
-		case OPT_NFS_EXPORT_OFF:
-			config->nfs_export = false;
-			nfs_export_opt = true;
-			break;
-
-		case OPT_XINO_ON:
-			config->xino = OVL_XINO_ON;
-			break;
-
-		case OPT_XINO_OFF:
-			config->xino = OVL_XINO_OFF;
-			break;
-
-		case OPT_XINO_AUTO:
-			config->xino = OVL_XINO_AUTO;
-			break;
-
-		case OPT_METACOPY_ON:
-			config->metacopy = true;
-			metacopy_opt = true;
-			break;
-
-		case OPT_METACOPY_OFF:
-			config->metacopy = false;
-			metacopy_opt = true;
-			break;
-
-		case OPT_VOLATILE:
-			config->ovl_volatile = true;
-			break;
-
-		case OPT_USERXATTR:
-			config->userxattr = true;
-			break;
-
-		default:
-			pr_err("unrecognized mount option \"%s\" or missing value\n",
-					p);
-			return -EINVAL;
-		}
-	}
+	bool metacopy_opt = ctx->set & OVL_METACOPY_SET;
+	bool redirect_opt = ctx->set & OVL_REDIRECT_SET;
+	bool nfs_export_opt = ctx->set & OVL_NFS_EXPORT_SET;
+	bool index_opt = ctx->set & OVL_INDEX_SET;
 
 	/* Workdir/index are useless in non-upper mount */
 	if (!config->upperdir) {
@@ -647,7 +529,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 		config->ovl_volatile = false;
 	}
 
-	err = ovl_parse_redirect_mode(config, config->redirect_mode);
+	err = ovl_set_redirect_mode(config);
 	if (err)
 		return err;
 
@@ -662,7 +544,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 	if (config->metacopy && !config->redirect_dir) {
 		if (metacopy_opt && redirect_opt) {
 			pr_err("conflicting options: metacopy=on,redirect_dir=%s\n",
-			       config->redirect_mode);
+			       ovl_redirect_mode(config));
 			return -EINVAL;
 		}
 		if (redirect_opt) {
@@ -671,7 +553,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			 * in this conflict.
 			 */
 			pr_info("disabling metacopy due to redirect_dir=%s\n",
-				config->redirect_mode);
+				ovl_redirect_mode(config));
 			config->metacopy = false;
 		} else {
 			/* Automatically enable redirect otherwise. */
@@ -728,7 +610,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 	if (config->userxattr) {
 		if (config->redirect_follow && redirect_opt) {
 			pr_err("conflicting options: userxattr,redirect_dir=%s\n",
-			       config->redirect_mode);
+			       ovl_redirect_mode(config));
 			return -EINVAL;
 		}
 		if (config->metacopy && metacopy_opt) {
@@ -1926,12 +1808,128 @@ static struct dentry *ovl_get_root(struct super_block *sb,
 	return root;
 }
 
-static int ovl_fill_super(struct super_block *sb, void *data, int silent)
+static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+	int err = 0;
+	struct fs_parse_result result;
+	struct ovl_fs *ofs = fc->s_fs_info;
+	struct ovl_config *config = &ofs->config;
+	struct ovl_fs_context *ctx = fc->fs_private;
+	char *dup;
+	int opt;
+
+	/*
+	 * On remount overlayfs has always ignored all mount options no
+	 * matter if malformed or not so for backwards compatibility we
+	 * do the same here.
+	 */
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
+		return 0;
+
+	opt = fs_parse(fc, ovl_parameter_spec, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_lowerdir:
+		dup = kstrdup(param->string, GFP_KERNEL);
+		if (!dup) {
+			err = -ENOMEM;
+			break;
+		}
+
+		kfree(config->lowerdir);
+		config->lowerdir = dup;
+		break;
+	case Opt_upperdir:
+		dup = kstrdup(param->string, GFP_KERNEL);
+		if (!dup) {
+			err = -ENOMEM;
+			break;
+		}
+
+		kfree(config->upperdir);
+		config->upperdir = dup;
+		break;
+	case Opt_workdir:
+		dup = kstrdup(param->string, GFP_KERNEL);
+		if (!dup) {
+			err = -ENOMEM;
+			break;
+		}
+
+		kfree(config->workdir);
+		config->workdir = dup;
+		break;
+	case Opt_default_permissions:
+		config->default_permissions = true;
+		break;
+	case Opt_index:
+		config->index = result.uint_32;
+		ctx->set |= OVL_INDEX_SET;
+		break;
+	case Opt_uuid:
+		config->uuid = result.uint_32;
+		break;
+	case Opt_nfs_export:
+		config->nfs_export = result.uint_32;
+		ctx->set |= OVL_NFS_EXPORT_SET;
+		break;
+	case Opt_metacopy:
+		config->metacopy = result.uint_32;
+		ctx->set |= OVL_METACOPY_SET;
+		break;
+	case Opt_userxattr:
+		config->userxattr = true;
+		break;
+	case Opt_volatile:
+		config->ovl_volatile = true;
+		break;
+	case Opt_xino:
+		config->xino = result.uint_32;
+		break;
+	case Opt_redirect_dir:
+		config->redirect_mode = result.uint_32;
+		ctx->set |= OVL_REDIRECT_SET;
+		break;
+	default:
+		pr_err("unrecognized mount option \"%s\" or missing value\n", param->key);
+		return -EINVAL;
+	}
+
+	return err;
+}
+
+
+static int ovl_reconfigure(struct fs_context *fc)
 {
-	struct path upperpath = { };
+	struct super_block *sb = fc->root->d_sb;
+	struct ovl_fs *ofs = sb->s_fs_info;
+	struct super_block *upper_sb;
+	int ret = 0;
+
+	if (!(fc->sb_flags & SB_RDONLY) && ovl_force_readonly(ofs))
+		return -EROFS;
+
+	if (fc->sb_flags & SB_RDONLY && !sb_rdonly(sb)) {
+		upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
+		if (ovl_should_sync(ofs)) {
+			down_read(&upper_sb->s_umount);
+			ret = sync_filesystem(upper_sb);
+			up_read(&upper_sb->s_umount);
+		}
+	}
+
+	return ret;
+}
+
+static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
+{
+	struct ovl_fs *ofs = sb->s_fs_info;
+	struct ovl_fs_context *ctx = fc->fs_private;
+	struct path upperpath = {};
 	struct dentry *root_dentry;
 	struct ovl_entry *oe;
-	struct ovl_fs *ofs;
 	struct ovl_layer *layers;
 	struct cred *cred;
 	char *splitlower = NULL;
@@ -1939,36 +1937,24 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	int err;
 
 	err = -EIO;
-	if (WARN_ON(sb->s_user_ns != current_user_ns()))
-		goto out;
+	if (WARN_ON(fc->user_ns != current_user_ns()))
+		goto out_err;
 
+	ofs->share_whiteout = true;
 	sb->s_d_op = &ovl_dentry_operations;
 
-	err = -ENOMEM;
-	ofs = kzalloc(sizeof(struct ovl_fs), GFP_KERNEL);
-	if (!ofs)
-		goto out;
-
 	err = -ENOMEM;
 	ofs->creator_cred = cred = prepare_creds();
 	if (!cred)
 		goto out_err;
 
-	/* Is there a reason anyone would want not to share whiteouts? */
-	ofs->share_whiteout = true;
-
-	ofs->config.index = ovl_index_def;
-	ofs->config.uuid = true;
-	ofs->config.nfs_export = ovl_nfs_export_def;
-	ofs->config.xino = ovl_xino_def();
-	ofs->config.metacopy = ovl_metacopy_def;
-	err = ovl_parse_opt((char *) data, &ofs->config);
+	err = ovl_fs_params_verify(ctx, &ofs->config);
 	if (err)
 		goto out_err;
 
 	err = -EINVAL;
 	if (!ofs->config.lowerdir) {
-		if (!silent)
+		if (fc->sb_flags & SB_SILENT)
 			pr_err("missing 'lowerdir'\n");
 		goto out_err;
 	}
@@ -2113,25 +2099,90 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 out_free_oe:
 	ovl_free_entry(oe);
 out_err:
-	kfree(splitlower);
-	path_put(&upperpath);
-	ovl_free_fs(ofs);
-out:
 	return err;
 }
 
-static struct dentry *ovl_mount(struct file_system_type *fs_type, int flags,
-				const char *dev_name, void *raw_data)
+static int ovl_get_tree(struct fs_context *fc)
+{
+	return get_tree_nodev(fc, ovl_fill_super);
+}
+
+static inline void ovl_fs_context_free(struct ovl_fs_context *ctx)
+{
+	kfree(ctx);
+}
+
+static void ovl_free(struct fs_context *fc)
 {
-	return mount_nodev(fs_type, flags, raw_data, ovl_fill_super);
+	struct ovl_fs *ofs = fc->s_fs_info;
+	struct ovl_fs_context *ctx = fc->fs_private;
+
+	/*
+	 * ofs is stored in the fs_context when it is initialized.
+	 * ofs is transferred to the superblock on a successful mount,
+	 * but if an error occurs before the transfer we have to free
+	 * it here.
+	 */
+	if (ofs)
+		ovl_free_fs(ofs);
+
+	if (ctx)
+		ovl_fs_context_free(ctx);
+}
+
+static const struct fs_context_operations ovl_context_ops = {
+	.parse_param = ovl_parse_param,
+	.get_tree    = ovl_get_tree,
+	.reconfigure = ovl_reconfigure,
+	.free        = ovl_free,
+};
+
+/*
+ * This is called during fsopen() and will record the user namespace of
+ * the caller in fc->user_ns since we've raised FS_USERNS_MOUNT. We'll
+ * need it when we actually create the superblock to verify that the
+ * process creating the superblock is in the same user namespace as
+ * process that called fsopen().
+ */
+static int ovl_init_fs_context(struct fs_context *fc)
+{
+	struct ovl_fs_context *ctx;
+	struct ovl_fs *ofs;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL_ACCOUNT);
+	if (!ctx)
+		return -ENOMEM;
+
+	ofs = kzalloc(sizeof(struct ovl_fs), GFP_KERNEL);
+	if (!ofs) {
+		ovl_fs_context_free(ctx);
+		return -ENOMEM;
+	}
+
+	if (strcmp(ovl_redirect_mode_def(), "on") == 0)
+		ofs->config.redirect_mode = OVL_REDIRECT_DIR_ON;
+	else
+		ofs->config.redirect_mode = OVL_REDIRECT_DIR_OFF;
+
+	ofs->config.index	= ovl_index_def;
+	ofs->config.uuid	= true;
+	ofs->config.nfs_export	= ovl_nfs_export_def;
+	ofs->config.xino	= ovl_xino_def();
+	ofs->config.metacopy	= ovl_metacopy_def;
+
+	fc->s_fs_info		= ofs;
+	fc->fs_private		= ctx;
+	fc->ops			= &ovl_context_ops;
+	return 0;
 }
 
 static struct file_system_type ovl_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "overlay",
-	.fs_flags	= FS_USERNS_MOUNT,
-	.mount		= ovl_mount,
-	.kill_sb	= kill_anon_super,
+	.owner			= THIS_MODULE,
+	.name			= "overlay",
+	.init_fs_context	= ovl_init_fs_context,
+	.parameters		= ovl_parameter_spec,
+	.fs_flags		= FS_USERNS_MOUNT,
+	.kill_sb		= kill_anon_super,
 };
 MODULE_ALIAS_FS("overlay");
 

-- 
2.34.1

Re: [PATCH v3 2/3] ovl: port to new mount api

[Amir Goldstein]

On Tue, Jun 13, 2023 at 5:49 PM Christian Brauner <brauner@kernel.org> wrote:
>
> We recently ported util-linux to the new mount api. Now the mount(8)
> tool will by default use the new mount api. While trying hard to fall
> back to the old mount api gracefully there are still cases where we run
> into issues that are difficult to handle nicely.
>
> Now with mount(8) and libmount supporting the new mount api I expect an
> increase in the number of bug reports and issues we're going to see with
> filesystems that don't yet support the new mount api. So it's time we
> rectify this.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/overlayfs/ovl_entry.h |   2 +-
>  fs/overlayfs/super.c     | 557 ++++++++++++++++++++++++++---------------------
>  2 files changed, 305 insertions(+), 254 deletions(-)
>
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index e5207c4bf5b8..c72433c06006 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -12,7 +12,7 @@ struct ovl_config {
>         bool default_permissions;
>         bool redirect_dir;
>         bool redirect_follow;
> -       const char *redirect_mode;
> +       unsigned redirect_mode;

I have a separate patch to get rid of redirect_dir and redirect_follow
leaving only redirect_mode enum.

I've already rebased your patches over this change in my branch.

https://github.com/amir73il/linux/commits/fs-overlayfs-mount_api


>         bool index;
>         bool uuid;
>         bool nfs_export;
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index d9be5d318e1b..3392dc5d2082 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -16,6 +16,8 @@
>  #include <linux/posix_acl_xattr.h>
>  #include <linux/exportfs.h>
>  #include <linux/file.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
>  #include "overlayfs.h"
>
>  MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>");
> @@ -59,6 +61,79 @@ module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
>  MODULE_PARM_DESC(metacopy,
>                  "Default to on or off for the metadata only copy up feature");
>
> +enum {
> +       Opt_lowerdir,
> +       Opt_upperdir,
> +       Opt_workdir,
> +       Opt_default_permissions,
> +       Opt_redirect_dir,
> +       Opt_index,
> +       Opt_uuid,
> +       Opt_nfs_export,
> +       Opt_userxattr,
> +       Opt_xino,
> +       Opt_metacopy,
> +       Opt_volatile,
> +};

Renaming all those enums to lower case creates unneeded churn.
I undid that in my branch, so now the mount api porting patch is a
lot smaller.

> +
> +static const struct constant_table ovl_parameter_bool[] = {
> +       { "on",         true  },
> +       { "off",        false },
> +       {}
> +};
> +
> +static const struct constant_table ovl_parameter_xino[] = {
> +       { "on",         OVL_XINO_ON   },
> +       { "off",        OVL_XINO_OFF  },
> +       { "auto",       OVL_XINO_AUTO },
> +       {}
> +};
> +
> +enum {
> +       OVL_REDIRECT_DIR_ON,
> +       OVL_REDIRECT_DIR_OFF,
> +       OVL_REDIRECT_DIR_FOLLOW,
> +       OVL_REDIRECT_DIR_NOFOLLOW,
> +};
> +
> +static const struct constant_table ovl_parameter_redirect_dir[] = {
> +       { "on",         OVL_REDIRECT_DIR_ON       },
> +       { "off",        OVL_REDIRECT_DIR_OFF      },
> +       { "follow",     OVL_REDIRECT_DIR_FOLLOW   },
> +       { "nofollow",   OVL_REDIRECT_DIR_NOFOLLOW },
> +       {}
> +};
> +
> +static const char *ovl_redirect_mode(struct ovl_config *config)
> +{
> +       return ovl_parameter_redirect_dir[config->redirect_mode].name;
> +}
> +
> +static const struct fs_parameter_spec ovl_parameter_spec[] = {
> +       fsparam_string("lowerdir",          Opt_lowerdir),
> +       fsparam_string("upperdir",          Opt_upperdir),
> +       fsparam_string("workdir",           Opt_workdir),
> +       fsparam_flag("default_permissions", Opt_default_permissions),
> +       fsparam_enum("redirect_dir",        Opt_redirect_dir, ovl_parameter_redirect_dir),
> +       fsparam_enum("index",               Opt_index, ovl_parameter_bool),
> +       fsparam_enum("uuid",                Opt_uuid, ovl_parameter_bool),
> +       fsparam_enum("nfs_export",          Opt_nfs_export, ovl_parameter_bool),
> +       fsparam_flag("userxattr",           Opt_userxattr),
> +       fsparam_enum("xino",                Opt_xino, ovl_parameter_xino),
> +       fsparam_enum("metacopy",            Opt_metacopy, ovl_parameter_bool),
> +       fsparam_flag("volatile",            Opt_volatile),
> +       {}
> +};
> +
> +#define OVL_METACOPY_SET       BIT(0)
> +#define OVL_REDIRECT_SET       BIT(1)
> +#define OVL_NFS_EXPORT_SET     BIT(2)
> +#define OVL_INDEX_SET          BIT(3)
> +
> +struct ovl_fs_context {
> +       u8 set;
> +};
> +
>  static struct dentry *ovl_d_real(struct dentry *dentry,
>                                  const struct inode *inode)
>  {
> @@ -243,7 +318,6 @@ static void ovl_free_fs(struct ovl_fs *ofs)
>         kfree(ofs->config.lowerdir);
>         kfree(ofs->config.upperdir);
>         kfree(ofs->config.workdir);
> -       kfree(ofs->config.redirect_mode);
>         if (ofs->creator_cred)
>                 put_cred(ofs->creator_cred);
>         kfree(ofs);
> @@ -253,7 +327,8 @@ static void ovl_put_super(struct super_block *sb)
>  {
>         struct ovl_fs *ofs = sb->s_fs_info;
>
> -       ovl_free_fs(ofs);
> +       if (ofs)
> +               ovl_free_fs(ofs);
>  }
>
>  /* Sync real dirty inodes in upper filesystem (if it exists) */
> @@ -357,6 +432,7 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
>  {
>         struct super_block *sb = dentry->d_sb;
>         struct ovl_fs *ofs = sb->s_fs_info;
> +       const char *redirect_mode;
>
>         seq_show_option(m, "lowerdir", ofs->config.lowerdir);
>         if (ofs->config.upperdir) {
> @@ -365,8 +441,9 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
>         }
>         if (ofs->config.default_permissions)
>                 seq_puts(m, ",default_permissions");
> -       if (strcmp(ofs->config.redirect_mode, ovl_redirect_mode_def()) != 0)
> -               seq_printf(m, ",redirect_dir=%s", ofs->config.redirect_mode);
> +       redirect_mode = ovl_redirect_mode(&ofs->config);
> +       if (strcmp(redirect_mode, ovl_redirect_mode_def()) != 0)
> +               seq_printf(m, ",redirect_dir=%s", redirect_mode);
>         if (ofs->config.index != ovl_index_def)
>                 seq_printf(m, ",index=%s", ofs->config.index ? "on" : "off");
>         if (!ofs->config.uuid)
> @@ -386,27 +463,6 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
>         return 0;
>  }
>
> -static int ovl_remount(struct super_block *sb, int *flags, char *data)
> -{
> -       struct ovl_fs *ofs = sb->s_fs_info;
> -       struct super_block *upper_sb;
> -       int ret = 0;
> -
> -       if (!(*flags & SB_RDONLY) && ovl_force_readonly(ofs))
> -               return -EROFS;
> -
> -       if (*flags & SB_RDONLY && !sb_rdonly(sb)) {
> -               upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
> -               if (ovl_should_sync(ofs)) {
> -                       down_read(&upper_sb->s_umount);
> -                       ret = sync_filesystem(upper_sb);
> -                       up_read(&upper_sb->s_umount);
> -               }
> -       }
> -
> -       return ret;
> -}

Moving code around and changing it at the same time makes
reviewing very hard. I undid the move in my patch.
If you want to rearrange the functions in the file afterwards
I don't mind.

> -
>  static const struct super_operations ovl_super_operations = {
>         .alloc_inode    = ovl_alloc_inode,
>         .free_inode     = ovl_free_inode,
> @@ -416,216 +472,42 @@ static const struct super_operations ovl_super_operations = {
>         .sync_fs        = ovl_sync_fs,
>         .statfs         = ovl_statfs,
>         .show_options   = ovl_show_options,
> -       .remount_fs     = ovl_remount,
> -};
> -
> -enum {
> -       OPT_LOWERDIR,
> -       OPT_UPPERDIR,
> -       OPT_WORKDIR,
> -       OPT_DEFAULT_PERMISSIONS,
> -       OPT_REDIRECT_DIR,
> -       OPT_INDEX_ON,
> -       OPT_INDEX_OFF,
> -       OPT_UUID_ON,
> -       OPT_UUID_OFF,
> -       OPT_NFS_EXPORT_ON,
> -       OPT_USERXATTR,
> -       OPT_NFS_EXPORT_OFF,
> -       OPT_XINO_ON,
> -       OPT_XINO_OFF,
> -       OPT_XINO_AUTO,
> -       OPT_METACOPY_ON,
> -       OPT_METACOPY_OFF,
> -       OPT_VOLATILE,
> -       OPT_ERR,
> -};
> -
> -static const match_table_t ovl_tokens = {
> -       {OPT_LOWERDIR,                  "lowerdir=%s"},
> -       {OPT_UPPERDIR,                  "upperdir=%s"},
> -       {OPT_WORKDIR,                   "workdir=%s"},
> -       {OPT_DEFAULT_PERMISSIONS,       "default_permissions"},
> -       {OPT_REDIRECT_DIR,              "redirect_dir=%s"},
> -       {OPT_INDEX_ON,                  "index=on"},
> -       {OPT_INDEX_OFF,                 "index=off"},
> -       {OPT_USERXATTR,                 "userxattr"},
> -       {OPT_UUID_ON,                   "uuid=on"},
> -       {OPT_UUID_OFF,                  "uuid=off"},
> -       {OPT_NFS_EXPORT_ON,             "nfs_export=on"},
> -       {OPT_NFS_EXPORT_OFF,            "nfs_export=off"},
> -       {OPT_XINO_ON,                   "xino=on"},
> -       {OPT_XINO_OFF,                  "xino=off"},
> -       {OPT_XINO_AUTO,                 "xino=auto"},
> -       {OPT_METACOPY_ON,               "metacopy=on"},
> -       {OPT_METACOPY_OFF,              "metacopy=off"},
> -       {OPT_VOLATILE,                  "volatile"},
> -       {OPT_ERR,                       NULL}
>  };
>
> -static char *ovl_next_opt(char **s)
> +static int ovl_set_redirect_mode(struct ovl_config *config)
>  {
> -       char *sbegin = *s;
> -       char *p;
> -
> -       if (sbegin == NULL)
> -               return NULL;
> -
> -       for (p = sbegin; *p; p++) {
> -               if (*p == '\\') {
> -                       p++;
> -                       if (!*p)
> -                               break;
> -               } else if (*p == ',') {
> -                       *p = '\0';
> -                       *s = p + 1;
> -                       return sbegin;
> -               }
> -       }
> -       *s = NULL;
> -       return sbegin;
> -}
> -
> -static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
> -{
> -       if (strcmp(mode, "on") == 0) {
> +       switch (config->redirect_mode) {
> +       case OVL_REDIRECT_DIR_ON:
>                 config->redirect_dir = true;
>                 /*
>                  * Does not make sense to have redirect creation without
>                  * redirect following.
>                  */
>                 config->redirect_follow = true;
> -       } else if (strcmp(mode, "follow") == 0) {
> +               return 0;
> +       case OVL_REDIRECT_DIR_FOLLOW:
>                 config->redirect_follow = true;
> -       } else if (strcmp(mode, "off") == 0) {
> +               return 0;
> +       case OVL_REDIRECT_DIR_OFF:
>                 if (ovl_redirect_always_follow)
>                         config->redirect_follow = true;
> -       } else if (strcmp(mode, "nofollow") != 0) {
> -               pr_err("bad mount option \"redirect_dir=%s\"\n",
> -                      mode);
> -               return -EINVAL;
> +               return 0;
> +       case OVL_REDIRECT_DIR_NOFOLLOW:
> +               return 0;
>         }
>
> -       return 0;
> +       pr_err("invalid \"redirect_dir\" mount option\n");
> +       return -EINVAL;
>  }
>
> -static int ovl_parse_opt(char *opt, struct ovl_config *config)
> +static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
> +                               struct ovl_config *config)
>  {
> -       char *p;
>         int err;
> -       bool metacopy_opt = false, redirect_opt = false;
> -       bool nfs_export_opt = false, index_opt = false;
> -
> -       config->redirect_mode = kstrdup(ovl_redirect_mode_def(), GFP_KERNEL);
> -       if (!config->redirect_mode)
> -               return -ENOMEM;
> -
> -       while ((p = ovl_next_opt(&opt)) != NULL) {
> -               int token;
> -               substring_t args[MAX_OPT_ARGS];
> -
> -               if (!*p)
> -                       continue;
> -
> -               token = match_token(p, ovl_tokens, args);
> -               switch (token) {
> -               case OPT_UPPERDIR:
> -                       kfree(config->upperdir);
> -                       config->upperdir = match_strdup(&args[0]);
> -                       if (!config->upperdir)
> -                               return -ENOMEM;
> -                       break;
> -
> -               case OPT_LOWERDIR:
> -                       kfree(config->lowerdir);
> -                       config->lowerdir = match_strdup(&args[0]);
> -                       if (!config->lowerdir)
> -                               return -ENOMEM;
> -                       break;
> -
> -               case OPT_WORKDIR:
> -                       kfree(config->workdir);
> -                       config->workdir = match_strdup(&args[0]);
> -                       if (!config->workdir)
> -                               return -ENOMEM;
> -                       break;
> -
> -               case OPT_DEFAULT_PERMISSIONS:
> -                       config->default_permissions = true;
> -                       break;
> -
> -               case OPT_REDIRECT_DIR:
> -                       kfree(config->redirect_mode);
> -                       config->redirect_mode = match_strdup(&args[0]);
> -                       if (!config->redirect_mode)
> -                               return -ENOMEM;
> -                       redirect_opt = true;
> -                       break;
> -
> -               case OPT_INDEX_ON:
> -                       config->index = true;
> -                       index_opt = true;
> -                       break;
> -
> -               case OPT_INDEX_OFF:
> -                       config->index = false;
> -                       index_opt = true;
> -                       break;
> -
> -               case OPT_UUID_ON:
> -                       config->uuid = true;
> -                       break;
> -
> -               case OPT_UUID_OFF:
> -                       config->uuid = false;
> -                       break;
> -
> -               case OPT_NFS_EXPORT_ON:
> -                       config->nfs_export = true;
> -                       nfs_export_opt = true;
> -                       break;
> -
> -               case OPT_NFS_EXPORT_OFF:
> -                       config->nfs_export = false;
> -                       nfs_export_opt = true;
> -                       break;
> -
> -               case OPT_XINO_ON:
> -                       config->xino = OVL_XINO_ON;
> -                       break;
> -
> -               case OPT_XINO_OFF:
> -                       config->xino = OVL_XINO_OFF;
> -                       break;
> -
> -               case OPT_XINO_AUTO:
> -                       config->xino = OVL_XINO_AUTO;
> -                       break;
> -
> -               case OPT_METACOPY_ON:
> -                       config->metacopy = true;
> -                       metacopy_opt = true;
> -                       break;
> -
> -               case OPT_METACOPY_OFF:
> -                       config->metacopy = false;
> -                       metacopy_opt = true;
> -                       break;
> -
> -               case OPT_VOLATILE:
> -                       config->ovl_volatile = true;
> -                       break;
> -
> -               case OPT_USERXATTR:
> -                       config->userxattr = true;
> -                       break;
> -
> -               default:
> -                       pr_err("unrecognized mount option \"%s\" or missing value\n",
> -                                       p);
> -                       return -EINVAL;
> -               }
> -       }
> +       bool metacopy_opt = ctx->set & OVL_METACOPY_SET;
> +       bool redirect_opt = ctx->set & OVL_REDIRECT_SET;
> +       bool nfs_export_opt = ctx->set & OVL_NFS_EXPORT_SET;
> +       bool index_opt = ctx->set & OVL_INDEX_SET;
>
>         /* Workdir/index are useless in non-upper mount */
>         if (!config->upperdir) {
> @@ -647,7 +529,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>                 config->ovl_volatile = false;
>         }
>
> -       err = ovl_parse_redirect_mode(config, config->redirect_mode);
> +       err = ovl_set_redirect_mode(config);
>         if (err)
>                 return err;
>
> @@ -662,7 +544,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>         if (config->metacopy && !config->redirect_dir) {
>                 if (metacopy_opt && redirect_opt) {
>                         pr_err("conflicting options: metacopy=on,redirect_dir=%s\n",
> -                              config->redirect_mode);
> +                              ovl_redirect_mode(config));
>                         return -EINVAL;
>                 }
>                 if (redirect_opt) {
> @@ -671,7 +553,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>                          * in this conflict.
>                          */
>                         pr_info("disabling metacopy due to redirect_dir=%s\n",
> -                               config->redirect_mode);
> +                               ovl_redirect_mode(config));
>                         config->metacopy = false;
>                 } else {
>                         /* Automatically enable redirect otherwise. */
> @@ -728,7 +610,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>         if (config->userxattr) {
>                 if (config->redirect_follow && redirect_opt) {
>                         pr_err("conflicting options: userxattr,redirect_dir=%s\n",
> -                              config->redirect_mode);
> +                              ovl_redirect_mode(config));
>                         return -EINVAL;
>                 }
>                 if (config->metacopy && metacopy_opt) {
> @@ -1926,12 +1808,128 @@ static struct dentry *ovl_get_root(struct super_block *sb,
>         return root;
>  }
>
> -static int ovl_fill_super(struct super_block *sb, void *data, int silent)
> +static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
> +{
> +       int err = 0;
> +       struct fs_parse_result result;
> +       struct ovl_fs *ofs = fc->s_fs_info;
> +       struct ovl_config *config = &ofs->config;
> +       struct ovl_fs_context *ctx = fc->fs_private;
> +       char *dup;
> +       int opt;
> +
> +       /*
> +        * On remount overlayfs has always ignored all mount options no
> +        * matter if malformed or not so for backwards compatibility we
> +        * do the same here.
> +        */
> +       if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
> +               return 0;
> +
> +       opt = fs_parse(fc, ovl_parameter_spec, param, &result);
> +       if (opt < 0)
> +               return opt;
> +
> +       switch (opt) {
> +       case Opt_lowerdir:
> +               dup = kstrdup(param->string, GFP_KERNEL);
> +               if (!dup) {
> +                       err = -ENOMEM;
> +                       break;
> +               }
> +
> +               kfree(config->lowerdir);
> +               config->lowerdir = dup;
> +               break;
> +       case Opt_upperdir:
> +               dup = kstrdup(param->string, GFP_KERNEL);
> +               if (!dup) {
> +                       err = -ENOMEM;
> +                       break;
> +               }
> +
> +               kfree(config->upperdir);
> +               config->upperdir = dup;
> +               break;
> +       case Opt_workdir:
> +               dup = kstrdup(param->string, GFP_KERNEL);
> +               if (!dup) {
> +                       err = -ENOMEM;
> +                       break;
> +               }
> +
> +               kfree(config->workdir);
> +               config->workdir = dup;
> +               break;
> +       case Opt_default_permissions:
> +               config->default_permissions = true;
> +               break;
> +       case Opt_index:
> +               config->index = result.uint_32;
> +               ctx->set |= OVL_INDEX_SET;
> +               break;
> +       case Opt_uuid:
> +               config->uuid = result.uint_32;
> +               break;
> +       case Opt_nfs_export:
> +               config->nfs_export = result.uint_32;
> +               ctx->set |= OVL_NFS_EXPORT_SET;
> +               break;
> +       case Opt_metacopy:
> +               config->metacopy = result.uint_32;
> +               ctx->set |= OVL_METACOPY_SET;
> +               break;
> +       case Opt_userxattr:
> +               config->userxattr = true;
> +               break;
> +       case Opt_volatile:
> +               config->ovl_volatile = true;
> +               break;
> +       case Opt_xino:
> +               config->xino = result.uint_32;
> +               break;
> +       case Opt_redirect_dir:
> +               config->redirect_mode = result.uint_32;
> +               ctx->set |= OVL_REDIRECT_SET;
> +               break;
> +       default:
> +               pr_err("unrecognized mount option \"%s\" or missing value\n", param->key);
> +               return -EINVAL;
> +       }
> +
> +       return err;
> +}
> +
> +
> +static int ovl_reconfigure(struct fs_context *fc)
>  {
> -       struct path upperpath = { };
> +       struct super_block *sb = fc->root->d_sb;
> +       struct ovl_fs *ofs = sb->s_fs_info;
> +       struct super_block *upper_sb;
> +       int ret = 0;
> +
> +       if (!(fc->sb_flags & SB_RDONLY) && ovl_force_readonly(ofs))
> +               return -EROFS;
> +
> +       if (fc->sb_flags & SB_RDONLY && !sb_rdonly(sb)) {
> +               upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
> +               if (ovl_should_sync(ofs)) {
> +                       down_read(&upper_sb->s_umount);
> +                       ret = sync_filesystem(upper_sb);
> +                       up_read(&upper_sb->s_umount);
> +               }
> +       }
> +
> +       return ret;
> +}
> +
> +static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
> +{
> +       struct ovl_fs *ofs = sb->s_fs_info;
> +       struct ovl_fs_context *ctx = fc->fs_private;
> +       struct path upperpath = {};
>         struct dentry *root_dentry;
>         struct ovl_entry *oe;
> -       struct ovl_fs *ofs;
>         struct ovl_layer *layers;
>         struct cred *cred;
>         char *splitlower = NULL;
> @@ -1939,36 +1937,24 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>         int err;
>
>         err = -EIO;
> -       if (WARN_ON(sb->s_user_ns != current_user_ns()))
> -               goto out;
> +       if (WARN_ON(fc->user_ns != current_user_ns()))
> +               goto out_err;
>
> +       ofs->share_whiteout = true;
>         sb->s_d_op = &ovl_dentry_operations;
>
> -       err = -ENOMEM;
> -       ofs = kzalloc(sizeof(struct ovl_fs), GFP_KERNEL);
> -       if (!ofs)
> -               goto out;
> -
>         err = -ENOMEM;
>         ofs->creator_cred = cred = prepare_creds();
>         if (!cred)
>                 goto out_err;
>
> -       /* Is there a reason anyone would want not to share whiteouts? */
> -       ofs->share_whiteout = true;
> -
> -       ofs->config.index = ovl_index_def;
> -       ofs->config.uuid = true;
> -       ofs->config.nfs_export = ovl_nfs_export_def;
> -       ofs->config.xino = ovl_xino_def();
> -       ofs->config.metacopy = ovl_metacopy_def;
> -       err = ovl_parse_opt((char *) data, &ofs->config);
> +       err = ovl_fs_params_verify(ctx, &ofs->config);
>         if (err)
>                 goto out_err;
>
>         err = -EINVAL;
>         if (!ofs->config.lowerdir) {
> -               if (!silent)
> +               if (fc->sb_flags & SB_SILENT)

Test is negated here.
I fixed it in my branch.
No need to re-send.

>                         pr_err("missing 'lowerdir'\n");
>                 goto out_err;
>         }
> @@ -2113,25 +2099,90 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>  out_free_oe:
>         ovl_free_entry(oe);
>  out_err:
> -       kfree(splitlower);
> -       path_put(&upperpath);
> -       ovl_free_fs(ofs);
> -out:

Folded your fix here:
       ovl_free_fs(ofs);
       sb->s_fs_info = NULL;

>         return err;
>  }
>

Thanks,
Amir.

Re: [PATCH v3 2/3] ovl: port to new mount api

[Christian Brauner]

On Fri, Jun 16, 2023 at 04:21:29PM +0300, Amir Goldstein wrote:
> On Tue, Jun 13, 2023 at 5:49 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > We recently ported util-linux to the new mount api. Now the mount(8)
> > tool will by default use the new mount api. While trying hard to fall
> > back to the old mount api gracefully there are still cases where we run
> > into issues that are difficult to handle nicely.
> >
> > Now with mount(8) and libmount supporting the new mount api I expect an
> > increase in the number of bug reports and issues we're going to see with
> > filesystems that don't yet support the new mount api. So it's time we
> > rectify this.
> >
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  fs/overlayfs/ovl_entry.h |   2 +-
> >  fs/overlayfs/super.c     | 557 ++++++++++++++++++++++++++---------------------
> >  2 files changed, 305 insertions(+), 254 deletions(-)
> >
> > diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> > index e5207c4bf5b8..c72433c06006 100644
> > --- a/fs/overlayfs/ovl_entry.h
> > +++ b/fs/overlayfs/ovl_entry.h
> > @@ -12,7 +12,7 @@ struct ovl_config {
> >         bool default_permissions;
> >         bool redirect_dir;
> >         bool redirect_follow;
> > -       const char *redirect_mode;
> > +       unsigned redirect_mode;
> 
> I have a separate patch to get rid of redirect_dir and redirect_follow
> leaving only redirect_mode enum.
> 
> I've already rebased your patches over this change in my branch.
> 
> https://github.com/amir73il/linux/commits/fs-overlayfs-mount_api
> 
> 
> >         bool index;
> >         bool uuid;
> >         bool nfs_export;
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index d9be5d318e1b..3392dc5d2082 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
> > @@ -16,6 +16,8 @@
> >  #include <linux/posix_acl_xattr.h>
> >  #include <linux/exportfs.h>
> >  #include <linux/file.h>
> > +#include <linux/fs_context.h>
> > +#include <linux/fs_parser.h>
> >  #include "overlayfs.h"
> >
> >  MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>");
> > @@ -59,6 +61,79 @@ module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
> >  MODULE_PARM_DESC(metacopy,
> >                  "Default to on or off for the metadata only copy up feature");
> >
> > +enum {
> > +       Opt_lowerdir,
> > +       Opt_upperdir,
> > +       Opt_workdir,
> > +       Opt_default_permissions,
> > +       Opt_redirect_dir,
> > +       Opt_index,
> > +       Opt_uuid,
> > +       Opt_nfs_export,
> > +       Opt_userxattr,
> > +       Opt_xino,
> > +       Opt_metacopy,
> > +       Opt_volatile,
> > +};
> 
> Renaming all those enums to lower case creates unneeded churn.
> I undid that in my branch, so now the mount api porting patch is a
> lot smaller.

Every single filesystem apart from fuse and overlayfs uses the standard
"Opt_" syntax. Only fuse and overlayfs use OPT_* all uppercase. Even the
documenation uses Opt_* throughout.

So I'd appreciate it if you please did not go out of your way to deviate
from this. I already did the work to convert to the Opt_* format on
purpose. If you want less churn however, then you can ofc move this to a
preparatory patch that converts to the standard format.

Re: [PATCH v3 2/3] ovl: port to new mount api

[Amir Goldstein]

On Fri, Jun 16, 2023 at 4:28 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Jun 16, 2023 at 04:21:29PM +0300, Amir Goldstein wrote:
> > On Tue, Jun 13, 2023 at 5:49 PM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > We recently ported util-linux to the new mount api. Now the mount(8)
> > > tool will by default use the new mount api. While trying hard to fall
> > > back to the old mount api gracefully there are still cases where we run
> > > into issues that are difficult to handle nicely.
> > >
> > > Now with mount(8) and libmount supporting the new mount api I expect an
> > > increase in the number of bug reports and issues we're going to see with
> > > filesystems that don't yet support the new mount api. So it's time we
> > > rectify this.
> > >
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > ---
> > >  fs/overlayfs/ovl_entry.h |   2 +-
> > >  fs/overlayfs/super.c     | 557 ++++++++++++++++++++++++++---------------------
> > >  2 files changed, 305 insertions(+), 254 deletions(-)
> > >
> > > diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> > > index e5207c4bf5b8..c72433c06006 100644
> > > --- a/fs/overlayfs/ovl_entry.h
> > > +++ b/fs/overlayfs/ovl_entry.h
> > > @@ -12,7 +12,7 @@ struct ovl_config {
> > >         bool default_permissions;
> > >         bool redirect_dir;
> > >         bool redirect_follow;
> > > -       const char *redirect_mode;
> > > +       unsigned redirect_mode;
> >
> > I have a separate patch to get rid of redirect_dir and redirect_follow
> > leaving only redirect_mode enum.
> >
> > I've already rebased your patches over this change in my branch.
> >
> > https://github.com/amir73il/linux/commits/fs-overlayfs-mount_api
> >
> >
> > >         bool index;
> > >         bool uuid;
> > >         bool nfs_export;
> > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > > index d9be5d318e1b..3392dc5d2082 100644
> > > --- a/fs/overlayfs/super.c
> > > +++ b/fs/overlayfs/super.c
> > > @@ -16,6 +16,8 @@
> > >  #include <linux/posix_acl_xattr.h>
> > >  #include <linux/exportfs.h>
> > >  #include <linux/file.h>
> > > +#include <linux/fs_context.h>
> > > +#include <linux/fs_parser.h>
> > >  #include "overlayfs.h"
> > >
> > >  MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>");
> > > @@ -59,6 +61,79 @@ module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
> > >  MODULE_PARM_DESC(metacopy,
> > >                  "Default to on or off for the metadata only copy up feature");
> > >
> > > +enum {
> > > +       Opt_lowerdir,
> > > +       Opt_upperdir,
> > > +       Opt_workdir,
> > > +       Opt_default_permissions,
> > > +       Opt_redirect_dir,
> > > +       Opt_index,
> > > +       Opt_uuid,
> > > +       Opt_nfs_export,
> > > +       Opt_userxattr,
> > > +       Opt_xino,
> > > +       Opt_metacopy,
> > > +       Opt_volatile,
> > > +};
> >
> > Renaming all those enums to lower case creates unneeded churn.
> > I undid that in my branch, so now the mount api porting patch is a
> > lot smaller.
>
> Every single filesystem apart from fuse and overlayfs uses the standard
> "Opt_" syntax. Only fuse and overlayfs use OPT_* all uppercase. Even the
> documenation uses Opt_* throughout.
>
> So I'd appreciate it if you please did not go out of your way to deviate
> from this. I already did the work to convert to the Opt_* format on
> purpose. If you want less churn however, then you can ofc move this to a
> preparatory patch that converts to the standard format.

okay.
as long as the logical changes are separate from the renaming.

Thanks,
Amir.

Re: [PATCH v3 2/3] ovl: port to new mount api

[Amir Goldstein]

On Fri, Jun 16, 2023 at 4:37 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Jun 16, 2023 at 4:28 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Fri, Jun 16, 2023 at 04:21:29PM +0300, Amir Goldstein wrote:
> > > On Tue, Jun 13, 2023 at 5:49 PM Christian Brauner <brauner@kernel.org> wrote:
> > > >
> > > > We recently ported util-linux to the new mount api. Now the mount(8)
> > > > tool will by default use the new mount api. While trying hard to fall
> > > > back to the old mount api gracefully there are still cases where we run
> > > > into issues that are difficult to handle nicely.
> > > >
> > > > Now with mount(8) and libmount supporting the new mount api I expect an
> > > > increase in the number of bug reports and issues we're going to see with
> > > > filesystems that don't yet support the new mount api. So it's time we
> > > > rectify this.
> > > >
> > > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > > ---
> > > >  fs/overlayfs/ovl_entry.h |   2 +-
> > > >  fs/overlayfs/super.c     | 557 ++++++++++++++++++++++++++---------------------
> > > >  2 files changed, 305 insertions(+), 254 deletions(-)
> > > >
> > > > diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> > > > index e5207c4bf5b8..c72433c06006 100644
> > > > --- a/fs/overlayfs/ovl_entry.h
> > > > +++ b/fs/overlayfs/ovl_entry.h
> > > > @@ -12,7 +12,7 @@ struct ovl_config {
> > > >         bool default_permissions;
> > > >         bool redirect_dir;
> > > >         bool redirect_follow;
> > > > -       const char *redirect_mode;
> > > > +       unsigned redirect_mode;
> > >
> > > I have a separate patch to get rid of redirect_dir and redirect_follow
> > > leaving only redirect_mode enum.
> > >
> > > I've already rebased your patches over this change in my branch.
> > >
> > > https://github.com/amir73il/linux/commits/fs-overlayfs-mount_api
> > >
> > >
> > > >         bool index;
> > > >         bool uuid;
> > > >         bool nfs_export;
> > > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > > > index d9be5d318e1b..3392dc5d2082 100644
> > > > --- a/fs/overlayfs/super.c
> > > > +++ b/fs/overlayfs/super.c
> > > > @@ -16,6 +16,8 @@
> > > >  #include <linux/posix_acl_xattr.h>
> > > >  #include <linux/exportfs.h>
> > > >  #include <linux/file.h>
> > > > +#include <linux/fs_context.h>
> > > > +#include <linux/fs_parser.h>
> > > >  #include "overlayfs.h"
> > > >
> > > >  MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>");
> > > > @@ -59,6 +61,79 @@ module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
> > > >  MODULE_PARM_DESC(metacopy,
> > > >                  "Default to on or off for the metadata only copy up feature");
> > > >
> > > > +enum {
> > > > +       Opt_lowerdir,
> > > > +       Opt_upperdir,
> > > > +       Opt_workdir,
> > > > +       Opt_default_permissions,
> > > > +       Opt_redirect_dir,
> > > > +       Opt_index,
> > > > +       Opt_uuid,
> > > > +       Opt_nfs_export,
> > > > +       Opt_userxattr,
> > > > +       Opt_xino,
> > > > +       Opt_metacopy,
> > > > +       Opt_volatile,
> > > > +};
> > >
> > > Renaming all those enums to lower case creates unneeded churn.
> > > I undid that in my branch, so now the mount api porting patch is a
> > > lot smaller.
> >
> > Every single filesystem apart from fuse and overlayfs uses the standard
> > "Opt_" syntax. Only fuse and overlayfs use OPT_* all uppercase. Even the
> > documenation uses Opt_* throughout.
> >
> > So I'd appreciate it if you please did not go out of your way to deviate
> > from this. I already did the work to convert to the Opt_* format on
> > purpose. If you want less churn however, then you can ofc move this to a
> > preparatory patch that converts to the standard format.
>
> okay.
> as long as the logical changes are separate from the renaming.

Looking at it again, after my prep patches, this enum rename
does not make reviewing harder, so I restored the Opt_* names
and pushed to my branch.

Thanks,
Amir.

[PATCH v3 3/3] ovl: change layer mount option handling

[Christian Brauner]

We ran into issues where mount(8) passed multiple lower layers as one
big string through fsconfig(). But the fsconfig() FSCONFIG_SET_STRING
option is limited to 256 bytes in strndup_user(). While this would be
fixable by extending the fsconfig() buffer I'd rather encourage users to
append layers via multiple fsconfig() calls as the interface allows
nicely for this. This has also been requested as a feature before.

With this port to the new mount api the following will be possible:

        fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir", "/lower1", 0);

        /* set upper layer */
        fsconfig(fs_fd, FSCONFIG_SET_STRING, "upperdir", "/upper", 0);

        /* append "/lower2", "/lower3", and "/lower4" */
        fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir", ":/lower2:/lower3:/lower4", 0);

        /* turn index feature on */
        fsconfig(fs_fd, FSCONFIG_SET_STRING, "index", "on", 0);

        /* append "/lower5" */
        fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir", ":/lower5", 0);

Specifying ':' would have been rejected so this isn't a regression. And
we can't simply use "lowerdir=/lower" to append on top of existing
layers as "lowerdir=/lower,lowerdir=/other-lower" would make
"/other-lower" the only lower layer so we'd break uapi if we changed
this. So the ':' prefix seems a good compromise.

Users can choose to specify multiple layers at once or individual
layers. A layer is appended if it starts with ":". This requires that
the user has already added at least one layer before. If lowerdir is
specified again without a leading ":" then all previous layers are
dropped and replaced with the new layers. If lowerdir is specified and
empty than all layers are simply dropped.

An additional change is that overlayfs will now parse and resolve layers
right when they are specified in fsconfig() instead of deferring until
super block creation. This allows users to receive early errors.

It also allows users to actually use up to 500 layers something which
was theoretically possible but ended up not working due to the mount
option string passed via mount(2) being too large.

This also allows a more privileged process to set config options for a
lesser privileged process as the creds for fsconfig() and the creds for
fsopen() can differ. We could restrict that they match by enforcing that
the creds of fsopen() and fsconfig() match but I don't see why that
needs to be the case and allows for a good delegation mechanism.

Plus, in the future it means we're able to extend overlayfs mount
options and allow users to specify layers via file descriptors instead
of paths:

        fsconfig(FSCONFIG_SET_PATH{_EMPTY}, "lowerdir", "lower1", dirfd);

        /* append */
        fsconfig(FSCONFIG_SET_PATH{_EMPTY}, "lowerdir", "lower2", dirfd);

        /* append */
        fsconfig(FSCONFIG_SET_PATH{_EMPTY}, "lowerdir", "lower3", dirfd);

        /* clear all layers specified until now */
        fsconfig(FSCONFIG_SET_STRING, "lowerdir", NULL, 0);

This would be especially nice if users create an overlayfs mount on top
of idmapped layers or just in general private mounts created via
open_tree(OPEN_TREE_CLONE). Those mounts would then never have to appear
anywhere in the filesystem. But for now just do the minimal thing.

We should probably aim to move more validation into ovl_fs_parse_param()
so users get errors before fsconfig(FSCONFIG_CMD_CREATE). But that can
be done in additional patches later.

This is now also rebased on top of the lazy lowerdata lookup which
allows the specificatin of data only layers using the new "::" syntax.

The rules are simple. A data only layers cannot be followed by any
regular layers and data layers must be preceeded by at least one regular
layer.

Parsing the lowerdir mount option must change because of this. The
original patchset used the old lowerdir parsing function to split a
lowerdir mount option string such as:

        lowerdir=/lower1:/lower2::/lower3::/lower4

simply replacing each non-escaped ":" by "\0". So sequences of
non-escaped ":" were counted as layers. For example, the previous
lowerdir mount option above would've counted 6 layers instead of 4 and a
lowerdir mount option such as:

        lowerdir="/lower1:/lower2::/lower3::/lower4:::::::::::::::::::::::::::"

would be counted as 33 layers. Other than being ugly this didn't matter
much because kern_path() would reject the first "\0" layer. However,
this overcounting of layers becomes problematic when we base allocations
on it where we very much only want to allocate space for 4 layers
instead of 33.

So the new parsing function rejects non-escaped sequences of colons
other than ":" and "::" immediately instead of relying on kern_path().

Link: https://github.com/util-linux/util-linux/issues/2287
Link: https://github.com/util-linux/util-linux/issues/1992
Link: https://bugs.archlinux.org/task/78702
Link: https://lore.kernel.org/linux-unionfs/20230530-klagen-zudem-32c0908c2108@brauner
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/overlayfs/Makefile    |   2 +-
 fs/overlayfs/overlayfs.h |  23 +++
 fs/overlayfs/ovl_entry.h |   3 +-
 fs/overlayfs/params.c    | 388 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/overlayfs/super.c     | 376 +++++++++++++++------------------------------
 5 files changed, 534 insertions(+), 258 deletions(-)

diff --git a/fs/overlayfs/Makefile b/fs/overlayfs/Makefile
index 9164c585eb2f..4e173d56b11f 100644
--- a/fs/overlayfs/Makefile
+++ b/fs/overlayfs/Makefile
@@ -6,4 +6,4 @@
 obj-$(CONFIG_OVERLAY_FS) += overlay.o
 
 overlay-objs := super.o namei.o util.o inode.o file.o dir.o readdir.o \
-		copy_up.o export.o
+		copy_up.o export.o params.o
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index fcac4e2c56ab..7659ea6e02cb 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -119,6 +119,29 @@ struct ovl_fh {
 #define OVL_FH_FID_OFFSET	(OVL_FH_WIRE_OFFSET + \
 				 offsetof(struct ovl_fb, fid))
 
+/* params.c */
+#define OVL_MAX_STACK 500
+
+struct ovl_fs_context_layer {
+	char *name;
+	struct path path;
+};
+
+struct ovl_fs_context {
+	struct path upper;
+	struct path work;
+	size_t capacity;
+	size_t nr; /* includes nr_data */
+	size_t nr_data;
+	u8 set;
+	struct ovl_fs_context_layer *lower;
+};
+
+int ovl_parse_param_upperdir(const char *name, struct fs_context *fc,
+			     bool workdir);
+int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc);
+void ovl_parse_param_drop_lowerdir(struct ovl_fs_context *ctx);
+
 extern const char *const ovl_xattr_table[][2];
 static inline const char *ovl_xattr(struct ovl_fs *ofs, enum ovl_xattr ox)
 {
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index c72433c06006..7888ab33730b 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -6,7 +6,6 @@
  */
 
 struct ovl_config {
-	char *lowerdir;
 	char *upperdir;
 	char *workdir;
 	bool default_permissions;
@@ -41,6 +40,7 @@ struct ovl_layer {
 	int idx;
 	/* One fsid per unique underlying sb (upper fsid == 0) */
 	int fsid;
+	char *name;
 };
 
 /*
@@ -101,7 +101,6 @@ struct ovl_fs {
 	errseq_t errseq;
 };
 
-
 /* Number of lower layers, not including data-only layers */
 static inline unsigned int ovl_numlowerlayer(struct ovl_fs *ofs)
 {
diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
new file mode 100644
index 000000000000..a1606af1613f
--- /dev/null
+++ b/fs/overlayfs/params.c
@@ -0,0 +1,388 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/fs.h>
+#include <linux/namei.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
+#include <linux/posix_acl_xattr.h>
+#include <linux/xattr.h>
+#include "overlayfs.h"
+
+static ssize_t ovl_parse_param_split_lowerdirs(char *str)
+{
+	ssize_t nr_layers = 1, nr_colons = 0;
+	char *s, *d;
+
+	for (s = d = str;; s++, d++) {
+		if (*s == '\\') {
+			s++;
+		} else if (*s == ':') {
+			bool next_colon = (*(s + 1) == ':');
+
+			nr_colons++;
+			if (nr_colons == 2 && next_colon) {
+				pr_err("only single ':' or double '::' sequences of unescaped colons in lowerdir mount option allowed.\n");
+				return -EINVAL;
+			}
+			/* count layers, not colons */
+			if (!next_colon)
+				nr_layers++;
+
+			*d = '\0';
+			continue;
+		}
+
+		*d = *s;
+		if (!*s) {
+			/* trailing colons */
+			if (nr_colons) {
+				pr_err("unescaped trailing colons in lowerdir mount option.\n");
+				return -EINVAL;
+			}
+			break;
+		}
+		nr_colons = 0;
+	}
+
+	return nr_layers;
+}
+
+static int ovl_mount_dir_noesc(const char *name, struct path *path)
+{
+	int err = -EINVAL;
+
+	if (!*name) {
+		pr_err("empty lowerdir\n");
+		goto out;
+	}
+	err = kern_path(name, LOOKUP_FOLLOW, path);
+	if (err) {
+		pr_err("failed to resolve '%s': %i\n", name, err);
+		goto out;
+	}
+	err = -EINVAL;
+	if (ovl_dentry_weird(path->dentry)) {
+		pr_err("filesystem on '%s' not supported\n", name);
+		goto out_put;
+	}
+	if (!d_is_dir(path->dentry)) {
+		pr_err("'%s' not a directory\n", name);
+		goto out_put;
+	}
+	return 0;
+
+out_put:
+	path_put_init(path);
+out:
+	return err;
+}
+
+static void ovl_unescape(char *s)
+{
+	char *d = s;
+
+	for (;; s++, d++) {
+		if (*s == '\\')
+			s++;
+		*d = *s;
+		if (!*s)
+			break;
+	}
+}
+
+static int ovl_mount_dir(const char *name, struct path *path)
+{
+	int err = -ENOMEM;
+	char *tmp = kstrdup(name, GFP_KERNEL);
+
+	if (tmp) {
+		ovl_unescape(tmp);
+		err = ovl_mount_dir_noesc(tmp, path);
+
+		if (!err && path->dentry->d_flags & DCACHE_OP_REAL) {
+			pr_err("filesystem on '%s' not supported as upperdir\n",
+			       tmp);
+			path_put_init(path);
+			err = -EINVAL;
+		}
+		kfree(tmp);
+	}
+	return err;
+}
+
+int ovl_parse_param_upperdir(const char *name, struct fs_context *fc,
+			     bool workdir)
+{
+	int err;
+	struct ovl_fs *ofs = fc->s_fs_info;
+	struct ovl_config *config = &ofs->config;
+	struct ovl_fs_context *ctx = fc->fs_private;
+	struct path path;
+	char *dup;
+
+	err = ovl_mount_dir(name, &path);
+	if (err)
+		return err;
+
+	/*
+	 * Check whether upper path is read-only here to report failures
+	 * early. Don't forget to recheck when the superblock is created
+	 * as the mount attributes could change.
+	 */
+	if (__mnt_is_readonly(path.mnt)) {
+		path_put(&path);
+		return -EINVAL;
+	}
+
+	dup = kstrdup(name, GFP_KERNEL);
+	if (!dup) {
+		path_put(&path);
+		return -ENOMEM;
+	}
+
+	if (workdir) {
+		kfree(config->workdir);
+		config->workdir = dup;
+		path_put(&ctx->work);
+		ctx->work = path;
+	} else {
+		kfree(config->upperdir);
+		config->upperdir = dup;
+		path_put(&ctx->upper);
+		ctx->upper = path;
+	}
+	return 0;
+}
+
+void ovl_parse_param_drop_lowerdir(struct ovl_fs_context *ctx)
+{
+	for (size_t nr = 0; nr < ctx->nr; nr++) {
+		path_put(&ctx->lower[nr].path);
+		kfree(ctx->lower[nr].name);
+		ctx->lower[nr].name = NULL;
+	}
+	ctx->nr = 0;
+	ctx->nr_data = 0;
+}
+
+/*
+ * Parse lowerdir= mount option:
+ *
+ * (1) lowerdir=/lower1:/lower2:/lower3::/data1::/data2
+ *     Set "/lower1", "/lower2", and "/lower3" as lower layers and
+ *     "/data1" and "/data2" as data lower layers. Any existing lower
+ *     layers are replaced.
+ * (2) lowerdir=:/lower4
+ *     Append "/lower4" to current stack of lower layers. This requires
+ *     that there already is at least one lower layer configured.
+ * (3) lowerdir=::/lower5
+ *     Append data "/lower5" as data lower layer. This requires that
+ *     there's at least one regular lower layer present.
+ */
+int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
+{
+	int err;
+	struct ovl_fs_context *ctx = fc->fs_private;
+	struct ovl_fs_context_layer *l;
+	char *dup = NULL, *dup_iter;
+	ssize_t nr_lower = 0, nr = 0, nr_data = 0;
+	bool append = false, data_layer = false;
+
+	/*
+	 * Ensure we're backwards compatible with mount(2)
+	 * by allowing relative paths.
+	 */
+
+	/* drop all existing lower layers */
+	if (!*name) {
+		ovl_parse_param_drop_lowerdir(ctx);
+		return 0;
+	}
+
+	if (strncmp(name, "::", 2) == 0) {
+		/*
+		 * This is a data layer.
+		 * There must be at least one regular lower layer
+		 * specified.
+		 */
+		if (ctx->nr == 0) {
+			pr_err("data lower layers without regular lower layers not allowed");
+			return -EINVAL;
+		}
+
+		/* Skip the leading "::". */
+		name += 2;
+		data_layer = true;
+		/*
+		 * A data layer is automatically an append as there
+		 * must've been at least one regular lower layer.
+		 */
+		append = true;
+	} else if (*name == ':') {
+		/*
+		 * This is a regular lower layer.
+		 * If users want to append a layer enforce that they
+		 * have already specified a first layer before. It's
+		 * better to be strict.
+		 */
+		if (ctx->nr == 0) {
+			pr_err("cannot append layer if no previous layer has been specified");
+			return -EINVAL;
+		}
+
+		/*
+		 * Once a sequence of data layers has started regular
+		 * lower layers are forbidden.
+		 */
+		if (ctx->nr_data > 0) {
+			pr_err("regular lower layers cannot follow data lower layers");
+			return -EINVAL;
+		}
+
+		/* Skip the leading ":". */
+		name++;
+		append = true;
+	}
+
+	dup = kstrdup(name, GFP_KERNEL);
+	if (!dup)
+		return -ENOMEM;
+
+	err = -EINVAL;
+	nr_lower = ovl_parse_param_split_lowerdirs(dup);
+	if (nr_lower < 0)
+		goto out_err;
+
+	if ((nr_lower > OVL_MAX_STACK) ||
+	    (append && (size_add(ctx->nr, nr_lower) > OVL_MAX_STACK))) {
+		pr_err("too many lower directories, limit is %d\n", OVL_MAX_STACK);
+		goto out_err;
+	}
+
+	if (!append)
+		ovl_parse_param_drop_lowerdir(ctx);
+
+	/*
+	 * (1) append
+	 *
+	 * We want nr <= nr_lower <= capacity We know nr > 0 and nr <=
+	 * capacity. If nr == 0 this wouldn't be append. If nr +
+	 * nr_lower is <= capacity then nr <= nr_lower <= capacity
+	 * already holds. If nr + nr_lower exceeds capacity, we realloc.
+	 *
+	 * (2) replace
+	 *
+	 * Ensure we're backwards compatible with mount(2) which allows
+	 * "lowerdir=/a:/b:/c,lowerdir=/d:/e:/f" causing the last
+	 * specified lowerdir mount option to win.
+	 *
+	 * We want nr <= nr_lower <= capacity We know either (i) nr == 0
+	 * or (ii) nr > 0. We also know nr_lower > 0. The capacity
+	 * could've been changed multiple times already so we only know
+	 * nr <= capacity. If nr + nr_lower > capacity we realloc,
+	 * otherwise nr <= nr_lower <= capacity holds already.
+	 */
+	nr_lower += ctx->nr;
+	if (nr_lower > ctx->capacity) {
+		err = -ENOMEM;
+		l = krealloc_array(ctx->lower, nr_lower, sizeof(*ctx->lower),
+				   GFP_KERNEL_ACCOUNT);
+		if (!l)
+			goto out_err;
+
+		ctx->lower = l;
+		ctx->capacity = nr_lower;
+	}
+
+	/*
+	 *   (3) By (1) and (2) we know nr <= nr_lower <= capacity.
+	 *   (4) If ctx->nr == 0 => replace
+	 *       We have verified above that the lowerdir mount option
+	 *       isn't an append, i.e., the lowerdir mount option
+	 *       doesn't start with ":" or "::".
+	 * (4.1) The lowerdir mount options only contains regular lower
+	 *       layers ":".
+	 *       => Nothing to verify.
+	 * (4.2) The lowerdir mount options contains regular ":" and
+	 *       data "::" layers.
+	 *       => We need to verify that data lower layers "::" aren't
+	 *          followed by regular ":" lower layers
+	 *   (5) If ctx->nr > 0 => append
+	 *       We know that there's at least one regular layer
+	 *       otherwise we would've failed when parsing the previous
+	 *       lowerdir mount option.
+	 * (5.1) The lowerdir mount option is a regular layer ":" append
+	 *       => We need to verify that no data layers have been
+	 *          specified before.
+	 * (5.2) The lowerdir mount option is a data layer "::" append
+	 *       We know that there's at least one regular layer or
+	 *       other data layers. => There's nothing to verify.
+	 */
+	dup_iter = dup;
+	for (nr = ctx->nr; nr < nr_lower; nr++) {
+		l = &ctx->lower[nr];
+
+		err = ovl_mount_dir_noesc(dup_iter, &l->path);
+		if (err)
+			goto out_put;
+
+		err = -ENOMEM;
+		l->name = kstrdup(dup_iter, GFP_KERNEL_ACCOUNT);
+		if (!l->name)
+			goto out_put;
+
+		if (data_layer)
+			nr_data++;
+
+		/* Calling strchr() again would overrun. */
+		if ((nr + 1) == nr_lower)
+			break;
+
+		err = -EINVAL;
+		dup_iter = strchr(dup_iter, '\0') + 1;
+		if (*dup_iter) {
+			/*
+			 * This is a regular layer so we require that
+			 * there are no data layers.
+			 */
+			if ((ctx->nr_data + nr_data) > 0) {
+				pr_err("regular lower layers cannot follow data lower layers");
+				goto out_put;
+			}
+
+			data_layer = false;
+			continue;
+		}
+
+		/* This is a data lower layer. */
+		data_layer = true;
+		dup_iter++;
+	}
+	ctx->nr = nr_lower;
+	ctx->nr_data += nr_data;
+	kfree(dup);
+	return 0;
+
+out_put:
+	/*
+	 * We know nr >= ctx->nr < nr_lower. If we failed somewhere
+	 * we want to undo until nr == ctx->nr. This is correct for
+	 * both ctx->nr == 0 and ctx->nr > 0.
+	 */
+	for (; nr >= ctx->nr; nr--) {
+		l = &ctx->lower[nr];
+		kfree(l->name);
+		l->name = NULL;
+		path_put(&l->path);
+
+		/* don't overflow */
+		if (nr == 0)
+			break;
+	}
+
+out_err:
+	kfree(dup);
+
+	/* Intentionally don't realloc to a smaller size. */
+	return err;
+}
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 3392dc5d2082..b73b14c52961 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -27,8 +27,6 @@ MODULE_LICENSE("GPL");
 
 struct ovl_dir_cache;
 
-#define OVL_MAX_STACK 500
-
 static bool ovl_redirect_dir_def = IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_DIR);
 module_param_named(redirect_dir, ovl_redirect_dir_def, bool, 0644);
 MODULE_PARM_DESC(redirect_dir,
@@ -109,8 +107,11 @@ static const char *ovl_redirect_mode(struct ovl_config *config)
 	return ovl_parameter_redirect_dir[config->redirect_mode].name;
 }
 
+#define fsparam_string_empty(NAME, OPT) \
+	__fsparam(fs_param_is_string, NAME, OPT, fs_param_can_be_empty, NULL)
+
 static const struct fs_parameter_spec ovl_parameter_spec[] = {
-	fsparam_string("lowerdir",          Opt_lowerdir),
+	fsparam_string_empty("lowerdir",    Opt_lowerdir),
 	fsparam_string("upperdir",          Opt_upperdir),
 	fsparam_string("workdir",           Opt_workdir),
 	fsparam_flag("default_permissions", Opt_default_permissions),
@@ -125,15 +126,15 @@ static const struct fs_parameter_spec ovl_parameter_spec[] = {
 	{}
 };
 
+/*
+ * These options imply different behavior when they are explicitly
+ * specified than when they are left in their default state.
+ */
 #define OVL_METACOPY_SET	BIT(0)
 #define OVL_REDIRECT_SET	BIT(1)
 #define OVL_NFS_EXPORT_SET	BIT(2)
 #define OVL_INDEX_SET		BIT(3)
 
-struct ovl_fs_context {
-	u8 set;
-};
-
 static struct dentry *ovl_d_real(struct dentry *dentry,
 				 const struct inode *inode)
 {
@@ -308,6 +309,7 @@ static void ovl_free_fs(struct ovl_fs *ofs)
 	for (i = 0; i < ofs->numlayer; i++) {
 		iput(ofs->layers[i].trap);
 		mounts[i] = ofs->layers[i].mnt;
+		kfree(ofs->layers[i].name);
 	}
 	kern_unmount_array(mounts, ofs->numlayer);
 	kfree(ofs->layers);
@@ -315,7 +317,6 @@ static void ovl_free_fs(struct ovl_fs *ofs)
 		free_anon_bdev(ofs->fs[i].pseudo_dev);
 	kfree(ofs->fs);
 
-	kfree(ofs->config.lowerdir);
 	kfree(ofs->config.upperdir);
 	kfree(ofs->config.workdir);
 	if (ofs->creator_cred)
@@ -433,8 +434,17 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 	struct super_block *sb = dentry->d_sb;
 	struct ovl_fs *ofs = sb->s_fs_info;
 	const char *redirect_mode;
-
-	seq_show_option(m, "lowerdir", ofs->config.lowerdir);
+	size_t nr, nr_merged_lower = ofs->numlayer - ofs->numdatalayer;
+	const struct ovl_layer *data_layers = &ofs->layers[nr_merged_lower];
+
+	/* ofs->layers[0] is the upper layer */
+	seq_printf(m, ",lowerdir=%s", ofs->layers[1].name);
+	/* dump regular lower layers */
+	for (nr = 2; nr < nr_merged_lower; nr++)
+		seq_printf(m, ":%s", ofs->layers[nr].name);
+	/* dump data lower layers */
+	for (nr = 0; nr < ofs->numdatalayer; nr++)
+		seq_printf(m, "::%s", data_layers[nr].name);
 	if (ofs->config.upperdir) {
 		seq_show_option(m, "upperdir", ofs->config.upperdir);
 		seq_show_option(m, "workdir", ofs->config.workdir);
@@ -509,6 +519,11 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 	bool nfs_export_opt = ctx->set & OVL_NFS_EXPORT_SET;
 	bool index_opt = ctx->set & OVL_INDEX_SET;
 
+	if (ctx->nr_data > 0 && !config->metacopy) {
+		pr_err("lower data-only dirs require metacopy support.\n");
+		return -EINVAL;
+	}
+
 	/* Workdir/index are useless in non-upper mount */
 	if (!config->upperdir) {
 		if (config->workdir) {
@@ -723,69 +738,6 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs,
 	goto out_unlock;
 }
 
-static void ovl_unescape(char *s)
-{
-	char *d = s;
-
-	for (;; s++, d++) {
-		if (*s == '\\')
-			s++;
-		*d = *s;
-		if (!*s)
-			break;
-	}
-}
-
-static int ovl_mount_dir_noesc(const char *name, struct path *path)
-{
-	int err = -EINVAL;
-
-	if (!*name) {
-		pr_err("empty lowerdir\n");
-		goto out;
-	}
-	err = kern_path(name, LOOKUP_FOLLOW, path);
-	if (err) {
-		pr_err("failed to resolve '%s': %i\n", name, err);
-		goto out;
-	}
-	err = -EINVAL;
-	if (ovl_dentry_weird(path->dentry)) {
-		pr_err("filesystem on '%s' not supported\n", name);
-		goto out_put;
-	}
-	if (!d_is_dir(path->dentry)) {
-		pr_err("'%s' not a directory\n", name);
-		goto out_put;
-	}
-	return 0;
-
-out_put:
-	path_put_init(path);
-out:
-	return err;
-}
-
-static int ovl_mount_dir(const char *name, struct path *path)
-{
-	int err = -ENOMEM;
-	char *tmp = kstrdup(name, GFP_KERNEL);
-
-	if (tmp) {
-		ovl_unescape(tmp);
-		err = ovl_mount_dir_noesc(tmp, path);
-
-		if (!err && path->dentry->d_flags & DCACHE_OP_REAL) {
-			pr_err("filesystem on '%s' not supported as upperdir\n",
-			       tmp);
-			path_put_init(path);
-			err = -EINVAL;
-		}
-		kfree(tmp);
-	}
-	return err;
-}
-
 static int ovl_check_namelen(const struct path *path, struct ovl_fs *ofs,
 			     const char *name)
 {
@@ -806,10 +758,6 @@ static int ovl_lower_dir(const char *name, struct path *path,
 	int fh_type;
 	int err;
 
-	err = ovl_mount_dir_noesc(name, path);
-	if (err)
-		return err;
-
 	err = ovl_check_namelen(path, ofs, name);
 	if (err)
 		return err;
@@ -858,26 +806,6 @@ static bool ovl_workdir_ok(struct dentry *workdir, struct dentry *upperdir)
 	return ok;
 }
 
-static unsigned int ovl_split_lowerdirs(char *str)
-{
-	unsigned int ctr = 1;
-	char *s, *d;
-
-	for (s = d = str;; s++, d++) {
-		if (*s == '\\') {
-			s++;
-		} else if (*s == ':') {
-			*d = '\0';
-			ctr++;
-			continue;
-		}
-		*d = *s;
-		if (!*s)
-			break;
-	}
-	return ctr;
-}
-
 static int ovl_own_xattr_get(const struct xattr_handler *handler,
 			     struct dentry *dentry, struct inode *inode,
 			     const char *name, void *buffer, size_t size)
@@ -978,15 +906,12 @@ static int ovl_report_in_use(struct ovl_fs *ofs, const char *name)
 }
 
 static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
-			 struct ovl_layer *upper_layer, struct path *upperpath)
+			 struct ovl_layer *upper_layer,
+			 const struct path *upperpath)
 {
 	struct vfsmount *upper_mnt;
 	int err;
 
-	err = ovl_mount_dir(ofs->config.upperdir, upperpath);
-	if (err)
-		goto out;
-
 	/* Upperdir path should not be r/o */
 	if (__mnt_is_readonly(upperpath->mnt)) {
 		pr_err("upper fs is r/o, try multi-lower layers mount\n");
@@ -1016,6 +941,11 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
 	upper_layer->idx = 0;
 	upper_layer->fsid = 0;
 
+	err = -ENOMEM;
+	upper_layer->name = kstrdup(ofs->config.upperdir, GFP_KERNEL);
+	if (!upper_layer->name)
+		goto out;
+
 	/*
 	 * Inherit SB_NOSEC flag from upperdir.
 	 *
@@ -1273,46 +1203,37 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
 }
 
 static int ovl_get_workdir(struct super_block *sb, struct ovl_fs *ofs,
-			   const struct path *upperpath)
+			   const struct path *upperpath,
+			   const struct path *workpath)
 {
 	int err;
-	struct path workpath = { };
-
-	err = ovl_mount_dir(ofs->config.workdir, &workpath);
-	if (err)
-		goto out;
 
 	err = -EINVAL;
-	if (upperpath->mnt != workpath.mnt) {
+	if (upperpath->mnt != workpath->mnt) {
 		pr_err("workdir and upperdir must reside under the same mount\n");
-		goto out;
+		return err;
 	}
-	if (!ovl_workdir_ok(workpath.dentry, upperpath->dentry)) {
+	if (!ovl_workdir_ok(workpath->dentry, upperpath->dentry)) {
 		pr_err("workdir and upperdir must be separate subtrees\n");
-		goto out;
+		return err;
 	}
 
-	ofs->workbasedir = dget(workpath.dentry);
+	ofs->workbasedir = dget(workpath->dentry);
 
 	if (ovl_inuse_trylock(ofs->workbasedir)) {
 		ofs->workdir_locked = true;
 	} else {
 		err = ovl_report_in_use(ofs, "workdir");
 		if (err)
-			goto out;
+			return err;
 	}
 
 	err = ovl_setup_trap(sb, ofs->workbasedir, &ofs->workbasedir_trap,
 			     "workdir");
 	if (err)
-		goto out;
-
-	err = ovl_make_workdir(sb, ofs, &workpath);
-
-out:
-	path_put(&workpath);
+		return err;
 
-	return err;
+	return ovl_make_workdir(sb, ofs, workpath);
 }
 
 static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
@@ -1476,13 +1397,13 @@ static int ovl_get_data_fsid(struct ovl_fs *ofs)
 
 
 static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
-			  struct path *stack, unsigned int numlower,
-			  struct ovl_layer *layers)
+			  struct ovl_fs_context *ctx, struct ovl_layer *layers)
 {
 	int err;
 	unsigned int i;
+	size_t nr_merged_lower;
 
-	ofs->fs = kcalloc(numlower + 2, sizeof(struct ovl_sb), GFP_KERNEL);
+	ofs->fs = kcalloc(ctx->nr + 2, sizeof(struct ovl_sb), GFP_KERNEL);
 	if (ofs->fs == NULL)
 		return -ENOMEM;
 
@@ -1509,13 +1430,15 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 		ofs->fs[0].is_lower = false;
 	}
 
-	for (i = 0; i < numlower; i++) {
+	nr_merged_lower = ctx->nr - ctx->nr_data;
+	for (i = 0; i < ctx->nr; i++) {
+		struct ovl_fs_context_layer *l = &ctx->lower[i];
 		struct vfsmount *mnt;
 		struct inode *trap;
 		int fsid;
 
-		if (i < numlower - ofs->numdatalayer)
-			fsid = ovl_get_fsid(ofs, &stack[i]);
+		if (i < nr_merged_lower)
+			fsid = ovl_get_fsid(ofs, &l->path);
 		else
 			fsid = ovl_get_data_fsid(ofs);
 		if (fsid < 0)
@@ -1528,11 +1451,11 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 		 * the upperdir/workdir is in fact in-use by our
 		 * upperdir/workdir.
 		 */
-		err = ovl_setup_trap(sb, stack[i].dentry, &trap, "lowerdir");
+		err = ovl_setup_trap(sb, l->path.dentry, &trap, "lowerdir");
 		if (err)
 			return err;
 
-		if (ovl_is_inuse(stack[i].dentry)) {
+		if (ovl_is_inuse(l->path.dentry)) {
 			err = ovl_report_in_use(ofs, "lowerdir");
 			if (err) {
 				iput(trap);
@@ -1540,7 +1463,7 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 			}
 		}
 
-		mnt = clone_private_mount(&stack[i]);
+		mnt = clone_private_mount(&l->path);
 		err = PTR_ERR(mnt);
 		if (IS_ERR(mnt)) {
 			pr_err("failed to clone lowerpath\n");
@@ -1559,6 +1482,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 		layers[ofs->numlayer].idx = ofs->numlayer;
 		layers[ofs->numlayer].fsid = fsid;
 		layers[ofs->numlayer].fs = &ofs->fs[fsid];
+		layers[ofs->numlayer].name = l->name;
+		l->name = NULL;
 		ofs->numlayer++;
 		ofs->fs[fsid].is_lower = true;
 	}
@@ -1599,104 +1524,59 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 }
 
 static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb,
-				const char *lower, unsigned int numlower,
-				struct ovl_fs *ofs, struct ovl_layer *layers)
+					    struct ovl_fs_context *ctx,
+					    struct ovl_fs *ofs,
+					    struct ovl_layer *layers)
 {
 	int err;
-	struct path *stack = NULL;
-	struct ovl_path *lowerstack;
-	unsigned int numlowerdata = 0;
 	unsigned int i;
+	size_t nr_merged_lower;
 	struct ovl_entry *oe;
+	struct ovl_path *lowerstack;
 
-	if (!ofs->config.upperdir && numlower == 1) {
+	struct ovl_fs_context_layer *l;
+
+	if (!ofs->config.upperdir && ctx->nr == 1) {
 		pr_err("at least 2 lowerdir are needed while upperdir nonexistent\n");
 		return ERR_PTR(-EINVAL);
 	}
 
-	stack = kcalloc(numlower, sizeof(struct path), GFP_KERNEL);
-	if (!stack)
-		return ERR_PTR(-ENOMEM);
+	err = -EINVAL;
+	for (i = 0; i < ctx->nr; i++) {
+		l = &ctx->lower[i];
 
-	for (i = 0; i < numlower;) {
-		err = ovl_lower_dir(lower, &stack[i], ofs, &sb->s_stack_depth);
+		err = ovl_lower_dir(l->name, &l->path, ofs, &sb->s_stack_depth);
 		if (err)
-			goto out_err;
-
-		lower = strchr(lower, '\0') + 1;
-
-		i++;
-		if (i == numlower)
-			break;
-
-		err = -EINVAL;
-		/*
-		 * Empty lower layer path could mean :: separator that indicates
-		 * a data-only lower data.
-		 * Several data-only layers are allowed, but they all need to be
-		 * at the bottom of the stack.
-		 */
-		if (*lower) {
-			/* normal lower dir */
-			if (numlowerdata) {
-				pr_err("lower data-only dirs must be at the bottom of the stack.\n");
-				goto out_err;
-			}
-		} else {
-			/* data-only lower dir */
-			if (!ofs->config.metacopy) {
-				pr_err("lower data-only dirs require metacopy support.\n");
-				goto out_err;
-			}
-			if (i == numlower - 1) {
-				pr_err("lowerdir argument must not end with double colon.\n");
-				goto out_err;
-			}
-			lower++;
-			numlower--;
-			numlowerdata++;
-		}
-	}
-
-	if (numlowerdata) {
-		ofs->numdatalayer = numlowerdata;
-		pr_info("using the lowest %d of %d lowerdirs as data layers\n",
-			numlowerdata, numlower);
+			return ERR_PTR(err);
 	}
 
 	err = -EINVAL;
 	sb->s_stack_depth++;
 	if (sb->s_stack_depth > FILESYSTEM_MAX_STACK_DEPTH) {
 		pr_err("maximum fs stacking depth exceeded\n");
-		goto out_err;
+		return ERR_PTR(err);
 	}
 
-	err = ovl_get_layers(sb, ofs, stack, numlower, layers);
+	err = ovl_get_layers(sb, ofs, ctx, layers);
 	if (err)
-		goto out_err;
+		return ERR_PTR(err);
 
 	err = -ENOMEM;
 	/* Data-only layers are not merged in root directory */
-	oe = ovl_alloc_entry(numlower - numlowerdata);
+	nr_merged_lower = ctx->nr - ctx->nr_data;
+	oe = ovl_alloc_entry(nr_merged_lower);
 	if (!oe)
-		goto out_err;
+		return ERR_PTR(err);
 
 	lowerstack = ovl_lowerstack(oe);
-	for (i = 0; i < numlower - numlowerdata; i++) {
-		lowerstack[i].dentry = dget(stack[i].dentry);
-		lowerstack[i].layer = &ofs->layers[i+1];
+	for (i = 0; i < nr_merged_lower; i++) {
+		l = &ctx->lower[i];
+		lowerstack[i].dentry = dget(l->path.dentry);
+		lowerstack[i].layer = &ofs->layers[i + 1];
 	}
-
-out:
-	for (i = 0; i < numlower; i++)
-		path_put(&stack[i]);
-	kfree(stack);
+	ofs->numdatalayer = ctx->nr_data;
 
 	return oe;
-
-out_err:
-	oe = ERR_PTR(err);
-	goto out;
 }
 
 /*
@@ -1804,6 +1684,12 @@ static struct dentry *ovl_get_root(struct super_block *sb,
 	ovl_set_upperdata(d_inode(root));
 	ovl_inode_init(d_inode(root), &oip, ino, fsid);
 	ovl_dentry_init_flags(root, upperdentry, oe, DCACHE_OP_WEAK_REVALIDATE);
+	/*
+	 * We're going to put upper path when we call
+	 * fs_context_operations->free() take an additional
+	 * reference so we can just call path_put().
+	 */
+	dget(upperdentry);
 
 	return root;
 }
@@ -1815,7 +1701,6 @@ static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	struct ovl_fs *ofs = fc->s_fs_info;
 	struct ovl_config *config = &ofs->config;
 	struct ovl_fs_context *ctx = fc->fs_private;
-	char *dup;
 	int opt;
 
 	/*
@@ -1832,34 +1717,13 @@ static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
 
 	switch (opt) {
 	case Opt_lowerdir:
-		dup = kstrdup(param->string, GFP_KERNEL);
-		if (!dup) {
-			err = -ENOMEM;
-			break;
-		}
-
-		kfree(config->lowerdir);
-		config->lowerdir = dup;
+		err = ovl_parse_param_lowerdir(param->string, fc);
 		break;
 	case Opt_upperdir:
-		dup = kstrdup(param->string, GFP_KERNEL);
-		if (!dup) {
-			err = -ENOMEM;
-			break;
-		}
-
-		kfree(config->upperdir);
-		config->upperdir = dup;
-		break;
+		fallthrough;
 	case Opt_workdir:
-		dup = kstrdup(param->string, GFP_KERNEL);
-		if (!dup) {
-			err = -ENOMEM;
-			break;
-		}
-
-		kfree(config->workdir);
-		config->workdir = dup;
+		err = ovl_parse_param_upperdir(param->string, fc,
+					       (Opt_workdir == opt));
 		break;
 	case Opt_default_permissions:
 		config->default_permissions = true;
@@ -1927,13 +1791,10 @@ static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct ovl_fs *ofs = sb->s_fs_info;
 	struct ovl_fs_context *ctx = fc->fs_private;
-	struct path upperpath = {};
 	struct dentry *root_dentry;
 	struct ovl_entry *oe;
 	struct ovl_layer *layers;
 	struct cred *cred;
-	char *splitlower = NULL;
-	unsigned int numlower;
 	int err;
 
 	err = -EIO;
@@ -1952,28 +1813,20 @@ static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (err)
 		goto out_err;
 
+	/*
+	 * Check ctx->nr instead of ofs->config.lowerdir since we're
+	 * going to set ofs->config.lowerdir here after we know that the
+	 * user specified all layers.
+	 */
 	err = -EINVAL;
-	if (!ofs->config.lowerdir) {
+	if (ctx->nr == 0) {
 		if (fc->sb_flags & SB_SILENT)
 			pr_err("missing 'lowerdir'\n");
 		goto out_err;
 	}
 
 	err = -ENOMEM;
-	splitlower = kstrdup(ofs->config.lowerdir, GFP_KERNEL);
-	if (!splitlower)
-		goto out_err;
-
-	err = -EINVAL;
-	numlower = ovl_split_lowerdirs(splitlower);
-	if (numlower > OVL_MAX_STACK) {
-		pr_err("too many lower directories, limit is %d\n",
-		       OVL_MAX_STACK);
-		goto out_err;
-	}
-
-	err = -ENOMEM;
-	layers = kcalloc(numlower + 1, sizeof(struct ovl_layer), GFP_KERNEL);
+	layers = kcalloc(ctx->nr + 1, sizeof(struct ovl_layer), GFP_KERNEL);
 	if (!layers)
 		goto out_err;
 
@@ -2005,7 +1858,7 @@ static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 			goto out_err;
 		}
 
-		err = ovl_get_upper(sb, ofs, &layers[0], &upperpath);
+		err = ovl_get_upper(sb, ofs, &layers[0], &ctx->upper);
 		if (err)
 			goto out_err;
 
@@ -2019,7 +1872,7 @@ static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 			}
 		}
 
-		err = ovl_get_workdir(sb, ofs, &upperpath);
+		err = ovl_get_workdir(sb, ofs, &ctx->upper, &ctx->work);
 		if (err)
 			goto out_err;
 
@@ -2029,7 +1882,7 @@ static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 		sb->s_stack_depth = upper_sb->s_stack_depth;
 		sb->s_time_gran = upper_sb->s_time_gran;
 	}
-	oe = ovl_get_lowerstack(sb, splitlower, numlower, ofs, layers);
+	oe = ovl_get_lowerstack(sb, ctx, ofs, layers);
 	err = PTR_ERR(oe);
 	if (IS_ERR(oe))
 		goto out_err;
@@ -2044,7 +1897,7 @@ static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 	}
 
 	if (!ovl_force_readonly(ofs) && ofs->config.index) {
-		err = ovl_get_indexdir(sb, ofs, oe, &upperpath);
+		err = ovl_get_indexdir(sb, ofs, oe, &ctx->upper);
 		if (err)
 			goto out_free_oe;
 
@@ -2085,13 +1938,10 @@ static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_iflags |= SB_I_SKIP_SYNC;
 
 	err = -ENOMEM;
-	root_dentry = ovl_get_root(sb, upperpath.dentry, oe);
+	root_dentry = ovl_get_root(sb, ctx->upper.dentry, oe);
 	if (!root_dentry)
 		goto out_free_oe;
 
-	mntput(upperpath.mnt);
-	kfree(splitlower);
-
 	sb->s_root = root_dentry;
 
 	return 0;
@@ -2109,6 +1959,10 @@ static int ovl_get_tree(struct fs_context *fc)
 
 static inline void ovl_fs_context_free(struct ovl_fs_context *ctx)
 {
+	ovl_parse_param_drop_lowerdir(ctx);
+	path_put(&ctx->upper);
+	path_put(&ctx->work);
+	kfree(ctx->lower);
 	kfree(ctx);
 }
 
@@ -2153,11 +2007,18 @@ static int ovl_init_fs_context(struct fs_context *fc)
 	if (!ctx)
 		return -ENOMEM;
 
+	/*
+	 * By default we allocate for three lower layers. It's likely
+	 * that it'll cover most users.
+	 */
+	ctx->lower = kmalloc_array(3, sizeof(*ctx->lower), GFP_KERNEL_ACCOUNT);
+	if (!ctx->lower)
+		goto out_err;
+	ctx->capacity = 3;
+
 	ofs = kzalloc(sizeof(struct ovl_fs), GFP_KERNEL);
-	if (!ofs) {
-		ovl_fs_context_free(ctx);
-		return -ENOMEM;
-	}
+	if (!ofs)
+		goto out_err;
 
 	if (strcmp(ovl_redirect_mode_def(), "on") == 0)
 		ofs->config.redirect_mode = OVL_REDIRECT_DIR_ON;
@@ -2174,6 +2035,11 @@ static int ovl_init_fs_context(struct fs_context *fc)
 	fc->fs_private		= ctx;
 	fc->ops			= &ovl_context_ops;
 	return 0;
+
+out_err:
+	ovl_fs_context_free(ctx);
+	return -ENOMEM;
+
 }
 
 static struct file_system_type ovl_fs_type = {

-- 
2.34.1

Re: [PATCH v3 3/3] ovl: change layer mount option handling

[Amir Goldstein]

On Tue, Jun 13, 2023 at 5:49 PM Christian Brauner <brauner@kernel.org> wrote:
>
> We ran into issues where mount(8) passed multiple lower layers as one
> big string through fsconfig(). But the fsconfig() FSCONFIG_SET_STRING
> option is limited to 256 bytes in strndup_user(). While this would be
> fixable by extending the fsconfig() buffer I'd rather encourage users to
> append layers via multiple fsconfig() calls as the interface allows
> nicely for this. This has also been requested as a feature before.
>
> With this port to the new mount api the following will be possible:
>
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir", "/lower1", 0);
>
>         /* set upper layer */
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "upperdir", "/upper", 0);
>
>         /* append "/lower2", "/lower3", and "/lower4" */
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir", ":/lower2:/lower3:/lower4", 0);
>
>         /* turn index feature on */
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "index", "on", 0);
>
>         /* append "/lower5" */
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir", ":/lower5", 0);
>
> Specifying ':' would have been rejected so this isn't a regression. And
> we can't simply use "lowerdir=/lower" to append on top of existing
> layers as "lowerdir=/lower,lowerdir=/other-lower" would make
> "/other-lower" the only lower layer so we'd break uapi if we changed
> this. So the ':' prefix seems a good compromise.
>
> Users can choose to specify multiple layers at once or individual
> layers. A layer is appended if it starts with ":". This requires that
> the user has already added at least one layer before. If lowerdir is
> specified again without a leading ":" then all previous layers are
> dropped and replaced with the new layers. If lowerdir is specified and
> empty than all layers are simply dropped.
>
> An additional change is that overlayfs will now parse and resolve layers
> right when they are specified in fsconfig() instead of deferring until
> super block creation. This allows users to receive early errors.
>
> It also allows users to actually use up to 500 layers something which
> was theoretically possible but ended up not working due to the mount
> option string passed via mount(2) being too large.
>
> This also allows a more privileged process to set config options for a
> lesser privileged process as the creds for fsconfig() and the creds for
> fsopen() can differ. We could restrict that they match by enforcing that
> the creds of fsopen() and fsconfig() match but I don't see why that
> needs to be the case and allows for a good delegation mechanism.
>
> Plus, in the future it means we're able to extend overlayfs mount
> options and allow users to specify layers via file descriptors instead
> of paths:
>
>         fsconfig(FSCONFIG_SET_PATH{_EMPTY}, "lowerdir", "lower1", dirfd);
>
>         /* append */
>         fsconfig(FSCONFIG_SET_PATH{_EMPTY}, "lowerdir", "lower2", dirfd);
>
>         /* append */
>         fsconfig(FSCONFIG_SET_PATH{_EMPTY}, "lowerdir", "lower3", dirfd);
>
>         /* clear all layers specified until now */
>         fsconfig(FSCONFIG_SET_STRING, "lowerdir", NULL, 0);
>
> This would be especially nice if users create an overlayfs mount on top
> of idmapped layers or just in general private mounts created via
> open_tree(OPEN_TREE_CLONE). Those mounts would then never have to appear
> anywhere in the filesystem. But for now just do the minimal thing.
>
> We should probably aim to move more validation into ovl_fs_parse_param()
> so users get errors before fsconfig(FSCONFIG_CMD_CREATE). But that can
> be done in additional patches later.
>
> This is now also rebased on top of the lazy lowerdata lookup which
> allows the specificatin of data only layers using the new "::" syntax.
>
> The rules are simple. A data only layers cannot be followed by any
> regular layers and data layers must be preceeded by at least one regular
> layer.
>
> Parsing the lowerdir mount option must change because of this. The
> original patchset used the old lowerdir parsing function to split a
> lowerdir mount option string such as:
>
>         lowerdir=/lower1:/lower2::/lower3::/lower4
>
> simply replacing each non-escaped ":" by "\0". So sequences of
> non-escaped ":" were counted as layers. For example, the previous
> lowerdir mount option above would've counted 6 layers instead of 4 and a
> lowerdir mount option such as:
>
>         lowerdir="/lower1:/lower2::/lower3::/lower4:::::::::::::::::::::::::::"
>
> would be counted as 33 layers. Other than being ugly this didn't matter
> much because kern_path() would reject the first "\0" layer. However,
> this overcounting of layers becomes problematic when we base allocations
> on it where we very much only want to allocate space for 4 layers
> instead of 33.
>
> So the new parsing function rejects non-escaped sequences of colons
> other than ":" and "::" immediately instead of relying on kern_path().
>
> Link: https://github.com/util-linux/util-linux/issues/2287
> Link: https://github.com/util-linux/util-linux/issues/1992
> Link: https://bugs.archlinux.org/task/78702
> Link: https://lore.kernel.org/linux-unionfs/20230530-klagen-zudem-32c0908c2108@brauner
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/overlayfs/Makefile    |   2 +-
>  fs/overlayfs/overlayfs.h |  23 +++
>  fs/overlayfs/ovl_entry.h |   3 +-
>  fs/overlayfs/params.c    | 388 +++++++++++++++++++++++++++++++++++++++++++++++
>  fs/overlayfs/super.c     | 376 +++++++++++++++------------------------------
>  5 files changed, 534 insertions(+), 258 deletions(-)
>
> diff --git a/fs/overlayfs/Makefile b/fs/overlayfs/Makefile
> index 9164c585eb2f..4e173d56b11f 100644
> --- a/fs/overlayfs/Makefile
> +++ b/fs/overlayfs/Makefile
> @@ -6,4 +6,4 @@
>  obj-$(CONFIG_OVERLAY_FS) += overlay.o
>
>  overlay-objs := super.o namei.o util.o inode.o file.o dir.o readdir.o \
> -               copy_up.o export.o
> +               copy_up.o export.o params.o
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index fcac4e2c56ab..7659ea6e02cb 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -119,6 +119,29 @@ struct ovl_fh {
>  #define OVL_FH_FID_OFFSET      (OVL_FH_WIRE_OFFSET + \
>                                  offsetof(struct ovl_fb, fid))
>
> +/* params.c */
> +#define OVL_MAX_STACK 500
> +
> +struct ovl_fs_context_layer {
> +       char *name;
> +       struct path path;
> +};
> +
> +struct ovl_fs_context {
> +       struct path upper;
> +       struct path work;
> +       size_t capacity;
> +       size_t nr; /* includes nr_data */
> +       size_t nr_data;
> +       u8 set;
> +       struct ovl_fs_context_layer *lower;
> +};
> +
> +int ovl_parse_param_upperdir(const char *name, struct fs_context *fc,
> +                            bool workdir);
> +int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc);
> +void ovl_parse_param_drop_lowerdir(struct ovl_fs_context *ctx);
> +
>  extern const char *const ovl_xattr_table[][2];
>  static inline const char *ovl_xattr(struct ovl_fs *ofs, enum ovl_xattr ox)
>  {
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index c72433c06006..7888ab33730b 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -6,7 +6,6 @@
>   */
>
>  struct ovl_config {
> -       char *lowerdir;
>         char *upperdir;
>         char *workdir;
>         bool default_permissions;
> @@ -41,6 +40,7 @@ struct ovl_layer {
>         int idx;
>         /* One fsid per unique underlying sb (upper fsid == 0) */
>         int fsid;
> +       char *name;
>  };
>
>  /*
> @@ -101,7 +101,6 @@ struct ovl_fs {
>         errseq_t errseq;
>  };
>
> -
>  /* Number of lower layers, not including data-only layers */
>  static inline unsigned int ovl_numlowerlayer(struct ovl_fs *ofs)
>  {
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> new file mode 100644
> index 000000000000..a1606af1613f
> --- /dev/null
> +++ b/fs/overlayfs/params.c
> @@ -0,0 +1,388 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/fs.h>
> +#include <linux/namei.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
> +#include <linux/posix_acl_xattr.h>
> +#include <linux/xattr.h>
> +#include "overlayfs.h"
> +
> +static ssize_t ovl_parse_param_split_lowerdirs(char *str)
> +{
> +       ssize_t nr_layers = 1, nr_colons = 0;
> +       char *s, *d;
> +
> +       for (s = d = str;; s++, d++) {
> +               if (*s == '\\') {
> +                       s++;
> +               } else if (*s == ':') {
> +                       bool next_colon = (*(s + 1) == ':');
> +
> +                       nr_colons++;
> +                       if (nr_colons == 2 && next_colon) {
> +                               pr_err("only single ':' or double '::' sequences of unescaped colons in lowerdir mount option allowed.\n");
> +                               return -EINVAL;
> +                       }
> +                       /* count layers, not colons */
> +                       if (!next_colon)
> +                               nr_layers++;
> +
> +                       *d = '\0';
> +                       continue;
> +               }
> +
> +               *d = *s;
> +               if (!*s) {
> +                       /* trailing colons */
> +                       if (nr_colons) {
> +                               pr_err("unescaped trailing colons in lowerdir mount option.\n");
> +                               return -EINVAL;
> +                       }
> +                       break;
> +               }
> +               nr_colons = 0;
> +       }
> +
> +       return nr_layers;
> +}
> +
> +static int ovl_mount_dir_noesc(const char *name, struct path *path)
> +{
> +       int err = -EINVAL;
> +
> +       if (!*name) {
> +               pr_err("empty lowerdir\n");
> +               goto out;
> +       }
> +       err = kern_path(name, LOOKUP_FOLLOW, path);
> +       if (err) {
> +               pr_err("failed to resolve '%s': %i\n", name, err);
> +               goto out;
> +       }
> +       err = -EINVAL;
> +       if (ovl_dentry_weird(path->dentry)) {
> +               pr_err("filesystem on '%s' not supported\n", name);
> +               goto out_put;
> +       }
> +       if (!d_is_dir(path->dentry)) {
> +               pr_err("'%s' not a directory\n", name);
> +               goto out_put;
> +       }
> +       return 0;
> +
> +out_put:
> +       path_put_init(path);
> +out:
> +       return err;
> +}
> +
> +static void ovl_unescape(char *s)
> +{
> +       char *d = s;
> +
> +       for (;; s++, d++) {
> +               if (*s == '\\')
> +                       s++;
> +               *d = *s;
> +               if (!*s)
> +                       break;
> +       }
> +}
> +
> +static int ovl_mount_dir(const char *name, struct path *path)
> +{
> +       int err = -ENOMEM;
> +       char *tmp = kstrdup(name, GFP_KERNEL);
> +
> +       if (tmp) {
> +               ovl_unescape(tmp);
> +               err = ovl_mount_dir_noesc(tmp, path);
> +
> +               if (!err && path->dentry->d_flags & DCACHE_OP_REAL) {
> +                       pr_err("filesystem on '%s' not supported as upperdir\n",
> +                              tmp);
> +                       path_put_init(path);
> +                       err = -EINVAL;
> +               }
> +               kfree(tmp);
> +       }
> +       return err;
> +}
> +
> +int ovl_parse_param_upperdir(const char *name, struct fs_context *fc,
> +                            bool workdir)
> +{
> +       int err;
> +       struct ovl_fs *ofs = fc->s_fs_info;
> +       struct ovl_config *config = &ofs->config;
> +       struct ovl_fs_context *ctx = fc->fs_private;
> +       struct path path;
> +       char *dup;
> +
> +       err = ovl_mount_dir(name, &path);
> +       if (err)
> +               return err;
> +
> +       /*
> +        * Check whether upper path is read-only here to report failures
> +        * early. Don't forget to recheck when the superblock is created
> +        * as the mount attributes could change.
> +        */
> +       if (__mnt_is_readonly(path.mnt)) {
> +               path_put(&path);
> +               return -EINVAL;
> +       }
> +
> +       dup = kstrdup(name, GFP_KERNEL);
> +       if (!dup) {
> +               path_put(&path);
> +               return -ENOMEM;
> +       }
> +
> +       if (workdir) {
> +               kfree(config->workdir);
> +               config->workdir = dup;
> +               path_put(&ctx->work);
> +               ctx->work = path;
> +       } else {
> +               kfree(config->upperdir);
> +               config->upperdir = dup;
> +               path_put(&ctx->upper);
> +               ctx->upper = path;
> +       }
> +       return 0;
> +}
> +
> +void ovl_parse_param_drop_lowerdir(struct ovl_fs_context *ctx)
> +{
> +       for (size_t nr = 0; nr < ctx->nr; nr++) {
> +               path_put(&ctx->lower[nr].path);
> +               kfree(ctx->lower[nr].name);
> +               ctx->lower[nr].name = NULL;
> +       }
> +       ctx->nr = 0;
> +       ctx->nr_data = 0;
> +}
> +
> +/*
> + * Parse lowerdir= mount option:
> + *
> + * (1) lowerdir=/lower1:/lower2:/lower3::/data1::/data2
> + *     Set "/lower1", "/lower2", and "/lower3" as lower layers and
> + *     "/data1" and "/data2" as data lower layers. Any existing lower
> + *     layers are replaced.
> + * (2) lowerdir=:/lower4
> + *     Append "/lower4" to current stack of lower layers. This requires
> + *     that there already is at least one lower layer configured.
> + * (3) lowerdir=::/lower5
> + *     Append data "/lower5" as data lower layer. This requires that
> + *     there's at least one regular lower layer present.
> + */
> +int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
> +{
> +       int err;
> +       struct ovl_fs_context *ctx = fc->fs_private;
> +       struct ovl_fs_context_layer *l;
> +       char *dup = NULL, *dup_iter;
> +       ssize_t nr_lower = 0, nr = 0, nr_data = 0;
> +       bool append = false, data_layer = false;
> +
> +       /*
> +        * Ensure we're backwards compatible with mount(2)
> +        * by allowing relative paths.
> +        */
> +
> +       /* drop all existing lower layers */
> +       if (!*name) {
> +               ovl_parse_param_drop_lowerdir(ctx);
> +               return 0;
> +       }
> +
> +       if (strncmp(name, "::", 2) == 0) {
> +               /*
> +                * This is a data layer.
> +                * There must be at least one regular lower layer
> +                * specified.
> +                */
> +               if (ctx->nr == 0) {
> +                       pr_err("data lower layers without regular lower layers not allowed");
> +                       return -EINVAL;
> +               }
> +
> +               /* Skip the leading "::". */
> +               name += 2;
> +               data_layer = true;
> +               /*
> +                * A data layer is automatically an append as there
> +                * must've been at least one regular lower layer.
> +                */
> +               append = true;
> +       } else if (*name == ':') {
> +               /*
> +                * This is a regular lower layer.
> +                * If users want to append a layer enforce that they
> +                * have already specified a first layer before. It's
> +                * better to be strict.
> +                */
> +               if (ctx->nr == 0) {
> +                       pr_err("cannot append layer if no previous layer has been specified");
> +                       return -EINVAL;
> +               }
> +
> +               /*
> +                * Once a sequence of data layers has started regular
> +                * lower layers are forbidden.
> +                */
> +               if (ctx->nr_data > 0) {
> +                       pr_err("regular lower layers cannot follow data lower layers");
> +                       return -EINVAL;
> +               }
> +
> +               /* Skip the leading ":". */
> +               name++;
> +               append = true;
> +       }
> +
> +       dup = kstrdup(name, GFP_KERNEL);
> +       if (!dup)
> +               return -ENOMEM;
> +
> +       err = -EINVAL;
> +       nr_lower = ovl_parse_param_split_lowerdirs(dup);
> +       if (nr_lower < 0)
> +               goto out_err;
> +
> +       if ((nr_lower > OVL_MAX_STACK) ||
> +           (append && (size_add(ctx->nr, nr_lower) > OVL_MAX_STACK))) {
> +               pr_err("too many lower directories, limit is %d\n", OVL_MAX_STACK);
> +               goto out_err;
> +       }
> +
> +       if (!append)
> +               ovl_parse_param_drop_lowerdir(ctx);
> +
> +       /*
> +        * (1) append
> +        *
> +        * We want nr <= nr_lower <= capacity We know nr > 0 and nr <=
> +        * capacity. If nr == 0 this wouldn't be append. If nr +
> +        * nr_lower is <= capacity then nr <= nr_lower <= capacity
> +        * already holds. If nr + nr_lower exceeds capacity, we realloc.
> +        *
> +        * (2) replace
> +        *
> +        * Ensure we're backwards compatible with mount(2) which allows
> +        * "lowerdir=/a:/b:/c,lowerdir=/d:/e:/f" causing the last
> +        * specified lowerdir mount option to win.
> +        *
> +        * We want nr <= nr_lower <= capacity We know either (i) nr == 0
> +        * or (ii) nr > 0. We also know nr_lower > 0. The capacity
> +        * could've been changed multiple times already so we only know
> +        * nr <= capacity. If nr + nr_lower > capacity we realloc,
> +        * otherwise nr <= nr_lower <= capacity holds already.
> +        */
> +       nr_lower += ctx->nr;
> +       if (nr_lower > ctx->capacity) {
> +               err = -ENOMEM;
> +               l = krealloc_array(ctx->lower, nr_lower, sizeof(*ctx->lower),
> +                                  GFP_KERNEL_ACCOUNT);
> +               if (!l)
> +                       goto out_err;
> +
> +               ctx->lower = l;
> +               ctx->capacity = nr_lower;
> +       }
> +
> +       /*
> +        *   (3) By (1) and (2) we know nr <= nr_lower <= capacity.
> +        *   (4) If ctx->nr == 0 => replace
> +        *       We have verified above that the lowerdir mount option
> +        *       isn't an append, i.e., the lowerdir mount option
> +        *       doesn't start with ":" or "::".
> +        * (4.1) The lowerdir mount options only contains regular lower
> +        *       layers ":".
> +        *       => Nothing to verify.
> +        * (4.2) The lowerdir mount options contains regular ":" and
> +        *       data "::" layers.
> +        *       => We need to verify that data lower layers "::" aren't
> +        *          followed by regular ":" lower layers
> +        *   (5) If ctx->nr > 0 => append
> +        *       We know that there's at least one regular layer
> +        *       otherwise we would've failed when parsing the previous
> +        *       lowerdir mount option.
> +        * (5.1) The lowerdir mount option is a regular layer ":" append
> +        *       => We need to verify that no data layers have been
> +        *          specified before.
> +        * (5.2) The lowerdir mount option is a data layer "::" append
> +        *       We know that there's at least one regular layer or
> +        *       other data layers. => There's nothing to verify.
> +        */
> +       dup_iter = dup;
> +       for (nr = ctx->nr; nr < nr_lower; nr++) {
> +               l = &ctx->lower[nr];
> +
> +               err = ovl_mount_dir_noesc(dup_iter, &l->path);
> +               if (err)
> +                       goto out_put;
> +
> +               err = -ENOMEM;
> +               l->name = kstrdup(dup_iter, GFP_KERNEL_ACCOUNT);
> +               if (!l->name)
> +                       goto out_put;
> +
> +               if (data_layer)
> +                       nr_data++;
> +
> +               /* Calling strchr() again would overrun. */
> +               if ((nr + 1) == nr_lower)
> +                       break;
> +
> +               err = -EINVAL;
> +               dup_iter = strchr(dup_iter, '\0') + 1;
> +               if (*dup_iter) {
> +                       /*
> +                        * This is a regular layer so we require that
> +                        * there are no data layers.
> +                        */
> +                       if ((ctx->nr_data + nr_data) > 0) {
> +                               pr_err("regular lower layers cannot follow data lower layers");
> +                               goto out_put;
> +                       }
> +
> +                       data_layer = false;
> +                       continue;
> +               }
> +
> +               /* This is a data lower layer. */
> +               data_layer = true;
> +               dup_iter++;
> +       }
> +       ctx->nr = nr_lower;
> +       ctx->nr_data += nr_data;
> +       kfree(dup);
> +       return 0;
> +
> +out_put:
> +       /*
> +        * We know nr >= ctx->nr < nr_lower. If we failed somewhere
> +        * we want to undo until nr == ctx->nr. This is correct for
> +        * both ctx->nr == 0 and ctx->nr > 0.
> +        */
> +       for (; nr >= ctx->nr; nr--) {
> +               l = &ctx->lower[nr];
> +               kfree(l->name);
> +               l->name = NULL;
> +               path_put(&l->path);
> +
> +               /* don't overflow */
> +               if (nr == 0)
> +                       break;
> +       }
> +
> +out_err:
> +       kfree(dup);
> +
> +       /* Intentionally don't realloc to a smaller size. */
> +       return err;
> +}
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 3392dc5d2082..b73b14c52961 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -27,8 +27,6 @@ MODULE_LICENSE("GPL");
>
>  struct ovl_dir_cache;
>
> -#define OVL_MAX_STACK 500
> -
>  static bool ovl_redirect_dir_def = IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_DIR);
>  module_param_named(redirect_dir, ovl_redirect_dir_def, bool, 0644);
>  MODULE_PARM_DESC(redirect_dir,
> @@ -109,8 +107,11 @@ static const char *ovl_redirect_mode(struct ovl_config *config)
>         return ovl_parameter_redirect_dir[config->redirect_mode].name;
>  }
>
> +#define fsparam_string_empty(NAME, OPT) \
> +       __fsparam(fs_param_is_string, NAME, OPT, fs_param_can_be_empty, NULL)
> +
>  static const struct fs_parameter_spec ovl_parameter_spec[] = {
> -       fsparam_string("lowerdir",          Opt_lowerdir),
> +       fsparam_string_empty("lowerdir",    Opt_lowerdir),
>         fsparam_string("upperdir",          Opt_upperdir),
>         fsparam_string("workdir",           Opt_workdir),
>         fsparam_flag("default_permissions", Opt_default_permissions),
> @@ -125,15 +126,15 @@ static const struct fs_parameter_spec ovl_parameter_spec[] = {
>         {}
>  };
>
> +/*
> + * These options imply different behavior when they are explicitly
> + * specified than when they are left in their default state.
> + */
>  #define OVL_METACOPY_SET       BIT(0)
>  #define OVL_REDIRECT_SET       BIT(1)
>  #define OVL_NFS_EXPORT_SET     BIT(2)
>  #define OVL_INDEX_SET          BIT(3)
>
> -struct ovl_fs_context {
> -       u8 set;
> -};
> -
>  static struct dentry *ovl_d_real(struct dentry *dentry,
>                                  const struct inode *inode)
>  {
> @@ -308,6 +309,7 @@ static void ovl_free_fs(struct ovl_fs *ofs)
>         for (i = 0; i < ofs->numlayer; i++) {
>                 iput(ofs->layers[i].trap);
>                 mounts[i] = ofs->layers[i].mnt;
> +               kfree(ofs->layers[i].name);
>         }
>         kern_unmount_array(mounts, ofs->numlayer);
>         kfree(ofs->layers);
> @@ -315,7 +317,6 @@ static void ovl_free_fs(struct ovl_fs *ofs)
>                 free_anon_bdev(ofs->fs[i].pseudo_dev);
>         kfree(ofs->fs);
>
> -       kfree(ofs->config.lowerdir);
>         kfree(ofs->config.upperdir);
>         kfree(ofs->config.workdir);
>         if (ofs->creator_cred)
> @@ -433,8 +434,17 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
>         struct super_block *sb = dentry->d_sb;
>         struct ovl_fs *ofs = sb->s_fs_info;
>         const char *redirect_mode;
> -
> -       seq_show_option(m, "lowerdir", ofs->config.lowerdir);
> +       size_t nr, nr_merged_lower = ofs->numlayer - ofs->numdatalayer;
> +       const struct ovl_layer *data_layers = &ofs->layers[nr_merged_lower];
> +
> +       /* ofs->layers[0] is the upper layer */
> +       seq_printf(m, ",lowerdir=%s", ofs->layers[1].name);
> +       /* dump regular lower layers */
> +       for (nr = 2; nr < nr_merged_lower; nr++)
> +               seq_printf(m, ":%s", ofs->layers[nr].name);
> +       /* dump data lower layers */
> +       for (nr = 0; nr < ofs->numdatalayer; nr++)
> +               seq_printf(m, "::%s", data_layers[nr].name);
>         if (ofs->config.upperdir) {
>                 seq_show_option(m, "upperdir", ofs->config.upperdir);
>                 seq_show_option(m, "workdir", ofs->config.workdir);
> @@ -509,6 +519,11 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>         bool nfs_export_opt = ctx->set & OVL_NFS_EXPORT_SET;
>         bool index_opt = ctx->set & OVL_INDEX_SET;
>
> +       if (ctx->nr_data > 0 && !config->metacopy) {
> +               pr_err("lower data-only dirs require metacopy support.\n");
> +               return -EINVAL;
> +       }
> +
>         /* Workdir/index are useless in non-upper mount */
>         if (!config->upperdir) {
>                 if (config->workdir) {
> @@ -723,69 +738,6 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs,
>         goto out_unlock;
>  }
>
> -static void ovl_unescape(char *s)
> -{
> -       char *d = s;
> -
> -       for (;; s++, d++) {
> -               if (*s == '\\')
> -                       s++;
> -               *d = *s;
> -               if (!*s)
> -                       break;
> -       }
> -}
> -
> -static int ovl_mount_dir_noesc(const char *name, struct path *path)
> -{
> -       int err = -EINVAL;
> -
> -       if (!*name) {
> -               pr_err("empty lowerdir\n");
> -               goto out;
> -       }
> -       err = kern_path(name, LOOKUP_FOLLOW, path);
> -       if (err) {
> -               pr_err("failed to resolve '%s': %i\n", name, err);
> -               goto out;
> -       }
> -       err = -EINVAL;
> -       if (ovl_dentry_weird(path->dentry)) {
> -               pr_err("filesystem on '%s' not supported\n", name);
> -               goto out_put;
> -       }
> -       if (!d_is_dir(path->dentry)) {
> -               pr_err("'%s' not a directory\n", name);
> -               goto out_put;
> -       }
> -       return 0;
> -
> -out_put:
> -       path_put_init(path);
> -out:
> -       return err;
> -}
> -
> -static int ovl_mount_dir(const char *name, struct path *path)
> -{
> -       int err = -ENOMEM;
> -       char *tmp = kstrdup(name, GFP_KERNEL);
> -
> -       if (tmp) {
> -               ovl_unescape(tmp);
> -               err = ovl_mount_dir_noesc(tmp, path);
> -
> -               if (!err && path->dentry->d_flags & DCACHE_OP_REAL) {
> -                       pr_err("filesystem on '%s' not supported as upperdir\n",
> -                              tmp);
> -                       path_put_init(path);
> -                       err = -EINVAL;
> -               }
> -               kfree(tmp);
> -       }
> -       return err;
> -}
> -
>  static int ovl_check_namelen(const struct path *path, struct ovl_fs *ofs,
>                              const char *name)
>  {
> @@ -806,10 +758,6 @@ static int ovl_lower_dir(const char *name, struct path *path,
>         int fh_type;
>         int err;
>
> -       err = ovl_mount_dir_noesc(name, path);
> -       if (err)
> -               return err;
> -
>         err = ovl_check_namelen(path, ofs, name);
>         if (err)
>                 return err;
> @@ -858,26 +806,6 @@ static bool ovl_workdir_ok(struct dentry *workdir, struct dentry *upperdir)
>         return ok;
>  }
>
> -static unsigned int ovl_split_lowerdirs(char *str)
> -{
> -       unsigned int ctr = 1;
> -       char *s, *d;
> -
> -       for (s = d = str;; s++, d++) {
> -               if (*s == '\\') {
> -                       s++;
> -               } else if (*s == ':') {
> -                       *d = '\0';
> -                       ctr++;
> -                       continue;
> -               }
> -               *d = *s;
> -               if (!*s)
> -                       break;
> -       }
> -       return ctr;
> -}
> -
>  static int ovl_own_xattr_get(const struct xattr_handler *handler,
>                              struct dentry *dentry, struct inode *inode,
>                              const char *name, void *buffer, size_t size)
> @@ -978,15 +906,12 @@ static int ovl_report_in_use(struct ovl_fs *ofs, const char *name)
>  }
>
>  static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
> -                        struct ovl_layer *upper_layer, struct path *upperpath)
> +                        struct ovl_layer *upper_layer,
> +                        const struct path *upperpath)
>  {
>         struct vfsmount *upper_mnt;
>         int err;
>
> -       err = ovl_mount_dir(ofs->config.upperdir, upperpath);
> -       if (err)
> -               goto out;
> -
>         /* Upperdir path should not be r/o */
>         if (__mnt_is_readonly(upperpath->mnt)) {
>                 pr_err("upper fs is r/o, try multi-lower layers mount\n");
> @@ -1016,6 +941,11 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
>         upper_layer->idx = 0;
>         upper_layer->fsid = 0;
>
> +       err = -ENOMEM;
> +       upper_layer->name = kstrdup(ofs->config.upperdir, GFP_KERNEL);
> +       if (!upper_layer->name)
> +               goto out;
> +
>         /*
>          * Inherit SB_NOSEC flag from upperdir.
>          *
> @@ -1273,46 +1203,37 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
>  }
>
>  static int ovl_get_workdir(struct super_block *sb, struct ovl_fs *ofs,
> -                          const struct path *upperpath)
> +                          const struct path *upperpath,
> +                          const struct path *workpath)
>  {
>         int err;
> -       struct path workpath = { };
> -
> -       err = ovl_mount_dir(ofs->config.workdir, &workpath);
> -       if (err)
> -               goto out;
>
>         err = -EINVAL;
> -       if (upperpath->mnt != workpath.mnt) {
> +       if (upperpath->mnt != workpath->mnt) {
>                 pr_err("workdir and upperdir must reside under the same mount\n");
> -               goto out;
> +               return err;
>         }
> -       if (!ovl_workdir_ok(workpath.dentry, upperpath->dentry)) {
> +       if (!ovl_workdir_ok(workpath->dentry, upperpath->dentry)) {
>                 pr_err("workdir and upperdir must be separate subtrees\n");
> -               goto out;
> +               return err;
>         }
>
> -       ofs->workbasedir = dget(workpath.dentry);
> +       ofs->workbasedir = dget(workpath->dentry);
>
>         if (ovl_inuse_trylock(ofs->workbasedir)) {
>                 ofs->workdir_locked = true;
>         } else {
>                 err = ovl_report_in_use(ofs, "workdir");
>                 if (err)
> -                       goto out;
> +                       return err;
>         }
>
>         err = ovl_setup_trap(sb, ofs->workbasedir, &ofs->workbasedir_trap,
>                              "workdir");
>         if (err)
> -               goto out;
> -
> -       err = ovl_make_workdir(sb, ofs, &workpath);
> -
> -out:
> -       path_put(&workpath);
> +               return err;
>
> -       return err;
> +       return ovl_make_workdir(sb, ofs, workpath);
>  }
>
>  static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
> @@ -1476,13 +1397,13 @@ static int ovl_get_data_fsid(struct ovl_fs *ofs)
>
>
>  static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
> -                         struct path *stack, unsigned int numlower,
> -                         struct ovl_layer *layers)
> +                         struct ovl_fs_context *ctx, struct ovl_layer *layers)
>  {
>         int err;
>         unsigned int i;
> +       size_t nr_merged_lower;
>
> -       ofs->fs = kcalloc(numlower + 2, sizeof(struct ovl_sb), GFP_KERNEL);
> +       ofs->fs = kcalloc(ctx->nr + 2, sizeof(struct ovl_sb), GFP_KERNEL);
>         if (ofs->fs == NULL)
>                 return -ENOMEM;
>
> @@ -1509,13 +1430,15 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>                 ofs->fs[0].is_lower = false;
>         }
>
> -       for (i = 0; i < numlower; i++) {
> +       nr_merged_lower = ctx->nr - ctx->nr_data;
> +       for (i = 0; i < ctx->nr; i++) {
> +               struct ovl_fs_context_layer *l = &ctx->lower[i];
>                 struct vfsmount *mnt;
>                 struct inode *trap;
>                 int fsid;
>
> -               if (i < numlower - ofs->numdatalayer)
> -                       fsid = ovl_get_fsid(ofs, &stack[i]);
> +               if (i < nr_merged_lower)
> +                       fsid = ovl_get_fsid(ofs, &l->path);
>                 else
>                         fsid = ovl_get_data_fsid(ofs);
>                 if (fsid < 0)
> @@ -1528,11 +1451,11 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>                  * the upperdir/workdir is in fact in-use by our
>                  * upperdir/workdir.
>                  */
> -               err = ovl_setup_trap(sb, stack[i].dentry, &trap, "lowerdir");
> +               err = ovl_setup_trap(sb, l->path.dentry, &trap, "lowerdir");
>                 if (err)
>                         return err;
>
> -               if (ovl_is_inuse(stack[i].dentry)) {
> +               if (ovl_is_inuse(l->path.dentry)) {
>                         err = ovl_report_in_use(ofs, "lowerdir");
>                         if (err) {
>                                 iput(trap);
> @@ -1540,7 +1463,7 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>                         }
>                 }
>
> -               mnt = clone_private_mount(&stack[i]);
> +               mnt = clone_private_mount(&l->path);
>                 err = PTR_ERR(mnt);
>                 if (IS_ERR(mnt)) {
>                         pr_err("failed to clone lowerpath\n");
> @@ -1559,6 +1482,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>                 layers[ofs->numlayer].idx = ofs->numlayer;
>                 layers[ofs->numlayer].fsid = fsid;
>                 layers[ofs->numlayer].fs = &ofs->fs[fsid];
> +               layers[ofs->numlayer].name = l->name;
> +               l->name = NULL;
>                 ofs->numlayer++;
>                 ofs->fs[fsid].is_lower = true;
>         }
> @@ -1599,104 +1524,59 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>  }
>
>  static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb,
> -                               const char *lower, unsigned int numlower,
> -                               struct ovl_fs *ofs, struct ovl_layer *layers)
> +                                           struct ovl_fs_context *ctx,
> +                                           struct ovl_fs *ofs,
> +                                           struct ovl_layer *layers)
>  {
>         int err;
> -       struct path *stack = NULL;
> -       struct ovl_path *lowerstack;
> -       unsigned int numlowerdata = 0;
>         unsigned int i;
> +       size_t nr_merged_lower;
>         struct ovl_entry *oe;
> +       struct ovl_path *lowerstack;
>
> -       if (!ofs->config.upperdir && numlower == 1) {
> +       struct ovl_fs_context_layer *l;
> +
> +       if (!ofs->config.upperdir && ctx->nr == 1) {
>                 pr_err("at least 2 lowerdir are needed while upperdir nonexistent\n");
>                 return ERR_PTR(-EINVAL);
>         }
>
> -       stack = kcalloc(numlower, sizeof(struct path), GFP_KERNEL);
> -       if (!stack)
> -               return ERR_PTR(-ENOMEM);
> +       err = -EINVAL;
> +       for (i = 0; i < ctx->nr; i++) {
> +               l = &ctx->lower[i];
>
> -       for (i = 0; i < numlower;) {
> -               err = ovl_lower_dir(lower, &stack[i], ofs, &sb->s_stack_depth);
> +               err = ovl_lower_dir(l->name, &l->path, ofs, &sb->s_stack_depth);
>                 if (err)
> -                       goto out_err;
> -
> -               lower = strchr(lower, '\0') + 1;
> -
> -               i++;
> -               if (i == numlower)
> -                       break;
> -
> -               err = -EINVAL;
> -               /*
> -                * Empty lower layer path could mean :: separator that indicates
> -                * a data-only lower data.
> -                * Several data-only layers are allowed, but they all need to be
> -                * at the bottom of the stack.
> -                */
> -               if (*lower) {
> -                       /* normal lower dir */
> -                       if (numlowerdata) {
> -                               pr_err("lower data-only dirs must be at the bottom of the stack.\n");
> -                               goto out_err;
> -                       }
> -               } else {
> -                       /* data-only lower dir */
> -                       if (!ofs->config.metacopy) {
> -                               pr_err("lower data-only dirs require metacopy support.\n");
> -                               goto out_err;
> -                       }
> -                       if (i == numlower - 1) {
> -                               pr_err("lowerdir argument must not end with double colon.\n");
> -                               goto out_err;
> -                       }
> -                       lower++;
> -                       numlower--;
> -                       numlowerdata++;
> -               }
> -       }
> -
> -       if (numlowerdata) {
> -               ofs->numdatalayer = numlowerdata;
> -               pr_info("using the lowest %d of %d lowerdirs as data layers\n",
> -                       numlowerdata, numlower);
> +                       return ERR_PTR(err);
>         }
>
>         err = -EINVAL;
>         sb->s_stack_depth++;
>         if (sb->s_stack_depth > FILESYSTEM_MAX_STACK_DEPTH) {
>                 pr_err("maximum fs stacking depth exceeded\n");
> -               goto out_err;
> +               return ERR_PTR(err);
>         }
>
> -       err = ovl_get_layers(sb, ofs, stack, numlower, layers);
> +       err = ovl_get_layers(sb, ofs, ctx, layers);
>         if (err)
> -               goto out_err;
> +               return ERR_PTR(err);
>
>         err = -ENOMEM;
>         /* Data-only layers are not merged in root directory */
> -       oe = ovl_alloc_entry(numlower - numlowerdata);
> +       nr_merged_lower = ctx->nr - ctx->nr_data;
> +       oe = ovl_alloc_entry(nr_merged_lower);
>         if (!oe)
> -               goto out_err;
> +               return ERR_PTR(err);
>
>         lowerstack = ovl_lowerstack(oe);
> -       for (i = 0; i < numlower - numlowerdata; i++) {
> -               lowerstack[i].dentry = dget(stack[i].dentry);
> -               lowerstack[i].layer = &ofs->layers[i+1];
> +       for (i = 0; i < nr_merged_lower; i++) {
> +               l = &ctx->lower[i];
> +               lowerstack[i].dentry = dget(l->path.dentry);
> +               lowerstack[i].layer = &ofs->layers[i + 1];
>         }
> -
> -out:
> -       for (i = 0; i < numlower; i++)
> -               path_put(&stack[i]);
> -       kfree(stack);
> +       ofs->numdatalayer = ctx->nr_data;
>
>         return oe;
> -
> -out_err:
> -       oe = ERR_PTR(err);
> -       goto out;
>  }
>
>  /*
> @@ -1804,6 +1684,12 @@ static struct dentry *ovl_get_root(struct super_block *sb,
>         ovl_set_upperdata(d_inode(root));
>         ovl_inode_init(d_inode(root), &oip, ino, fsid);
>         ovl_dentry_init_flags(root, upperdentry, oe, DCACHE_OP_WEAK_REVALIDATE);
> +       /*
> +        * We're going to put upper path when we call
> +        * fs_context_operations->free() take an additional
> +        * reference so we can just call path_put().
> +        */
> +       dget(upperdentry);

That's a very odd context for this comment.
The fact is that ovl_inode_init() takes ownership of upperdentry
so it is better to clarify that ovl_get_root() takes ownership of
ovl_get_root() takes ownership of upperdentry.
I will post a prep patch....

[...]

> @@ -1952,28 +1813,20 @@ static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>         if (err)
>                 goto out_err;
>
> +       /*
> +        * Check ctx->nr instead of ofs->config.lowerdir since we're
> +        * going to set ofs->config.lowerdir here after we know that the
> +        * user specified all layers.
> +        */

outdated comment - removed in my branch.

>         err = -EINVAL;
> -       if (!ofs->config.lowerdir) {
> +       if (ctx->nr == 0) {
>                 if (fc->sb_flags & SB_SILENT)
>                         pr_err("missing 'lowerdir'\n");
>                 goto out_err;
>         }

[...]

> @@ -2085,13 +1938,10 @@ static int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>         sb->s_iflags |= SB_I_SKIP_SYNC;
>
>         err = -ENOMEM;
> -       root_dentry = ovl_get_root(sb, upperpath.dentry, oe);
> +       root_dentry = ovl_get_root(sb, ctx->upper.dentry, oe);
>         if (!root_dentry)
>                 goto out_free_oe;
>
> -       mntput(upperpath.mnt);
> -       kfree(splitlower);
> -
>         sb->s_root = root_dentry;
>

... I will post a prep patch to replace mntput(upperpath.mnt)
here with path_put(&upperpath), so you patch just moves
path_put() to fs_context_operations->free().

Thanks,
Amir.

Re: [PATCH v3 3/3] ovl: change layer mount option handling

[Amir Goldstein]

On Tue, Jun 13, 2023 at 5:49 PM Christian Brauner <brauner@kernel.org> wrote:
>
> We ran into issues where mount(8) passed multiple lower layers as one
> big string through fsconfig(). But the fsconfig() FSCONFIG_SET_STRING
> option is limited to 256 bytes in strndup_user(). While this would be
> fixable by extending the fsconfig() buffer I'd rather encourage users to
> append layers via multiple fsconfig() calls as the interface allows
> nicely for this. This has also been requested as a feature before.
>
> With this port to the new mount api the following will be possible:
>
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir", "/lower1", 0);
>
>         /* set upper layer */
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "upperdir", "/upper", 0);
>
>         /* append "/lower2", "/lower3", and "/lower4" */
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir", ":/lower2:/lower3:/lower4", 0);
>
>         /* turn index feature on */
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "index", "on", 0);
>
>         /* append "/lower5" */
>         fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir", ":/lower5", 0);
>
> Specifying ':' would have been rejected so this isn't a regression. And
> we can't simply use "lowerdir=/lower" to append on top of existing
> layers as "lowerdir=/lower,lowerdir=/other-lower" would make
> "/other-lower" the only lower layer so we'd break uapi if we changed
> this. So the ':' prefix seems a good compromise.
>
> Users can choose to specify multiple layers at once or individual
> layers. A layer is appended if it starts with ":". This requires that
> the user has already added at least one layer before. If lowerdir is
> specified again without a leading ":" then all previous layers are
> dropped and replaced with the new layers. If lowerdir is specified and
> empty than all layers are simply dropped.
>
> An additional change is that overlayfs will now parse and resolve layers
> right when they are specified in fsconfig() instead of deferring until
> super block creation. This allows users to receive early errors.
>
> It also allows users to actually use up to 500 layers something which
> was theoretically possible but ended up not working due to the mount
> option string passed via mount(2) being too large.
>
> This also allows a more privileged process to set config options for a
> lesser privileged process as the creds for fsconfig() and the creds for
> fsopen() can differ. We could restrict that they match by enforcing that
> the creds of fsopen() and fsconfig() match but I don't see why that
> needs to be the case and allows for a good delegation mechanism.
>
> Plus, in the future it means we're able to extend overlayfs mount
> options and allow users to specify layers via file descriptors instead
> of paths:
>
>         fsconfig(FSCONFIG_SET_PATH{_EMPTY}, "lowerdir", "lower1", dirfd);
>
>         /* append */
>         fsconfig(FSCONFIG_SET_PATH{_EMPTY}, "lowerdir", "lower2", dirfd);
>
>         /* append */
>         fsconfig(FSCONFIG_SET_PATH{_EMPTY}, "lowerdir", "lower3", dirfd);
>
>         /* clear all layers specified until now */
>         fsconfig(FSCONFIG_SET_STRING, "lowerdir", NULL, 0);
>
> This would be especially nice if users create an overlayfs mount on top
> of idmapped layers or just in general private mounts created via
> open_tree(OPEN_TREE_CLONE). Those mounts would then never have to appear
> anywhere in the filesystem. But for now just do the minimal thing.
>
> We should probably aim to move more validation into ovl_fs_parse_param()
> so users get errors before fsconfig(FSCONFIG_CMD_CREATE). But that can
> be done in additional patches later.
>
> This is now also rebased on top of the lazy lowerdata lookup which
> allows the specificatin of data only layers using the new "::" syntax.
>
> The rules are simple. A data only layers cannot be followed by any
> regular layers and data layers must be preceeded by at least one regular
> layer.
>
> Parsing the lowerdir mount option must change because of this. The
> original patchset used the old lowerdir parsing function to split a
> lowerdir mount option string such as:
>
>         lowerdir=/lower1:/lower2::/lower3::/lower4
>
> simply replacing each non-escaped ":" by "\0". So sequences of
> non-escaped ":" were counted as layers. For example, the previous
> lowerdir mount option above would've counted 6 layers instead of 4 and a
> lowerdir mount option such as:
>
>         lowerdir="/lower1:/lower2::/lower3::/lower4:::::::::::::::::::::::::::"
>
> would be counted as 33 layers. Other than being ugly this didn't matter
> much because kern_path() would reject the first "\0" layer. However,
> this overcounting of layers becomes problematic when we base allocations
> on it where we very much only want to allocate space for 4 layers
> instead of 33.
>
> So the new parsing function rejects non-escaped sequences of colons
> other than ":" and "::" immediately instead of relying on kern_path().
>
> Link: https://github.com/util-linux/util-linux/issues/2287
> Link: https://github.com/util-linux/util-linux/issues/1992
> Link: https://bugs.archlinux.org/task/78702
> Link: https://lore.kernel.org/linux-unionfs/20230530-klagen-zudem-32c0908c2108@brauner
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/overlayfs/Makefile    |   2 +-
>  fs/overlayfs/overlayfs.h |  23 +++
>  fs/overlayfs/ovl_entry.h |   3 +-
>  fs/overlayfs/params.c    | 388 +++++++++++++++++++++++++++++++++++++++++++++++
>  fs/overlayfs/super.c     | 376 +++++++++++++++------------------------------
>  5 files changed, 534 insertions(+), 258 deletions(-)
>
> diff --git a/fs/overlayfs/Makefile b/fs/overlayfs/Makefile
> index 9164c585eb2f..4e173d56b11f 100644
> --- a/fs/overlayfs/Makefile
> +++ b/fs/overlayfs/Makefile
> @@ -6,4 +6,4 @@
>  obj-$(CONFIG_OVERLAY_FS) += overlay.o
>
>  overlay-objs := super.o namei.o util.o inode.o file.o dir.o readdir.o \
> -               copy_up.o export.o
> +               copy_up.o export.o params.o
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index fcac4e2c56ab..7659ea6e02cb 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -119,6 +119,29 @@ struct ovl_fh {
>  #define OVL_FH_FID_OFFSET      (OVL_FH_WIRE_OFFSET + \
>                                  offsetof(struct ovl_fb, fid))
>
> +/* params.c */
> +#define OVL_MAX_STACK 500
> +
> +struct ovl_fs_context_layer {
> +       char *name;
> +       struct path path;
> +};
> +
> +struct ovl_fs_context {
> +       struct path upper;
> +       struct path work;
> +       size_t capacity;
> +       size_t nr; /* includes nr_data */
> +       size_t nr_data;
> +       u8 set;
> +       struct ovl_fs_context_layer *lower;
> +};
> +
> +int ovl_parse_param_upperdir(const char *name, struct fs_context *fc,
> +                            bool workdir);
> +int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc);
> +void ovl_parse_param_drop_lowerdir(struct ovl_fs_context *ctx);
> +
>  extern const char *const ovl_xattr_table[][2];
>  static inline const char *ovl_xattr(struct ovl_fs *ofs, enum ovl_xattr ox)
>  {
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index c72433c06006..7888ab33730b 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -6,7 +6,6 @@
>   */
>
>  struct ovl_config {
> -       char *lowerdir;
>         char *upperdir;
>         char *workdir;
>         bool default_permissions;
> @@ -41,6 +40,7 @@ struct ovl_layer {
>         int idx;
>         /* One fsid per unique underlying sb (upper fsid == 0) */
>         int fsid;
> +       char *name;
>  };
>
>  /*
> @@ -101,7 +101,6 @@ struct ovl_fs {
>         errseq_t errseq;
>  };
>
> -
>  /* Number of lower layers, not including data-only layers */
>  static inline unsigned int ovl_numlowerlayer(struct ovl_fs *ofs)
>  {
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> new file mode 100644
> index 000000000000..a1606af1613f
> --- /dev/null
> +++ b/fs/overlayfs/params.c
> @@ -0,0 +1,388 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/fs.h>
> +#include <linux/namei.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
> +#include <linux/posix_acl_xattr.h>
> +#include <linux/xattr.h>
> +#include "overlayfs.h"
> +
> +static ssize_t ovl_parse_param_split_lowerdirs(char *str)
> +{
> +       ssize_t nr_layers = 1, nr_colons = 0;
> +       char *s, *d;
> +
> +       for (s = d = str;; s++, d++) {
> +               if (*s == '\\') {
> +                       s++;
> +               } else if (*s == ':') {
> +                       bool next_colon = (*(s + 1) == ':');
> +
> +                       nr_colons++;
> +                       if (nr_colons == 2 && next_colon) {
> +                               pr_err("only single ':' or double '::' sequences of unescaped colons in lowerdir mount option allowed.\n");
> +                               return -EINVAL;
> +                       }
> +                       /* count layers, not colons */
> +                       if (!next_colon)
> +                               nr_layers++;
> +
> +                       *d = '\0';
> +                       continue;
> +               }
> +
> +               *d = *s;
> +               if (!*s) {
> +                       /* trailing colons */
> +                       if (nr_colons) {
> +                               pr_err("unescaped trailing colons in lowerdir mount option.\n");
> +                               return -EINVAL;
> +                       }
> +                       break;
> +               }
> +               nr_colons = 0;
> +       }
> +
> +       return nr_layers;
> +}
> +
> +static int ovl_mount_dir_noesc(const char *name, struct path *path)
> +{
> +       int err = -EINVAL;
> +
> +       if (!*name) {
> +               pr_err("empty lowerdir\n");
> +               goto out;
> +       }
> +       err = kern_path(name, LOOKUP_FOLLOW, path);
> +       if (err) {
> +               pr_err("failed to resolve '%s': %i\n", name, err);
> +               goto out;
> +       }
> +       err = -EINVAL;
> +       if (ovl_dentry_weird(path->dentry)) {
> +               pr_err("filesystem on '%s' not supported\n", name);
> +               goto out_put;
> +       }
> +       if (!d_is_dir(path->dentry)) {
> +               pr_err("'%s' not a directory\n", name);
> +               goto out_put;
> +       }
> +       return 0;
> +
> +out_put:
> +       path_put_init(path);
> +out:
> +       return err;
> +}
> +
> +static void ovl_unescape(char *s)
> +{
> +       char *d = s;
> +
> +       for (;; s++, d++) {
> +               if (*s == '\\')
> +                       s++;
> +               *d = *s;
> +               if (!*s)
> +                       break;
> +       }
> +}
> +
> +static int ovl_mount_dir(const char *name, struct path *path)
> +{
> +       int err = -ENOMEM;
> +       char *tmp = kstrdup(name, GFP_KERNEL);
> +
> +       if (tmp) {
> +               ovl_unescape(tmp);
> +               err = ovl_mount_dir_noesc(tmp, path);
> +
> +               if (!err && path->dentry->d_flags & DCACHE_OP_REAL) {
> +                       pr_err("filesystem on '%s' not supported as upperdir\n",
> +                              tmp);
> +                       path_put_init(path);
> +                       err = -EINVAL;
> +               }
> +               kfree(tmp);
> +       }
> +       return err;
> +}
> +
> +int ovl_parse_param_upperdir(const char *name, struct fs_context *fc,
> +                            bool workdir)
> +{
> +       int err;
> +       struct ovl_fs *ofs = fc->s_fs_info;
> +       struct ovl_config *config = &ofs->config;
> +       struct ovl_fs_context *ctx = fc->fs_private;
> +       struct path path;
> +       char *dup;
> +
> +       err = ovl_mount_dir(name, &path);
> +       if (err)
> +               return err;
> +
> +       /*
> +        * Check whether upper path is read-only here to report failures
> +        * early. Don't forget to recheck when the superblock is created
> +        * as the mount attributes could change.
> +        */
> +       if (__mnt_is_readonly(path.mnt)) {
> +               path_put(&path);
> +               return -EINVAL;
> +       }
> +
> +       dup = kstrdup(name, GFP_KERNEL);
> +       if (!dup) {
> +               path_put(&path);
> +               return -ENOMEM;
> +       }
> +
> +       if (workdir) {
> +               kfree(config->workdir);
> +               config->workdir = dup;
> +               path_put(&ctx->work);
> +               ctx->work = path;
> +       } else {
> +               kfree(config->upperdir);
> +               config->upperdir = dup;
> +               path_put(&ctx->upper);
> +               ctx->upper = path;
> +       }
> +       return 0;
> +}
> +
> +void ovl_parse_param_drop_lowerdir(struct ovl_fs_context *ctx)
> +{
> +       for (size_t nr = 0; nr < ctx->nr; nr++) {
> +               path_put(&ctx->lower[nr].path);
> +               kfree(ctx->lower[nr].name);
> +               ctx->lower[nr].name = NULL;
> +       }
> +       ctx->nr = 0;
> +       ctx->nr_data = 0;
> +}
> +
> +/*
> + * Parse lowerdir= mount option:
> + *
> + * (1) lowerdir=/lower1:/lower2:/lower3::/data1::/data2
> + *     Set "/lower1", "/lower2", and "/lower3" as lower layers and
> + *     "/data1" and "/data2" as data lower layers. Any existing lower
> + *     layers are replaced.
> + * (2) lowerdir=:/lower4
> + *     Append "/lower4" to current stack of lower layers. This requires
> + *     that there already is at least one lower layer configured.
> + * (3) lowerdir=::/lower5
> + *     Append data "/lower5" as data lower layer. This requires that
> + *     there's at least one regular lower layer present.
> + */
> +int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
> +{
> +       int err;
> +       struct ovl_fs_context *ctx = fc->fs_private;
> +       struct ovl_fs_context_layer *l;
> +       char *dup = NULL, *dup_iter;
> +       ssize_t nr_lower = 0, nr = 0, nr_data = 0;
> +       bool append = false, data_layer = false;
> +
> +       /*
> +        * Ensure we're backwards compatible with mount(2)
> +        * by allowing relative paths.
> +        */
> +
> +       /* drop all existing lower layers */
> +       if (!*name) {
> +               ovl_parse_param_drop_lowerdir(ctx);
> +               return 0;
> +       }
> +
> +       if (strncmp(name, "::", 2) == 0) {
> +               /*
> +                * This is a data layer.
> +                * There must be at least one regular lower layer
> +                * specified.
> +                */
> +               if (ctx->nr == 0) {
> +                       pr_err("data lower layers without regular lower layers not allowed");
> +                       return -EINVAL;
> +               }
> +
> +               /* Skip the leading "::". */
> +               name += 2;
> +               data_layer = true;
> +               /*
> +                * A data layer is automatically an append as there
> +                * must've been at least one regular lower layer.
> +                */
> +               append = true;
> +       } else if (*name == ':') {
> +               /*
> +                * This is a regular lower layer.
> +                * If users want to append a layer enforce that they
> +                * have already specified a first layer before. It's
> +                * better to be strict.
> +                */
> +               if (ctx->nr == 0) {
> +                       pr_err("cannot append layer if no previous layer has been specified");
> +                       return -EINVAL;
> +               }
> +
> +               /*
> +                * Once a sequence of data layers has started regular
> +                * lower layers are forbidden.
> +                */
> +               if (ctx->nr_data > 0) {
> +                       pr_err("regular lower layers cannot follow data lower layers");
> +                       return -EINVAL;
> +               }
> +
> +               /* Skip the leading ":". */
> +               name++;
> +               append = true;
> +       }
> +
> +       dup = kstrdup(name, GFP_KERNEL);
> +       if (!dup)
> +               return -ENOMEM;
> +
> +       err = -EINVAL;
> +       nr_lower = ovl_parse_param_split_lowerdirs(dup);
> +       if (nr_lower < 0)
> +               goto out_err;
> +
> +       if ((nr_lower > OVL_MAX_STACK) ||
> +           (append && (size_add(ctx->nr, nr_lower) > OVL_MAX_STACK))) {
> +               pr_err("too many lower directories, limit is %d\n", OVL_MAX_STACK);
> +               goto out_err;
> +       }
> +
> +       if (!append)
> +               ovl_parse_param_drop_lowerdir(ctx);
> +
> +       /*
> +        * (1) append
> +        *
> +        * We want nr <= nr_lower <= capacity We know nr > 0 and nr <=
> +        * capacity. If nr == 0 this wouldn't be append. If nr +
> +        * nr_lower is <= capacity then nr <= nr_lower <= capacity
> +        * already holds. If nr + nr_lower exceeds capacity, we realloc.
> +        *
> +        * (2) replace
> +        *
> +        * Ensure we're backwards compatible with mount(2) which allows
> +        * "lowerdir=/a:/b:/c,lowerdir=/d:/e:/f" causing the last
> +        * specified lowerdir mount option to win.
> +        *
> +        * We want nr <= nr_lower <= capacity We know either (i) nr == 0
> +        * or (ii) nr > 0. We also know nr_lower > 0. The capacity
> +        * could've been changed multiple times already so we only know
> +        * nr <= capacity. If nr + nr_lower > capacity we realloc,
> +        * otherwise nr <= nr_lower <= capacity holds already.
> +        */
> +       nr_lower += ctx->nr;
> +       if (nr_lower > ctx->capacity) {
> +               err = -ENOMEM;
> +               l = krealloc_array(ctx->lower, nr_lower, sizeof(*ctx->lower),
> +                                  GFP_KERNEL_ACCOUNT);
> +               if (!l)
> +                       goto out_err;
> +
> +               ctx->lower = l;
> +               ctx->capacity = nr_lower;
> +       }
> +
> +       /*
> +        *   (3) By (1) and (2) we know nr <= nr_lower <= capacity.
> +        *   (4) If ctx->nr == 0 => replace
> +        *       We have verified above that the lowerdir mount option
> +        *       isn't an append, i.e., the lowerdir mount option
> +        *       doesn't start with ":" or "::".
> +        * (4.1) The lowerdir mount options only contains regular lower
> +        *       layers ":".
> +        *       => Nothing to verify.
> +        * (4.2) The lowerdir mount options contains regular ":" and
> +        *       data "::" layers.
> +        *       => We need to verify that data lower layers "::" aren't
> +        *          followed by regular ":" lower layers
> +        *   (5) If ctx->nr > 0 => append
> +        *       We know that there's at least one regular layer
> +        *       otherwise we would've failed when parsing the previous
> +        *       lowerdir mount option.
> +        * (5.1) The lowerdir mount option is a regular layer ":" append
> +        *       => We need to verify that no data layers have been
> +        *          specified before.
> +        * (5.2) The lowerdir mount option is a data layer "::" append
> +        *       We know that there's at least one regular layer or
> +        *       other data layers. => There's nothing to verify.
> +        */
> +       dup_iter = dup;
> +       for (nr = ctx->nr; nr < nr_lower; nr++) {
> +               l = &ctx->lower[nr];

missing here:
                   memset(l, 0, sizeof(*l));

otherwise, when trying to mount ovl over illegal fs (vfat)...

> +
> +               err = ovl_mount_dir_noesc(dup_iter, &l->path);
> +               if (err)
> +                       goto out_put;
> +
> +               err = -ENOMEM;
> +               l->name = kstrdup(dup_iter, GFP_KERNEL_ACCOUNT);
> +               if (!l->name)
> +                       goto out_put;
> +
> +               if (data_layer)
> +                       nr_data++;
> +
> +               /* Calling strchr() again would overrun. */
> +               if ((nr + 1) == nr_lower)
> +                       break;
> +
> +               err = -EINVAL;
> +               dup_iter = strchr(dup_iter, '\0') + 1;
> +               if (*dup_iter) {
> +                       /*
> +                        * This is a regular layer so we require that
> +                        * there are no data layers.
> +                        */
> +                       if ((ctx->nr_data + nr_data) > 0) {
> +                               pr_err("regular lower layers cannot follow data lower layers");
> +                               goto out_put;
> +                       }
> +
> +                       data_layer = false;
> +                       continue;
> +               }
> +
> +               /* This is a data lower layer. */
> +               data_layer = true;
> +               dup_iter++;
> +       }
> +       ctx->nr = nr_lower;
> +       ctx->nr_data += nr_data;
> +       kfree(dup);
> +       return 0;
> +
> +out_put:
> +       /*
> +        * We know nr >= ctx->nr < nr_lower. If we failed somewhere
> +        * we want to undo until nr == ctx->nr. This is correct for
> +        * both ctx->nr == 0 and ctx->nr > 0.
> +        */
> +       for (; nr >= ctx->nr; nr--) {
> +               l = &ctx->lower[nr];
> +               kfree(l->name);
> +               l->name = NULL;
> +               path_put(&l->path);
> +

...this is kfreeing a garbage pointer.

I will fold that into my overlayfs-next branch.

Thanks,
Amir.

Re: [PATCH v3 0/3] ovl: port to new mount api & updated layer parsing

[Amir Goldstein]

On Tue, Jun 13, 2023 at 5:49 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Hey everyone,
>
> /* v3 */
> (1) Add a patch that makes sure that struct ovl_layer starts with a
>     pointer to struct vfsmount as ovl_free_fs() relies on this during
>     cleanup.
> (2) Rebase on top of overlayfs-next brought in substantial rework due to
>     the data layer work. I had to redo a bunch of the parsing and adapt
>     it to the new changes.

Sorry for that pain.
I do like your version much better than mine :)

For the entire series:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

>
> This ports overlayfs to the new mount api and modifies layer parsing to
> allow for additive layer specification via fsconfig().
>
> This passes xfstests including the new data layer lookup work. Here's a
> 500 layer overlayfs mount including a couple of data only layers:
>
> 1094 28 0:48 / /mnt/test rw,relatime shared:214 - overlay none rw,lowerdir=/mnt/ovl-lower1:/mnt/ovl-lower2:/mnt/ovl-lower3:/mnt/ovl-lower4:/mnt/ovl-lower5:/mnt/ovl-lower6:/mnt/ovl-lower7:/mnt/ovl-lower8:/mnt/ovl-lower9:/mnt/ovl-lower10:/mnt/ovl-lower11:/mnt/ovl-lower12:/mnt/ovl-lower13:/mnt/ovl-lower14:/mnt/ovl-lower15:/mnt/ovl-lower16:/mnt/ovl-lower17:/mnt/ovl-lower18:/mnt/ovl-lower19:/mnt/ovl-lower20:/mnt/ovl-lower21:/mnt/ovl-lower22:/mnt/ovl-lower23:/mnt/ovl-lower24:/mnt/ovl-lower25:/mnt/ovl-lower26:/mnt/ovl-lower27:/mnt/ovl-lower28:/mnt/ovl-lower29:/mnt/ovl-lower30:/mnt/ovl-lower31:/mnt/ovl-lower32:/mnt/ovl-lower33:/mnt/ovl-lower34:/mnt/ovl-lower35:/mnt/ovl-lower36:/mnt/ovl-lower37:/mnt/ovl-lower38:/mnt/ovl-lower39:/mnt/ovl-lower40:/mnt/ovl-lower41:/mnt/ovl-lower42:/mnt/ovl-lower43:/mnt/ovl-lower44:/mnt/ovl-lower45:/mnt/ovl-lower46:/mnt/ovl-lower47:/mnt/ovl-lower48:/mnt/ovl-lower49:/mnt/ovl-lower50:/mnt/ovl-lower51:/mnt/ovl-lower52:/mnt/ovl-lower53:/mnt/ovl-lower54:/mnt/ovl-lower
>  55:/mnt/ovl-lower56:/mnt/ovl-lower57:/mnt/ovl-lower58:/mnt/ovl-lower59:/mnt/ovl-lower60:/mnt/ovl-lower61:/mnt/ovl-lower62:/mnt/ovl-lower63:/mnt/ovl-lower64:/mnt/ovl-lower65:/mnt/ovl-lower66:/mnt/ovl-lower67:/mnt/ovl-lower68:/mnt/ovl-lower69:/mnt/ovl-lower70:/mnt/ovl-lower71:/mnt/ovl-lower72:/mnt/ovl-lower73:/mnt/ovl-lower74:/mnt/ovl-lower75:/mnt/ovl-lower76:/mnt/ovl-lower77:/mnt/ovl-lower78:/mnt/ovl-lower79:/mnt/ovl-lower80:/mnt/ovl-lower81:/mnt/ovl-lower82:/mnt/ovl-lower83:/mnt/ovl-lower84:/mnt/ovl-lower85:/mnt/ovl-lower86:/mnt/ovl-lower87:/mnt/ovl-lower88:/mnt/ovl-lower89:/mnt/ovl-lower90:/mnt/ovl-lower91:/mnt/ovl-lower92:/mnt/ovl-lower93:/mnt/ovl-lower94:/mnt/ovl-lower95:/mnt/ovl-lower96:/mnt/ovl-lower97:/mnt/ovl-lower98:/mnt/ovl-lower99:/mnt/ovl-lower100:/mnt/ovl-lower101:/mnt/ovl-lower102:/mnt/ovl-lower103:/mnt/ovl-lower104:/mnt/ovl-lower105:/mnt/ovl-lower106:/mnt/ovl-lower107:/mnt/ovl-lower108:/mnt/ovl-lower109:/mnt/ovl-lower110:/mnt/ovl-lower111:/mnt/ovl-lower112:/mnt/ovl-low
>  er113:/mnt/ovl-lower114:/mnt/ovl-lower115:/mnt/ovl-lower116:/mnt/ovl-lower117:/mnt/ovl-lower118:/mnt/ovl-lower119:/mnt/ovl-lower120:/mnt/ovl-lower121:/mnt/ovl-lower122:/mnt/ovl-lower123:/mnt/ovl-lower124:/mnt/ovl-lower125:/mnt/ovl-lower126:/mnt/ovl-lower127:/mnt/ovl-lower128:/mnt/ovl-lower129:/mnt/ovl-lower130:/mnt/ovl-lower131:/mnt/ovl-lower132:/mnt/ovl-lower133:/mnt/ovl-lower134:/mnt/ovl-lower135:/mnt/ovl-lower136:/mnt/ovl-lower137:/mnt/ovl-lower138:/mnt/ovl-lower139:/mnt/ovl-lower140:/mnt/ovl-lower141:/mnt/ovl-lower142:/mnt/ovl-lower143:/mnt/ovl-lower144:/mnt/ovl-lower145:/mnt/ovl-lower146:/mnt/ovl-lower147:/mnt/ovl-lower148:/mnt/ovl-lower149:/mnt/ovl-lower150:/mnt/ovl-lower151:/mnt/ovl-lower152:/mnt/ovl-lower153:/mnt/ovl-lower154:/mnt/ovl-lower155:/mnt/ovl-lower156:/mnt/ovl-lower157:/mnt/ovl-lower158:/mnt/ovl-lower159:/mnt/ovl-lower160:/mnt/ovl-lower161:/mnt/ovl-lower162:/mnt/ovl-lower163:/mnt/ovl-lower164:/mnt/ovl-lower165:/mnt/ovl-lower166:/mnt/ovl-lower167:/mnt/ovl-lower168:/
>  mnt/ovl-lower169:/mnt/ovl-lower170:/mnt/ovl-lower171:/mnt/ovl-lower172:/mnt/ovl-lower173:/mnt/ovl-lower174:/mnt/ovl-lower175:/mnt/ovl-lower176:/mnt/ovl-lower177:/mnt/ovl-lower178:/mnt/ovl-lower179:/mnt/ovl-lower180:/mnt/ovl-lower181:/mnt/ovl-lower182:/mnt/ovl-lower183:/mnt/ovl-lower184:/mnt/ovl-lower185:/mnt/ovl-lower186:/mnt/ovl-lower187:/mnt/ovl-lower188:/mnt/ovl-lower189:/mnt/ovl-lower190:/mnt/ovl-lower191:/mnt/ovl-lower192:/mnt/ovl-lower193:/mnt/ovl-lower194:/mnt/ovl-lower195:/mnt/ovl-lower196:/mnt/ovl-lower197:/mnt/ovl-lower198:/mnt/ovl-lower199:/mnt/ovl-lower200:/mnt/ovl-lower201:/mnt/ovl-lower202:/mnt/ovl-lower203:/mnt/ovl-lower204:/mnt/ovl-lower205:/mnt/ovl-lower206:/mnt/ovl-lower207:/mnt/ovl-lower208:/mnt/ovl-lower209:/mnt/ovl-lower210:/mnt/ovl-lower211:/mnt/ovl-lower212:/mnt/ovl-lower213:/mnt/ovl-lower214:/mnt/ovl-lower215:/mnt/ovl-lower216:/mnt/ovl-lower217:/mnt/ovl-lower218:/mnt/ovl-lower219:/mnt/ovl-lower220:/mnt/ovl-lower221:/mnt/ovl-lower222:/mnt/ovl-lower223:/mnt/ovl
>  -lower224:/mnt/ovl-lower225:/mnt/ovl-lower226:/mnt/ovl-lower227:/mnt/ovl-lower228:/mnt/ovl-lower229:/mnt/ovl-lower230:/mnt/ovl-lower231:/mnt/ovl-lower232:/mnt/ovl-lower233:/mnt/ovl-lower234:/mnt/ovl-lower235:/mnt/ovl-lower236:/mnt/ovl-lower237:/mnt/ovl-lower238:/mnt/ovl-lower239:/mnt/ovl-lower240:/mnt/ovl-lower241:/mnt/ovl-lower242:/mnt/ovl-lower243:/mnt/ovl-lower244:/mnt/ovl-lower245:/mnt/ovl-lower246:/mnt/ovl-lower247:/mnt/ovl-lower248:/mnt/ovl-lower249:/mnt/ovl-lower250:/mnt/ovl-lower251:/mnt/ovl-lower252:/mnt/ovl-lower253:/mnt/ovl-lower254:/mnt/ovl-lower255:/mnt/ovl-lower256:/mnt/ovl-lower257:/mnt/ovl-lower258:/mnt/ovl-lower259:/mnt/ovl-lower260:/mnt/ovl-lower261:/mnt/ovl-lower262:/mnt/ovl-lower263:/mnt/ovl-lower264:/mnt/ovl-lower265:/mnt/ovl-lower266:/mnt/ovl-lower267:/mnt/ovl-lower268:/mnt/ovl-lower269:/mnt/ovl-lower270:/mnt/ovl-lower271:/mnt/ovl-lower272:/mnt/ovl-lower273:/mnt/ovl-lower274:/mnt/ovl-lower275:/mnt/ovl-lower276:/mnt/ovl-lower277:/mnt/ovl-lower278:/mnt/ovl-lower2
>  79:/mnt/ovl-lower280:/mnt/ovl-lower281:/mnt/ovl-lower282:/mnt/ovl-lower283:/mnt/ovl-lower284:/mnt/ovl-lower285:/mnt/ovl-lower286:/mnt/ovl-lower287:/mnt/ovl-lower288:/mnt/ovl-lower289:/mnt/ovl-lower290:/mnt/ovl-lower291:/mnt/ovl-lower292:/mnt/ovl-lower293:/mnt/ovl-lower294:/mnt/ovl-lower295:/mnt/ovl-lower296:/mnt/ovl-lower297:/mnt/ovl-lower298:/mnt/ovl-lower299:/mnt/ovl-lower300:/mnt/ovl-lower301:/mnt/ovl-lower302:/mnt/ovl-lower303:/mnt/ovl-lower304:/mnt/ovl-lower305:/mnt/ovl-lower306:/mnt/ovl-lower307:/mnt/ovl-lower308:/mnt/ovl-lower309:/mnt/ovl-lower310:/mnt/ovl-lower311:/mnt/ovl-lower312:/mnt/ovl-lower313:/mnt/ovl-lower314:/mnt/ovl-lower315:/mnt/ovl-lower316:/mnt/ovl-lower317:/mnt/ovl-lower318:/mnt/ovl-lower319:/mnt/ovl-lower320:/mnt/ovl-lower321:/mnt/ovl-lower322:/mnt/ovl-lower323:/mnt/ovl-lower324:/mnt/ovl-lower325:/mnt/ovl-lower326:/mnt/ovl-lower327:/mnt/ovl-lower328:/mnt/ovl-lower329:/mnt/ovl-lower330:/mnt/ovl-lower331:/mnt/ovl-lower332:/mnt/ovl-lower333:/mnt/ovl-lower334:/mnt
>  /ovl-lower335:/mnt/ovl-lower336:/mnt/ovl-lower337:/mnt/ovl-lower338:/mnt/ovl-lower339:/mnt/ovl-lower340:/mnt/ovl-lower341:/mnt/ovl-lower342:/mnt/ovl-lower343:/mnt/ovl-lower344:/mnt/ovl-lower345:/mnt/ovl-lower346:/mnt/ovl-lower347:/mnt/ovl-lower348:/mnt/ovl-lower349:/mnt/ovl-lower350:/mnt/ovl-lower351:/mnt/ovl-lower352:/mnt/ovl-lower353:/mnt/ovl-lower354:/mnt/ovl-lower355:/mnt/ovl-lower356:/mnt/ovl-lower357:/mnt/ovl-lower358:/mnt/ovl-lower359:/mnt/ovl-lower360:/mnt/ovl-lower361:/mnt/ovl-lower362:/mnt/ovl-lower363:/mnt/ovl-lower364:/mnt/ovl-lower365:/mnt/ovl-lower366:/mnt/ovl-lower367:/mnt/ovl-lower368:/mnt/ovl-lower369:/mnt/ovl-lower370:/mnt/ovl-lower371:/mnt/ovl-lower372:/mnt/ovl-lower373:/mnt/ovl-lower374:/mnt/ovl-lower375:/mnt/ovl-lower376:/mnt/ovl-lower377:/mnt/ovl-lower378:/mnt/ovl-lower379:/mnt/ovl-lower380:/mnt/ovl-lower381:/mnt/ovl-lower382:/mnt/ovl-lower383:/mnt/ovl-lower384:/mnt/ovl-lower385:/mnt/ovl-lower386:/mnt/ovl-lower387:/mnt/ovl-lower388:/mnt/ovl-lower389:/mnt/ovl-lo
>  wer390:/mnt/ovl-lower391:/mnt/ovl-lower392:/mnt/ovl-lower393:/mnt/ovl-lower394:/mnt/ovl-lower395:/mnt/ovl-lower396:/mnt/ovl-lower397:/mnt/ovl-lower398:/mnt/ovl-lower399:/mnt/ovl-lower400:/mnt/ovl-lower401:/mnt/ovl-lower402:/mnt/ovl-lower403:/mnt/ovl-lower404:/mnt/ovl-lower405:/mnt/ovl-lower406:/mnt/ovl-lower407:/mnt/ovl-lower408:/mnt/ovl-lower409:/mnt/ovl-lower410:/mnt/ovl-lower411:/mnt/ovl-lower412:/mnt/ovl-lower413:/mnt/ovl-lower414:/mnt/ovl-lower415:/mnt/ovl-lower416:/mnt/ovl-lower417:/mnt/ovl-lower418:/mnt/ovl-lower419:/mnt/ovl-lower420:/mnt/ovl-lower421:/mnt/ovl-lower422:/mnt/ovl-lower423:/mnt/ovl-lower424:/mnt/ovl-lower425:/mnt/ovl-lower426:/mnt/ovl-lower427:/mnt/ovl-lower428:/mnt/ovl-lower429:/mnt/ovl-lower430:/mnt/ovl-lower431:/mnt/ovl-lower432:/mnt/ovl-lower433:/mnt/ovl-lower434:/mnt/ovl-lower435:/mnt/ovl-lower436:/mnt/ovl-lower437:/mnt/ovl-lower438:/mnt/ovl-lower439:/mnt/ovl-lower440:/mnt/ovl-lower441:/mnt/ovl-lower442:/mnt/ovl-lower443:/mnt/ovl-lower444:/mnt/ovl-lower445:
>  /mnt/ovl-lower446:/mnt/ovl-lower447:/mnt/ovl-lower448:/mnt/ovl-lower449:/mnt/ovl-lower450:/mnt/ovl-lower451:/mnt/ovl-lower452:/mnt/ovl-lower453:/mnt/ovl-lower454:/mnt/ovl-lower455:/mnt/ovl-lower456:/mnt/ovl-lower457:/mnt/ovl-lower458:/mnt/ovl-lower459:/mnt/ovl-lower460:/mnt/ovl-lower461:/mnt/ovl-lower462:/mnt/ovl-lower463:/mnt/ovl-lower464:/mnt/ovl-lower465:/mnt/ovl-lower466:/mnt/ovl-lower467:/mnt/ovl-lower468:/mnt/ovl-lower469:/mnt/ovl-lower470:/mnt/ovl-lower471:/mnt/ovl-lower472:/mnt/ovl-lower473:/mnt/ovl-lower474:/mnt/ovl-lower475:/mnt/ovl-lower476:/mnt/ovl-lower477:/mnt/ovl-lower478:/mnt/ovl-lower479:/mnt/ovl-lower480:/mnt/ovl-lower481:/mnt/ovl-lower482:/mnt/ovl-lower483:/mnt/ovl-lower484:/mnt/ovl-lower485:/mnt/ovl-lower486:/mnt/ovl-lower487:/mnt/ovl-lower488:/mnt/ovl-lower489:/mnt/ovl-lower490:/mnt/ovl-lower491:/mnt/ovl-lower492:/mnt/ovl-lower493:/mnt/ovl-lower494:/mnt/ovl-lower495:/mnt/ovl-lower496::/mnt/ovl-lower497::/mnt/ovl-lower498::/mnt/ovl-lower499::/mnt/ovl-lower500,upp
>  erdir=/mnt/ovl-upper,workdir=/mnt/ovl-work,redirect_dir=on,index=on,metacopy=on
>
> Christian
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
> Changes in v3:
> - Fix potential NULL deref in ovl_init_fs_context().
> - Don't create one long mount option string for lowerdir. Just use
>   seq_printf() to create it on demand.
> - Link to v2: https://lore.kernel.org/r/20230605-fs-overlayfs-mount_api-v2-0-3da91c97e0c0@kernel.org
>
> Changes in v2:
> - Split into two patches. First patch ports to new mount api without changing
>   layer parsing. Second patch implements new layer parsing.
> - Move layer parsing into a separate file.
> - Link to v1: https://lore.kernel.org/r/20230605-fs-overlayfs-mount_api-v1-1-a8d78c3fbeaf@kernel.org
>
> ---
>
>
>
> ---
> base-commit: f777c8266c1230fa44de7261b61a13d787b27f44
> change-id: 20230605-fs-overlayfs-mount_api-20ea8b04eff4
>

Re: [PATCH v3 0/3] ovl: port to new mount api & updated layer parsing

[Amir Goldstein]

On Tue, Jun 13, 2023 at 6:02 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, Jun 13, 2023 at 5:49 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > Hey everyone,
> >
> > /* v3 */
> > (1) Add a patch that makes sure that struct ovl_layer starts with a
> >     pointer to struct vfsmount as ovl_free_fs() relies on this during
> >     cleanup.
> > (2) Rebase on top of overlayfs-next brought in substantial rework due to
> >     the data layer work. I had to redo a bunch of the parsing and adapt
> >     it to the new changes.
>
> Sorry for that pain.
> I do like your version much better than mine :)
>
> For the entire series:
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>

Miklos,

If you like, I prepared a pull-able branch at:
https://github.com/amir73il/linux/commits/fs-overlayfs-mount_api

With some more cleanups.
I will post the cleanup patches soon.

The branch above already passed basic sanity tests.
I will run more tests on it, but if you can to push it
to overlayfs-next for soaking, that would be great.

Thanks,
Amir.