[RFC][PATCH 0/3] Prepare for supporting more filesystems with fanotify

[RFC][PATCH 0/3] Prepare for supporting more filesystems with fanotify

[Amir Goldstein]

Miklos,

This is the second part of the proposal to support fanotify reporing
file ids on overlayfs.

The first part [1] relaxes the requirements for filesystems to support
reporting events with fid to require only the ->encode_fh() operation.

In this patch set, overlayfs is changed to meet the new requirements
with default overlay configurations (i.e. no need for nfs_export=on).

The overlayfs and vfs/fanotify patch sets are completely independent.
The are both available on my github branch [2] and there is a simple
LTP test variant that tests reporting fid from overlayfs [3].

Patches 2-3 are not really needed to support reporting fanotify events
with fid, because overlayfs already reports a non-zero f_fsid, it's just
not a very good fsid.  So before allowing to report overlayfs fids, I
prefer to fix f_fsid to be more unique.

I went back and forth about this change of behavior (from real fsid to
per-instance fsid) - can we make this change without breaking any
existing workloads? I don't know.

For now, I added a mount option with a very lousy name (uuid=nogen)
as an escape hatch, but I also left a const bool ovl_uuid_gen_def var
that we can wire to a Kconfig/module option if that is desired.

Thanks,
Amir.

[1] https://lore.kernel.org/linux-fsdevel/20230425130105.2606684-1-amir73il@gmail.com/
[2] https://github.com/amir73il/linux/commits/exportfs_encode_fid
[3] https://github.com/amir73il/ltp/commits/exportfs_encode_fid

Amir Goldstein (3):
  ovl: support encoding non-decodeable file handles
  ovl: report a per-instance f_fsid by default
  ovl: use persistent s_uuid with index=on

 Documentation/filesystems/overlayfs.rst | 10 ++--
 fs/overlayfs/export.c                   | 26 ++++++++---
 fs/overlayfs/inode.c                    |  2 +-
 fs/overlayfs/overlayfs.h                | 10 ++++
 fs/overlayfs/ovl_entry.h                |  3 +-
 fs/overlayfs/super.c                    | 62 ++++++++++++++++++++++---
 fs/overlayfs/util.c                     | 41 ++++++++++++++++
 7 files changed, 135 insertions(+), 19 deletions(-)

-- 
2.34.1

[RFC][PATCH 1/3] ovl: support encoding non-decodeable file handles

[Amir Goldstein]

When all layers support file handles, we support encoding non-decodeable
file handles (a.k.a. fid) even with nfs_export=off.

When file handles do not need to be decoded, we do not need to copy up
redirected lower directories on encode, and we encode also non-indexed
upper with lower file handle, so fid will not change on copy up.

This enables reporting fanotify events with file handles on overlayfs
with default config/mount options.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/export.c    | 26 ++++++++++++++++++++------
 fs/overlayfs/inode.c     |  2 +-
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/ovl_entry.h |  1 +
 fs/overlayfs/super.c     |  9 +++++++++
 5 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
index defd4e231ad2..dfd05ad2b722 100644
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -173,28 +173,37 @@ static int ovl_connect_layer(struct dentry *dentry)
  * U = upper file handle
  * L = lower file handle
  *
- * (*) Connecting an overlay dir from real lower dentry is not always
+ * (*) Decoding a connected overlay dir from real lower dentry is not always
  * possible when there are redirects in lower layers and non-indexed merge dirs.
  * To mitigate those case, we may copy up the lower dir ancestor before encode
- * a lower dir file handle.
+ * of a decodeable file handle for non-upper dir.
  *
  * Return 0 for upper file handle, > 0 for lower file handle or < 0 on error.
  */
 static int ovl_check_encode_origin(struct dentry *dentry)
 {
 	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+	bool decodeable = ofs->config.nfs_export;
+
+	/* Lower file handle for non-upper non-decodeable */
+	if (!ovl_dentry_upper(dentry) && !decodeable)
+		return 0;
 
 	/* Upper file handle for pure upper */
 	if (!ovl_dentry_lower(dentry))
 		return 0;
 
 	/*
-	 * Upper file handle for non-indexed upper.
-	 *
 	 * Root is never indexed, so if there's an upper layer, encode upper for
 	 * root.
 	 */
-	if (ovl_dentry_upper(dentry) &&
+	if (dentry == dentry->d_sb->s_root)
+		return 0;
+
+	/*
+	 * Upper decodeable file handle for non-indexed upper.
+	 */
+	if (ovl_dentry_upper(dentry) && decodeable &&
 	    !ovl_test_flag(OVL_INDEX, d_inode(dentry)))
 		return 0;
 
@@ -204,7 +213,7 @@ static int ovl_check_encode_origin(struct dentry *dentry)
 	 * ovl_connect_layer() will try to make origin's layer "connected" by
 	 * copying up a "connectable" ancestor.
 	 */
-	if (d_is_dir(dentry) && ovl_upper_mnt(ofs))
+	if (d_is_dir(dentry) && ovl_upper_mnt(ofs) && decodeable)
 		return ovl_connect_layer(dentry);
 
 	/* Lower file handle for indexed and non-upper dir/non-dir */
@@ -875,3 +884,8 @@ const struct export_operations ovl_export_operations = {
 	.get_name	= ovl_get_name,
 	.get_parent	= ovl_get_parent,
 };
+
+/* encode_fh() encodes non-decodeable file handles with nfs_export=off */
+const struct export_operations ovl_export_fid_operations = {
+	.encode_fh	= ovl_encode_fh,
+};
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 541cf3717fc2..b6bec4064390 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -1304,7 +1304,7 @@ static bool ovl_hash_bylower(struct super_block *sb, struct dentry *upper,
 		return false;
 
 	/* No, if non-indexed upper with NFS export */
-	if (sb->s_export_op && upper)
+	if (ofs->config.nfs_export && upper)
 		return false;
 
 	/* Otherwise, hash by lower inode for fsnotify */
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 4d0b278f5630..87d44b889129 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -734,3 +734,4 @@ int ovl_set_origin(struct ovl_fs *ofs, struct dentry *lower,
 
 /* export.c */
 extern const struct export_operations ovl_export_operations;
+extern const struct export_operations ovl_export_fid_operations;
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index fd11fe6d6d45..5cc0b6e65488 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -67,6 +67,7 @@ struct ovl_fs {
 	const struct cred *creator_cred;
 	bool tmpfile;
 	bool noxattr;
+	bool nofh;
 	/* Did we take the inuse lock? */
 	bool upperdir_locked;
 	bool workdir_locked;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index f1d9f75f8786..5ed8c2650293 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -954,6 +954,7 @@ static int ovl_lower_dir(const char *name, struct path *path,
 		pr_warn("fs on '%s' does not support file handles, falling back to index=off,nfs_export=off.\n",
 			name);
 	}
+	ofs->nofh |= !fh_type;
 	/*
 	 * Decoding origin file handle is required for persistent st_ino.
 	 * Without persistent st_ino, xino=auto falls back to xino=off.
@@ -1391,6 +1392,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
 		ofs->config.index = false;
 		pr_warn("upper fs does not support file handles, falling back to index=off.\n");
 	}
+	ofs->nofh |= !fh_type;
 
 	/* Check if upper fs has 32bit inode numbers */
 	if (fh_type != FILEID_INO32_GEN)
@@ -2049,8 +2051,15 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		ofs->config.nfs_export = false;
 	}
 
+	/*
+	 * Support encoding decodeable file handles with nfs_export=on
+	 * and encoding non-decodeable file handles with nfs_export=off
+	 * if all layers support file handles.
+	 */
 	if (ofs->config.nfs_export)
 		sb->s_export_op = &ovl_export_operations;
+	else if (!ofs->nofh)
+		sb->s_export_op = &ovl_export_fid_operations;
 
 	/* Never override disk quota limits or use reserved space */
 	cap_lower(cred->cap_effective, CAP_SYS_RESOURCE);
-- 
2.34.1

Re: [RFC][PATCH 1/3] ovl: support encoding non-decodeable file handles

[Amir Goldstein]

On Tue, Apr 25, 2023 at 4:22 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> When all layers support file handles, we support encoding non-decodeable
> file handles (a.k.a. fid) even with nfs_export=off.
>
> When file handles do not need to be decoded, we do not need to copy up
> redirected lower directories on encode, and we encode also non-indexed
> upper with lower file handle, so fid will not change on copy up.
>
> This enables reporting fanotify events with file handles on overlayfs
> with default config/mount options.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

FYI, the exportfs support for non-decodable file handles has been merged.

This follow up series which adds support for non-decodable file handles
to overlayfs has been pushed to:
https://github.com/amir73il/linux/commits/ovl_encode_fid
and to overlayfs-next (pending review by Miklos).

fanotify (over ovl) tests are available at:
https://github.com/amir73il/ltp/commits/ovl_encode_fid

Thanks,
Amir.

>  fs/overlayfs/export.c    | 26 ++++++++++++++++++++------
>  fs/overlayfs/inode.c     |  2 +-
>  fs/overlayfs/overlayfs.h |  1 +
>  fs/overlayfs/ovl_entry.h |  1 +
>  fs/overlayfs/super.c     |  9 +++++++++
>  5 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
> index defd4e231ad2..dfd05ad2b722 100644
> --- a/fs/overlayfs/export.c
> +++ b/fs/overlayfs/export.c
> @@ -173,28 +173,37 @@ static int ovl_connect_layer(struct dentry *dentry)
>   * U = upper file handle
>   * L = lower file handle
>   *
> - * (*) Connecting an overlay dir from real lower dentry is not always
> + * (*) Decoding a connected overlay dir from real lower dentry is not always
>   * possible when there are redirects in lower layers and non-indexed merge dirs.
>   * To mitigate those case, we may copy up the lower dir ancestor before encode
> - * a lower dir file handle.
> + * of a decodeable file handle for non-upper dir.
>   *
>   * Return 0 for upper file handle, > 0 for lower file handle or < 0 on error.
>   */
>  static int ovl_check_encode_origin(struct dentry *dentry)
>  {
>         struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
> +       bool decodeable = ofs->config.nfs_export;
> +
> +       /* Lower file handle for non-upper non-decodeable */
> +       if (!ovl_dentry_upper(dentry) && !decodeable)
> +               return 0;
>
>         /* Upper file handle for pure upper */
>         if (!ovl_dentry_lower(dentry))
>                 return 0;
>
>         /*
> -        * Upper file handle for non-indexed upper.
> -        *
>          * Root is never indexed, so if there's an upper layer, encode upper for
>          * root.
>          */
> -       if (ovl_dentry_upper(dentry) &&
> +       if (dentry == dentry->d_sb->s_root)
> +               return 0;
> +
> +       /*
> +        * Upper decodeable file handle for non-indexed upper.
> +        */
> +       if (ovl_dentry_upper(dentry) && decodeable &&
>             !ovl_test_flag(OVL_INDEX, d_inode(dentry)))
>                 return 0;
>
> @@ -204,7 +213,7 @@ static int ovl_check_encode_origin(struct dentry *dentry)
>          * ovl_connect_layer() will try to make origin's layer "connected" by
>          * copying up a "connectable" ancestor.
>          */
> -       if (d_is_dir(dentry) && ovl_upper_mnt(ofs))
> +       if (d_is_dir(dentry) && ovl_upper_mnt(ofs) && decodeable)
>                 return ovl_connect_layer(dentry);
>
>         /* Lower file handle for indexed and non-upper dir/non-dir */
> @@ -875,3 +884,8 @@ const struct export_operations ovl_export_operations = {
>         .get_name       = ovl_get_name,
>         .get_parent     = ovl_get_parent,
>  };
> +
> +/* encode_fh() encodes non-decodeable file handles with nfs_export=off */
> +const struct export_operations ovl_export_fid_operations = {
> +       .encode_fh      = ovl_encode_fh,
> +};
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 541cf3717fc2..b6bec4064390 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -1304,7 +1304,7 @@ static bool ovl_hash_bylower(struct super_block *sb, struct dentry *upper,
>                 return false;
>
>         /* No, if non-indexed upper with NFS export */
> -       if (sb->s_export_op && upper)
> +       if (ofs->config.nfs_export && upper)
>                 return false;
>
>         /* Otherwise, hash by lower inode for fsnotify */
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 4d0b278f5630..87d44b889129 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -734,3 +734,4 @@ int ovl_set_origin(struct ovl_fs *ofs, struct dentry *lower,
>
>  /* export.c */
>  extern const struct export_operations ovl_export_operations;
> +extern const struct export_operations ovl_export_fid_operations;
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index fd11fe6d6d45..5cc0b6e65488 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -67,6 +67,7 @@ struct ovl_fs {
>         const struct cred *creator_cred;
>         bool tmpfile;
>         bool noxattr;
> +       bool nofh;
>         /* Did we take the inuse lock? */
>         bool upperdir_locked;
>         bool workdir_locked;
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index f1d9f75f8786..5ed8c2650293 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -954,6 +954,7 @@ static int ovl_lower_dir(const char *name, struct path *path,
>                 pr_warn("fs on '%s' does not support file handles, falling back to index=off,nfs_export=off.\n",
>                         name);
>         }
> +       ofs->nofh |= !fh_type;
>         /*
>          * Decoding origin file handle is required for persistent st_ino.
>          * Without persistent st_ino, xino=auto falls back to xino=off.
> @@ -1391,6 +1392,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
>                 ofs->config.index = false;
>                 pr_warn("upper fs does not support file handles, falling back to index=off.\n");
>         }
> +       ofs->nofh |= !fh_type;
>
>         /* Check if upper fs has 32bit inode numbers */
>         if (fh_type != FILEID_INO32_GEN)
> @@ -2049,8 +2051,15 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>                 ofs->config.nfs_export = false;
>         }
>
> +       /*
> +        * Support encoding decodeable file handles with nfs_export=on
> +        * and encoding non-decodeable file handles with nfs_export=off
> +        * if all layers support file handles.
> +        */
>         if (ofs->config.nfs_export)
>                 sb->s_export_op = &ovl_export_operations;
> +       else if (!ofs->nofh)
> +               sb->s_export_op = &ovl_export_fid_operations;
>
>         /* Never override disk quota limits or use reserved space */
>         cap_lower(cred->cap_effective, CAP_SYS_RESOURCE);
> --
> 2.34.1
>

[RFC][PATCH 2/3] ovl: report a per-instance f_fsid by default

[Amir Goldstein]

ovl_statfs() reports the f_fsid filled by underlying upper fs.
This fsid is not unique among overlayfs instances on the same upper fs.

Generate a non-persistent uuid per overlayfs instance and use it as the
seed for f_fsid, similar to tmpfs instance uuid/fsid.

The old behavior can be restored with mount option uuid=nogen.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 Documentation/filesystems/overlayfs.rst | 10 +++---
 fs/overlayfs/overlayfs.h                |  6 ++++
 fs/overlayfs/ovl_entry.h                |  2 +-
 fs/overlayfs/super.c                    | 46 +++++++++++++++++++++----
 4 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
index 4c76fda07645..ad48ae9d3c43 100644
--- a/Documentation/filesystems/overlayfs.rst
+++ b/Documentation/filesystems/overlayfs.rst
@@ -571,10 +571,12 @@ Note: the mount options index=off,nfs_export=on are conflicting for a
 read-write mount and will result in an error.
 
 Note: the mount option uuid=off can be used to replace UUID of the underlying
-filesystem in file handles with null, and effectively disable UUID checks. This
-can be useful in case the underlying disk is copied and the UUID of this copy
-is changed. This is only applicable if all lower/upper/work directories are on
-the same filesystem, otherwise it will fallback to normal behaviour.
+filesystem in file handles with null, and effectively disable UUID checks.
+This can be useful in case the underlying disk is copied and the UUID of this
+copy is changed.  This is only applicable if all lower/upper/work directories
+are on the same filesystem, otherwise it will fallback to normal behaviour.
+The mount option uuid=nogen can be used to disable UUID generation for the
+overlay filesystem itself.  The two mount options are mutually exclusive.
 
 Volatile mount
 --------------
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 87d44b889129..dcdb02d0ddf8 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -63,6 +63,12 @@ enum {
 	OVL_XINO_ON,
 };
 
+enum {
+	OVL_UUID_OFF,
+	OVL_UUID_NOGEN,
+	OVL_UUID_ON,
+};
+
 /*
  * The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
  * where:
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 5cc0b6e65488..4f396b1ce2fb 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -14,7 +14,7 @@ struct ovl_config {
 	bool redirect_follow;
 	const char *redirect_mode;
 	bool index;
-	bool uuid;
+	int uuid;
 	bool nfs_export;
 	int xino;
 	bool metacopy;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 5ed8c2650293..ad2250f98b38 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -317,6 +317,7 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
 {
 	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
 	struct dentry *root_dentry = dentry->d_sb->s_root;
+	uuid_t *uuid = &dentry->d_sb->s_uuid;
 	struct path path;
 	int err;
 
@@ -326,6 +327,8 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
 	if (!err) {
 		buf->f_namelen = ofs->namelen;
 		buf->f_type = OVERLAYFS_SUPER_MAGIC;
+		if (!uuid_is_null(uuid))
+			buf->f_fsid = uuid_to_fsid(uuid->b);
 	}
 
 	return err;
@@ -353,6 +356,25 @@ static inline int ovl_xino_def(void)
 	return ovl_xino_auto_def ? OVL_XINO_AUTO : OVL_XINO_OFF;
 }
 
+static const char * const ovl_uuid_str[] = {
+	"off",
+	"nogen",
+	"on",
+};
+
+/* XXX: do we need a config for this? */
+static const bool ovl_uuid_gen_def = true;
+
+static inline int ovl_uuid_def(void)
+{
+	return ovl_uuid_gen_def ? OVL_UUID_ON : OVL_UUID_NOGEN;
+}
+
+static inline int ovl_want_uuid_gen(struct ovl_fs *ofs)
+{
+	return ofs->config.uuid != OVL_UUID_NOGEN;
+}
+
 /**
  * ovl_show_options
  * @m: the seq_file handle
@@ -377,8 +399,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 		seq_printf(m, ",redirect_dir=%s", ofs->config.redirect_mode);
 	if (ofs->config.index != ovl_index_def)
 		seq_printf(m, ",index=%s", ofs->config.index ? "on" : "off");
-	if (!ofs->config.uuid)
-		seq_puts(m, ",uuid=off");
+	if (ofs->config.uuid != ovl_uuid_def())
+		seq_printf(m, ",uuid=%s", ovl_uuid_str[ofs->config.uuid]);
 	if (ofs->config.nfs_export != ovl_nfs_export_def)
 		seq_printf(m, ",nfs_export=%s", ofs->config.nfs_export ?
 						"on" : "off");
@@ -437,6 +459,7 @@ enum {
 	OPT_INDEX_OFF,
 	OPT_UUID_ON,
 	OPT_UUID_OFF,
+	OPT_UUID_NOGEN,
 	OPT_NFS_EXPORT_ON,
 	OPT_USERXATTR,
 	OPT_NFS_EXPORT_OFF,
@@ -460,6 +483,7 @@ static const match_table_t ovl_tokens = {
 	{OPT_USERXATTR,			"userxattr"},
 	{OPT_UUID_ON,			"uuid=on"},
 	{OPT_UUID_OFF,			"uuid=off"},
+	{OPT_UUID_NOGEN,		"uuid=nogen"},
 	{OPT_NFS_EXPORT_ON,		"nfs_export=on"},
 	{OPT_NFS_EXPORT_OFF,		"nfs_export=off"},
 	{OPT_XINO_ON,			"xino=on"},
@@ -581,11 +605,15 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			break;
 
 		case OPT_UUID_ON:
-			config->uuid = true;
+			config->uuid = OVL_UUID_ON;
 			break;
 
 		case OPT_UUID_OFF:
-			config->uuid = false;
+			config->uuid = OVL_UUID_OFF;
+			break;
+
+		case OPT_UUID_NOGEN:
+			config->uuid = OVL_UUID_NOGEN;
 			break;
 
 		case OPT_NFS_EXPORT_ON:
@@ -1924,7 +1952,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	ofs->share_whiteout = true;
 
 	ofs->config.index = ovl_index_def;
-	ofs->config.uuid = true;
+	ofs->config.uuid = ovl_uuid_def();
 	ofs->config.nfs_export = ovl_nfs_export_def;
 	ofs->config.xino = ovl_xino_def();
 	ofs->config.metacopy = ovl_metacopy_def;
@@ -2019,10 +2047,14 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		sb->s_flags |= SB_RDONLY;
 
 	if (!ofs->config.uuid && ofs->numfs > 1) {
-		pr_warn("The uuid=off requires a single fs for lower and upper, falling back to uuid=on.\n");
-		ofs->config.uuid = true;
+		ofs->config.uuid = ovl_uuid_def();
+		pr_warn("The uuid=off requires a single fs for lower and upper, falling back to uuid=%s.\n",
+			ovl_uuid_str[ofs->config.uuid]);
 	}
 
+	if (ovl_want_uuid_gen(ofs))
+		uuid_gen(&sb->s_uuid);
+
 	if (!ovl_force_readonly(ofs) && ofs->config.index) {
 		err = ovl_get_indexdir(sb, ofs, oe, &upperpath);
 		if (err)
-- 
2.34.1

[RFC][PATCH 3/3] ovl: use persistent s_uuid with index=on

[Amir Goldstein]

With index=on, overlayfs instances are non-migratable, meaning that
the layers cannot be copied without breaking the index.

So when indexdir exists, store a persistent uuid in xattr on the
indexdir to give the overlayfs instance a persistent identifier.

This also makes f_fsid persistent and more reliable for reporting
fid info in fanotify events.

With mount option uuid=nogen, a persistent uuid is not be initialized
on indexdir, but if a persistent uuid already exists, it will be used.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/overlayfs.h |  3 +++
 fs/overlayfs/super.c     |  7 +++++++
 fs/overlayfs/util.c      | 41 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index dcdb02d0ddf8..9927472a3aaa 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -36,6 +36,7 @@ enum ovl_xattr {
 	OVL_XATTR_IMPURE,
 	OVL_XATTR_NLINK,
 	OVL_XATTR_UPPER,
+	OVL_XATTR_UUID,
 	OVL_XATTR_METACOPY,
 	OVL_XATTR_PROTATTR,
 };
@@ -431,6 +432,8 @@ bool ovl_already_copied_up(struct dentry *dentry, int flags);
 bool ovl_path_check_dir_xattr(struct ovl_fs *ofs, const struct path *path,
 			      enum ovl_xattr ox);
 bool ovl_path_check_origin_xattr(struct ovl_fs *ofs, const struct path *path);
+bool ovl_init_uuid_xattr(struct super_block *sb, struct ovl_fs *ofs,
+			 struct dentry *upperdentry, bool set);
 
 static inline bool ovl_check_origin_xattr(struct ovl_fs *ofs,
 					  struct dentry *upperdentry)
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ad2250f98b38..8364620e8722 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1535,6 +1535,9 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
 		if (err)
 			pr_err("failed to verify index dir 'upper' xattr\n");
 
+		/* Best effort get or set persistent uuid */
+		ovl_init_uuid_xattr(sb, ofs, ofs->indexdir, true);
+
 		/* Cleanup bad/stale/orphan index entries */
 		if (!err)
 			err = ovl_indexdir_cleanup(ofs);
@@ -2052,6 +2055,10 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 			ovl_uuid_str[ofs->config.uuid]);
 	}
 
+	/*
+	 * This uuid may be overridden by a persistent uuid stored in xattr on
+	 * index dir and it may be persisted in xattr on first index=on mount.
+	 */
 	if (ovl_want_uuid_gen(ofs))
 		uuid_gen(&sb->s_uuid);
 
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 923d66d131c1..8902db4b2975 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -589,6 +589,45 @@ bool ovl_path_check_origin_xattr(struct ovl_fs *ofs, const struct path *path)
 	return false;
 }
 
+/*
+ * Load persistent uuid from xattr into s_uuid if found, possibly overriding
+ * the random generated value in s_uuid.
+ * Otherwise, if @set is true and s_uuid contains a valid value, store this
+ * value in xattr.
+ */
+bool ovl_init_uuid_xattr(struct super_block *sb, struct ovl_fs *ofs,
+			 struct dentry *upperdentry, bool set)
+{
+	struct path path = {
+		.dentry = upperdentry,
+		.mnt = ovl_upper_mnt(ofs),
+	};
+	uuid_t uuid;
+	int res;
+
+	res = ovl_path_getxattr(ofs, &path, OVL_XATTR_UUID, uuid.b, UUID_SIZE);
+	if (res == UUID_SIZE) {
+		uuid_copy(&sb->s_uuid, &uuid);
+		return true;
+	}
+
+	if (res == -ENODATA) {
+		if (!set || uuid_is_null(&sb->s_uuid))
+			return false;
+
+		res = ovl_setxattr(ofs, upperdentry, OVL_XATTR_UUID,
+				   sb->s_uuid.b, UUID_SIZE);
+		if (res == 0)
+			return true;
+	} else {
+		set = false;
+	}
+
+	pr_warn("failed to %s uuid (%pd2, err=%i)\n",
+		set ? "set" : "get", upperdentry, res);
+	return false;
+}
+
 bool ovl_path_check_dir_xattr(struct ovl_fs *ofs, const struct path *path,
 			       enum ovl_xattr ox)
 {
@@ -611,6 +650,7 @@ bool ovl_path_check_dir_xattr(struct ovl_fs *ofs, const struct path *path,
 #define OVL_XATTR_IMPURE_POSTFIX	"impure"
 #define OVL_XATTR_NLINK_POSTFIX		"nlink"
 #define OVL_XATTR_UPPER_POSTFIX		"upper"
+#define OVL_XATTR_UUID_POSTFIX		"uuid"
 #define OVL_XATTR_METACOPY_POSTFIX	"metacopy"
 #define OVL_XATTR_PROTATTR_POSTFIX	"protattr"
 
@@ -625,6 +665,7 @@ const char *const ovl_xattr_table[][2] = {
 	OVL_XATTR_TAB_ENTRY(OVL_XATTR_IMPURE),
 	OVL_XATTR_TAB_ENTRY(OVL_XATTR_NLINK),
 	OVL_XATTR_TAB_ENTRY(OVL_XATTR_UPPER),
+	OVL_XATTR_TAB_ENTRY(OVL_XATTR_UUID),
 	OVL_XATTR_TAB_ENTRY(OVL_XATTR_METACOPY),
 	OVL_XATTR_TAB_ENTRY(OVL_XATTR_PROTATTR),
 };
-- 
2.34.1

Re: [RFC][PATCH 3/3] ovl: use persistent s_uuid with index=on

[Amir Goldstein]

On Tue, Apr 25, 2023 at 4:22 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> With index=on, overlayfs instances are non-migratable, meaning that
> the layers cannot be copied without breaking the index.
>
> So when indexdir exists, store a persistent uuid in xattr on the
> indexdir to give the overlayfs instance a persistent identifier.
>
> This also makes f_fsid persistent and more reliable for reporting
> fid info in fanotify events.
>
> With mount option uuid=nogen, a persistent uuid is not be initialized
> on indexdir, but if a persistent uuid already exists, it will be used.
>

This behavior (along with the grammatical mistakes) was changed in
https://github.com/amir73il/linux/commits/ovl_encode_fid

uuid=off or uuid=null both set ovl fsid to null regardless of persistent
uuid xattr.

Whether we need this backward compact option or not is to be determined.

> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/overlayfs.h |  3 +++
>  fs/overlayfs/super.c     |  7 +++++++
>  fs/overlayfs/util.c      | 41 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 51 insertions(+)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index dcdb02d0ddf8..9927472a3aaa 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -36,6 +36,7 @@ enum ovl_xattr {
>         OVL_XATTR_IMPURE,
>         OVL_XATTR_NLINK,
>         OVL_XATTR_UPPER,
> +       OVL_XATTR_UUID,
>         OVL_XATTR_METACOPY,
>         OVL_XATTR_PROTATTR,
>  };
> @@ -431,6 +432,8 @@ bool ovl_already_copied_up(struct dentry *dentry, int flags);
>  bool ovl_path_check_dir_xattr(struct ovl_fs *ofs, const struct path *path,
>                               enum ovl_xattr ox);
>  bool ovl_path_check_origin_xattr(struct ovl_fs *ofs, const struct path *path);
> +bool ovl_init_uuid_xattr(struct super_block *sb, struct ovl_fs *ofs,
> +                        struct dentry *upperdentry, bool set);
>
>  static inline bool ovl_check_origin_xattr(struct ovl_fs *ofs,
>                                           struct dentry *upperdentry)
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index ad2250f98b38..8364620e8722 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -1535,6 +1535,9 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
>                 if (err)
>                         pr_err("failed to verify index dir 'upper' xattr\n");
>
> +               /* Best effort get or set persistent uuid */
> +               ovl_init_uuid_xattr(sb, ofs, ofs->indexdir, true);
> +
>                 /* Cleanup bad/stale/orphan index entries */
>                 if (!err)
>                         err = ovl_indexdir_cleanup(ofs);
> @@ -2052,6 +2055,10 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>                         ovl_uuid_str[ofs->config.uuid]);
>         }
>
> +       /*
> +        * This uuid may be overridden by a persistent uuid stored in xattr on
> +        * index dir and it may be persisted in xattr on first index=on mount.
> +        */
>         if (ovl_want_uuid_gen(ofs))
>                 uuid_gen(&sb->s_uuid);
>
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 923d66d131c1..8902db4b2975 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -589,6 +589,45 @@ bool ovl_path_check_origin_xattr(struct ovl_fs *ofs, const struct path *path)
>         return false;
>  }
>
> +/*
> + * Load persistent uuid from xattr into s_uuid if found, possibly overriding
> + * the random generated value in s_uuid.
> + * Otherwise, if @set is true and s_uuid contains a valid value, store this
> + * value in xattr.
> + */
> +bool ovl_init_uuid_xattr(struct super_block *sb, struct ovl_fs *ofs,
> +                        struct dentry *upperdentry, bool set)
> +{
> +       struct path path = {
> +               .dentry = upperdentry,
> +               .mnt = ovl_upper_mnt(ofs),
> +       };
> +       uuid_t uuid;
> +       int res;
> +
> +       res = ovl_path_getxattr(ofs, &path, OVL_XATTR_UUID, uuid.b, UUID_SIZE);
> +       if (res == UUID_SIZE) {
> +               uuid_copy(&sb->s_uuid, &uuid);
> +               return true;
> +       }
> +
> +       if (res == -ENODATA) {
> +               if (!set || uuid_is_null(&sb->s_uuid))
> +                       return false;
> +
> +               res = ovl_setxattr(ofs, upperdentry, OVL_XATTR_UUID,
> +                                  sb->s_uuid.b, UUID_SIZE);
> +               if (res == 0)
> +                       return true;
> +       } else {
> +               set = false;
> +       }
> +
> +       pr_warn("failed to %s uuid (%pd2, err=%i)\n",
> +               set ? "set" : "get", upperdentry, res);
> +       return false;
> +}
> +
>  bool ovl_path_check_dir_xattr(struct ovl_fs *ofs, const struct path *path,
>                                enum ovl_xattr ox)
>  {
> @@ -611,6 +650,7 @@ bool ovl_path_check_dir_xattr(struct ovl_fs *ofs, const struct path *path,
>  #define OVL_XATTR_IMPURE_POSTFIX       "impure"
>  #define OVL_XATTR_NLINK_POSTFIX                "nlink"
>  #define OVL_XATTR_UPPER_POSTFIX                "upper"
> +#define OVL_XATTR_UUID_POSTFIX         "uuid"
>  #define OVL_XATTR_METACOPY_POSTFIX     "metacopy"
>  #define OVL_XATTR_PROTATTR_POSTFIX     "protattr"
>
> @@ -625,6 +665,7 @@ const char *const ovl_xattr_table[][2] = {
>         OVL_XATTR_TAB_ENTRY(OVL_XATTR_IMPURE),
>         OVL_XATTR_TAB_ENTRY(OVL_XATTR_NLINK),
>         OVL_XATTR_TAB_ENTRY(OVL_XATTR_UPPER),
> +       OVL_XATTR_TAB_ENTRY(OVL_XATTR_UUID),
>         OVL_XATTR_TAB_ENTRY(OVL_XATTR_METACOPY),
>         OVL_XATTR_TAB_ENTRY(OVL_XATTR_PROTATTR),
>  };
> --
> 2.34.1
>

Re: [RFC][PATCH 3/3] ovl: use persistent s_uuid with index=on

[Amir Goldstein]

On Thu, Jul 6, 2023 at 10:19 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, Apr 25, 2023 at 4:22 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > With index=on, overlayfs instances are non-migratable, meaning that
> > the layers cannot be copied without breaking the index.
> >
> > So when indexdir exists, store a persistent uuid in xattr on the
> > indexdir to give the overlayfs instance a persistent identifier.
> >
> > This also makes f_fsid persistent and more reliable for reporting
> > fid info in fanotify events.
> >
> > With mount option uuid=nogen, a persistent uuid is not be initialized
> > on indexdir, but if a persistent uuid already exists, it will be used.
> >
>
> This behavior (along with the grammatical mistakes) was changed in
> https://github.com/amir73il/linux/commits/ovl_encode_fid
>
> uuid=off or uuid=null both set ovl fsid to null regardless of persistent
> uuid xattr.
>

Sorry, that was meant to say "set ovl uuid to null..."
when ovl uuid is null then ovl fsid is not null, it is the fsid of the
uppermost fs.

This creates a dilemma wrt backward compat.

With index=off, the mounter has a choice between two sub-optimal options:
1. persistent ovl fsid (of upper fs)
2. unique ovl fsid (from random uuid)

If we change the default from legacy (1) to unique (2), that
could also break systems that rely on the persistent ovl fsid
of existing overlayfs layers.

With index=on, the choice is between:
1. persistent ovl fsid (of upper fs)
2. persistent and unique ovl fsid (from uuid xattr)

option (2) is superior, but still could break existing systems
that rely on (1) being persistent.

The decision to tie uuid xattr to the index dir and index=on
was rationalized in the commit message, but persistent and
unique fsid could also be implemented regardless of index=on.

I think I may have found a dignified way out of this mess:
- In ovl_fill_super(), check impure xattr on upper root dir
- If impure xattr does not exist (very likely new overlay),
  uuid_gen() and write the persistent uuid xattr on upper fs root
- If uuid xattr is written or already exists, use that to initialize
  s_uuid otherwise, leave it null
- in ovl_statfs(), override the upper fs fsid, only if ovl uuid is non-null

This gives:
1. Old overlayfs deployments retain old behavior wrt null uuid
    and upper fsid, as long as they have had at least one subdir
    of root copied up or looked up to trigger ovl_fix_origin()
2. New overlayfs deployments always generate and use a unique
    and persistent uuid/fsid
3. mount option uuid=off/null (*) can be used to retain legacy behavior
    on old/new overlayfs deployments (for whatever reason) and ignore
    existing persistent uuid xattr
4. mount option uuid=on can be used to force new behavior on an
    existing overlayfs with impure xattr and without uuid xattr

(*) uuid=off was originally introduced for the use case of copied layers.
     That is similar to the use case of copying disk images and dropping
     the old persistent ovl uuid makes sense in that case.

I will try to write this up.

Thanks,
Amir.

Re: [RFC][PATCH 3/3] ovl: use persistent s_uuid with index=on

[Amir Goldstein]

On Thu, Jul 6, 2023 at 1:14 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Thu, Jul 6, 2023 at 10:19 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Tue, Apr 25, 2023 at 4:22 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > With index=on, overlayfs instances are non-migratable, meaning that
> > > the layers cannot be copied without breaking the index.
> > >
> > > So when indexdir exists, store a persistent uuid in xattr on the
> > > indexdir to give the overlayfs instance a persistent identifier.
> > >
> > > This also makes f_fsid persistent and more reliable for reporting
> > > fid info in fanotify events.
> > >
> > > With mount option uuid=nogen, a persistent uuid is not be initialized
> > > on indexdir, but if a persistent uuid already exists, it will be used.
> > >
> >
> > This behavior (along with the grammatical mistakes) was changed in
> > https://github.com/amir73il/linux/commits/ovl_encode_fid
> >
> > uuid=off or uuid=null both set ovl fsid to null regardless of persistent
> > uuid xattr.
> >
>
> Sorry, that was meant to say "set ovl uuid to null..."
> when ovl uuid is null then ovl fsid is not null, it is the fsid of the
> uppermost fs.
>
> This creates a dilemma wrt backward compat.
>
> With index=off, the mounter has a choice between two sub-optimal options:
> 1. persistent ovl fsid (of upper fs)
> 2. unique ovl fsid (from random uuid)
>
> If we change the default from legacy (1) to unique (2), that
> could also break systems that rely on the persistent ovl fsid
> of existing overlayfs layers.
>
> With index=on, the choice is between:
> 1. persistent ovl fsid (of upper fs)
> 2. persistent and unique ovl fsid (from uuid xattr)
>
> option (2) is superior, but still could break existing systems
> that rely on (1) being persistent.
>
> The decision to tie uuid xattr to the index dir and index=on
> was rationalized in the commit message, but persistent and
> unique fsid could also be implemented regardless of index=on.
>
> I think I may have found a dignified way out of this mess:
> - In ovl_fill_super(), check impure xattr on upper root dir
> - If impure xattr does not exist (very likely new overlay),
>   uuid_gen() and write the persistent uuid xattr on upper fs root
> - If uuid xattr is written or already exists, use that to initialize
>   s_uuid otherwise, leave it null
> - in ovl_statfs(), override the upper fs fsid, only if ovl uuid is non-null
>
> This gives:
> 1. Old overlayfs deployments retain old behavior wrt null uuid
>     and upper fsid, as long as they have had at least one subdir
>     of root copied up or looked up to trigger ovl_fix_origin()
> 2. New overlayfs deployments always generate and use a unique
>     and persistent uuid/fsid
> 3. mount option uuid=off/null (*) can be used to retain legacy behavior
>     on old/new overlayfs deployments (for whatever reason) and ignore
>     existing persistent uuid xattr
> 4. mount option uuid=on can be used to force new behavior on an
>     existing overlayfs with impure xattr and without uuid xattr
>
> (*) uuid=off was originally introduced for the use case of copied layers.
>      That is similar to the use case of copying disk images and dropping
>      the old persistent ovl uuid makes sense in that case.
>
> I will try to write this up.
>

OK, this is what I got in overlayfs.rst:

UUID and fsid
-------------

The UUID of overlayfs instance itself and the fsid reported by statfs(2) are
controlled by the "uuid" mount option, which supports these values:

- "null":
    UUID of overlayfs in null, fsid is taken from upper most fs.
- "off":
    UUID of overlayfs in null, fsid is taken from upper most fs
    and UUID of underlying layers not checked.
- "on":
    UUID of overlayfs in generated on first mount used to report a unique fsid.
    If upper filesystem supports xattrs, the UUID is stored in xattr
    "trusted.overlay.uuid", making the fsid unique and persistent.
- "auto": (default)
    Upgrade to "uuid=on" on first time mount of new overlay filesystem.
    Downgrade to "uuid=null" for existing overlay filesystems that were never
    mounted with "uuid=on".

Pushed to:
https://github.com/amir73il/linux/commits/ovl_encode_fid

Will post next week.

Thanks,
Amir.