[PATCH v4 0/3] ovl: Add support for fs-verity checking of lowerdata

[PATCH v4 0/3] ovl: Add support for fs-verity checking of lowerdata

[Alexander Larsson]

This patchset adds support for using fs-verity to validate lowerdata
files by specifying an overlay.verity xattr on the metacopy
files.

This is primarily motivated by the Composefs usecase, where there will
be a read-only EROFS layer that contains redirect into a base data
layer which has fs-verity enabled on all files. However, it is also
useful in general if you want to ensure that the lowerdata files
matches the expected content over time.

I have also added some tests for this feature to xfstests[1].

This series depends on the commit
  fsverity: rework fsverity_get_digest() again
Which is in the "for-next" branch of 
  https://git.kernel.org/pub/scm/fs/fsverity/linux.git/

This series, plus the above commit are also in git here:
  https://github.com/alexlarsson/linux/tree/overlay-verity

I would love to see this go into 6.5. So Eric, could you maybe Ack the
implementation patches separately from the documentation patches? Then
maybe we can get this in early, and I promise to try to get the
documentation up to standard during the 6.5 cycle as needed.

Changes since v3:
 * Instead of using a overlay.digest xattr we extend the current
   overlay.metacopy xattr with version, flags and digest. This makes
   it flexible for later changes and allows us to use the existing
   xattr lookup to know ahead of time whether a file needs to have
   verity validated.

   I've done some performance checks on this new layout, and the
   results are essentially the same as before.

 * This is rebased on top of the latest overlayfs-next, which includes
   the changes to the new mount API, so that part has been redone.

 * The documentation changes have been rewritten to try to be more
   clear about the behaviour of i/o verification when verity is used.

Changes since v2:
 * Rebased on top of overlayfs-next
 * We now alway do verity verification the first time the file content
   is used, rather than doing it at lookup time for the non-lazy lookup
   case.

Changes since v1:
 * Rebased on v2 lazy lowerdata series
 * Dropped the "validate" mount option variant. We now only support
   "off", "on" and "require", where "off" is the default.
 * We now store the digest algorithm used in the overlay.verity xattr.
 * Dropped ability to configure default verity options, as this could
   cause problems moving layers between machines.
 * We now properly resolve dependent mount options by automatically
   enabling metacopy and redirect_dir if verity is on, or failing
   if the specified options conflict.
 * Streamlined and fixed the handling of creds in ovl_ensure_verity_loaded().
 * Renamed new helpers from ovl_entry_path_ to ovl_e_path_

[1] https://github.com/alexlarsson/xfstests/commits/verity-tests

Alexander Larsson (3):
  ovl: Add framework for verity support
  ovl: Validate verity xattr when resolving lowerdata
  ovl: Handle verity during copy-up

 Documentation/filesystems/fsverity.rst  |   2 +
 Documentation/filesystems/overlayfs.rst |  48 ++++++++
 fs/overlayfs/copy_up.c                  |  47 ++++++-
 fs/overlayfs/file.c                     |   8 +-
 fs/overlayfs/namei.c                    |  97 +++++++++++++--
 fs/overlayfs/overlayfs.h                |  53 ++++++--
 fs/overlayfs/ovl_entry.h                |   1 +
 fs/overlayfs/super.c                    |  69 +++++++++--
 fs/overlayfs/util.c                     | 156 +++++++++++++++++++++++-
 9 files changed, 448 insertions(+), 33 deletions(-)

-- 
2.40.1

[PATCH v4 1/3] ovl: Add framework for verity support

[Alexander Larsson]

This adds the scaffolding (docs, config, mount options) for supporting
the new digest field in the metacopy xattr. This which contains a
fs-verity digest that need to match the actual fs-verity digest of the
lowerdata file. The mount option "verity" specifies how this xattr is
handled.

If you enable verity ("verity=on") all existing xattrs are validated
before use, and during metacopy we generate verity xattr in the upper
metacopy file (if the source file has verity enabled). This means
later accesses can guarantee that the same data is used.

Additionally you can use "verity=require". In this mode all metacopy
files must have a valid verity xattr. For this to work metadata
copy-up must be able to create a verity xattr (so that later accesses
are validated). Therefore, in this mode, if the lower data file
doesn't have fs-verity enabled we fall back to a full copy rather than
a metacopy.

Actual implementation follows in a separate commit.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
---
 Documentation/filesystems/fsverity.rst  |  2 +
 Documentation/filesystems/overlayfs.rst | 48 +++++++++++++++++++
 fs/overlayfs/overlayfs.h                |  7 +++
 fs/overlayfs/ovl_entry.h                |  1 +
 fs/overlayfs/super.c                    | 64 ++++++++++++++++++++++---
 5 files changed, 116 insertions(+), 6 deletions(-)

diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index ede672dedf11..b3ba548e7b86 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -324,6 +324,8 @@ the file has fs-verity enabled.  This can perform better than
 FS_IOC_GETFLAGS and FS_IOC_MEASURE_VERITY because it doesn't require
 opening the file, and opening verity files can be expensive.
 
+.. _accessing_verity_files:
+
 Accessing verity files
 ======================
 
diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
index eb7d2c88ddec..b639d9efe9ae 100644
--- a/Documentation/filesystems/overlayfs.rst
+++ b/Documentation/filesystems/overlayfs.rst
@@ -405,6 +405,54 @@ when a "metacopy" file in one of the lower layers above it, has a "redirect"
 to the absolute path of the "lower data" file in the "data-only" lower layer.
 
 
+fs-verity support
+----------------------
+
+During metadata copy up of a lower file, if the source file has
+fs-verity enabled and overlay verity support is enabled, then the
+"trusted.overlay.verity" xattr is set on the new metacopy file. This
+specifies the expected fs-verity digest of the lowerdata file, which
+is used to verify the content of the lower file at the time the
+metacopy file is opened.
+
+When a layer containing verity xattrs is used, it means that any such
+metacopy file in the upper layer is guaranteed to match the content
+that was in the lower at the time of the copy-up. If at any time
+(during a mount, after a remount, etc) such a file in the lower is
+replaced or modified in any way, access to the corresponding file in
+overlayfs will result in EIO errors (either on open, due to overlayfs
+digest check, or from a later read due to fs-verity) and a detailed
+error is printed to the kernel logs. For more details of how fs-verity
+file access works, see :ref:`Documentation/filesystems/fsverity.rst
+<accessing_verity_files>`.
+
+Verity can be used as a general robustness check to detect accidental
+changes in the overlayfs directories in use. But, with additional care
+it can also give more powerful guarantees. For example, if the upper
+layer is fully trusted (by using dm-verity or something similar), then
+an untrusted lower layer can be used to supply validated file content
+for all metacopy files.  If additionally the untrusted lower
+directories are specified as "Data-only", then they can only supply
+such file content, and the entire mount can be trusted to match the
+upper layer.
+
+This feature is controlled by the "verity" mount option, which
+supports these values:
+
+- "off":
+    The verity xattr is never used. This is the default if verity
+    option is not specified.
+- "on":
+    Whenever a metacopy files specifies an expected digest, the
+    corresponding data file must match the specified digest.
+    When generating a metacopy file the verity xattr will be set
+    from the source file fs-verity digest (if it has one).
+- "require":
+    Same as "on", but additionally all metacopy files must specify a
+    verity xattr. This means metadata copy up will only be used if
+    the data file has fs-verity enabled, otherwise a full copy-up is
+    used.
+
 Sharing and copying layers
 --------------------------
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 5b6ac03af192..7414d6d8fb1c 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -70,12 +70,19 @@ enum {
 	OVL_XINO_ON,
 };
 
+enum {
+	OVL_VERITY_OFF,
+	OVL_VERITY_ON,
+	OVL_VERITY_REQUIRE,
+};
+
 /* The set of options that user requested explicitly via mount options */
 struct ovl_opt_set {
 	bool metacopy;
 	bool redirect;
 	bool nfs_export;
 	bool index;
+	bool verity;
 };
 
 /*
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 306e1ecdc96d..e999c73fb0c3 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -10,6 +10,7 @@ struct ovl_config {
 	char *workdir;
 	bool default_permissions;
 	int redirect_mode;
+	int verity_mode;
 	bool index;
 	bool uuid;
 	bool nfs_export;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ed4b35c9d647..3f8bbd158a2a 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -366,6 +366,23 @@ static inline int ovl_xino_def(void)
 	return ovl_xino_auto_def ? OVL_XINO_AUTO : OVL_XINO_OFF;
 }
 
+static const struct constant_table ovl_parameter_verity[] = {
+	{ "off",	OVL_VERITY_OFF     },
+	{ "on",		OVL_VERITY_ON      },
+	{ "require",	OVL_VERITY_REQUIRE },
+	{}
+};
+
+static const char *ovl_verity_mode(struct ovl_config *config)
+{
+	return ovl_parameter_verity[config->verity_mode].name;
+}
+
+static int ovl_verity_mode_def(void)
+{
+	return OVL_VERITY_OFF;
+}
+
 /**
  * ovl_show_options
  * @m: the seq_file handle
@@ -414,6 +431,9 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 		seq_puts(m, ",volatile");
 	if (ofs->config.userxattr)
 		seq_puts(m, ",userxattr");
+	if (ofs->config.verity_mode != ovl_verity_mode_def())
+		seq_printf(m, ",verity=%s",
+			   ovl_verity_mode(&ofs->config));
 	return 0;
 }
 
@@ -463,6 +483,7 @@ enum {
 	Opt_xino,
 	Opt_metacopy,
 	Opt_volatile,
+	Opt_verity,
 };
 
 static const struct constant_table ovl_parameter_bool[] = {
@@ -487,6 +508,7 @@ static const struct fs_parameter_spec ovl_parameter_spec[] = {
 	fsparam_enum("xino",                Opt_xino, ovl_parameter_xino),
 	fsparam_enum("metacopy",            Opt_metacopy, ovl_parameter_bool),
 	fsparam_flag("volatile",            Opt_volatile),
+	fsparam_enum("verity",              Opt_verity, ovl_parameter_verity),
 	{}
 };
 
@@ -557,6 +579,10 @@ static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	case Opt_userxattr:
 		config->userxattr = true;
 		break;
+	case Opt_verity:
+		config->verity_mode = result.uint_32;
+		ctx->set.verity = true;
+		break;
 	default:
 		pr_err("unrecognized mount option \"%s\" or missing value\n",
 		       param->key);
@@ -596,6 +622,18 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 		config->ovl_volatile = false;
 	}
 
+	/* Resolve verity -> metacopy dependency */
+	if (config->verity_mode && !config->metacopy) {
+		/* Don't allow explicit specified conflicting combinations */
+		if (set.metacopy) {
+			pr_err("conflicting options: metacopy=off,verity=%s\n",
+			       ovl_verity_mode(config));
+			return -EINVAL;
+		}
+		/* Otherwise automatically enable metacopy. */
+		config->metacopy = true;
+	}
+
 	/*
 	 * This is to make the logic below simpler.  It doesn't make any other
 	 * difference, since redirect_dir=on is only used for upper.
@@ -610,11 +648,12 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 			       ovl_redirect_mode(config));
 			return -EINVAL;
 		}
+		if (set.verity && set.redirect) {
+			pr_err("conflicting options: verity=%s,redirect_dir=%s\n",
+			       ovl_verity_mode(config), ovl_redirect_mode(config));
+			return -EINVAL;
+		}
 		if (set.redirect) {
-			/*
-			 * There was an explicit redirect_dir=... that resulted
-			 * in this conflict.
-			 */
 			pr_info("disabling metacopy due to redirect_dir=%s\n",
 				ovl_redirect_mode(config));
 			config->metacopy = false;
@@ -646,7 +685,7 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 		}
 	}
 
-	/* Resolve nfs_export -> !metacopy dependency */
+	/* Resolve nfs_export -> !metacopy && !verity dependency */
 	if (config->nfs_export && config->metacopy) {
 		if (set.nfs_export && set.metacopy) {
 			pr_err("conflicting options: nfs_export=on,metacopy=on\n");
@@ -659,6 +698,14 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 			 */
 			pr_info("disabling nfs_export due to metacopy=on\n");
 			config->nfs_export = false;
+		} else if (set.verity) {
+			/*
+			 * There was an explicit verity=.. that resulted
+			 * in this conflict.
+			 */
+			pr_info("disabling nfs_export due to verity=%s\n",
+				ovl_verity_mode(config));
+			config->nfs_export = false;
 		} else {
 			/*
 			 * There was an explicit nfs_export=on that resulted
@@ -670,7 +717,7 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 	}
 
 
-	/* Resolve userxattr -> !redirect && !metacopy dependency */
+	/* Resolve userxattr -> !redirect && !metacopy && !verity dependency */
 	if (config->userxattr) {
 		if (set.redirect &&
 		    config->redirect_mode != OVL_REDIRECT_NOFOLLOW) {
@@ -682,6 +729,11 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 			pr_err("conflicting options: userxattr,metacopy=on\n");
 			return -EINVAL;
 		}
+		if (config->verity_mode) {
+			pr_err("conflicting options: userxattr,verity=%s\n",
+			       ovl_verity_mode(config));
+			return -EINVAL;
+		}
 		/*
 		 * Silently disable default setting of redirect and metacopy.
 		 * This shall be the default in the future as well: these
-- 
2.40.1

Re: [PATCH v4 1/3] ovl: Add framework for verity support

[Amir Goldstein]

On Tue, Jun 20, 2023 at 1:15 PM Alexander Larsson <alexl@redhat.com> wrote:
>
> This adds the scaffolding (docs, config, mount options) for supporting
> the new digest field in the metacopy xattr. This which contains a
> fs-verity digest that need to match the actual fs-verity digest of the
> lowerdata file. The mount option "verity" specifies how this xattr is
> handled.
>
> If you enable verity ("verity=on") all existing xattrs are validated
> before use, and during metacopy we generate verity xattr in the upper
> metacopy file (if the source file has verity enabled). This means
> later accesses can guarantee that the same data is used.
>
> Additionally you can use "verity=require". In this mode all metacopy
> files must have a valid verity xattr. For this to work metadata
> copy-up must be able to create a verity xattr (so that later accesses
> are validated). Therefore, in this mode, if the lower data file
> doesn't have fs-verity enabled we fall back to a full copy rather than
> a metacopy.
>
> Actual implementation follows in a separate commit.
>
> Signed-off-by: Alexander Larsson <alexl@redhat.com>
> ---
>  Documentation/filesystems/fsverity.rst  |  2 +
>  Documentation/filesystems/overlayfs.rst | 48 +++++++++++++++++++
>  fs/overlayfs/overlayfs.h                |  7 +++
>  fs/overlayfs/ovl_entry.h                |  1 +
>  fs/overlayfs/super.c                    | 64 ++++++++++++++++++++++---
>  5 files changed, 116 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index ede672dedf11..b3ba548e7b86 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -324,6 +324,8 @@ the file has fs-verity enabled.  This can perform better than
>  FS_IOC_GETFLAGS and FS_IOC_MEASURE_VERITY because it doesn't require
>  opening the file, and opening verity files can be expensive.
>
> +.. _accessing_verity_files:
> +
>  Accessing verity files
>  ======================
>
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index eb7d2c88ddec..b639d9efe9ae 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -405,6 +405,54 @@ when a "metacopy" file in one of the lower layers above it, has a "redirect"
>  to the absolute path of the "lower data" file in the "data-only" lower layer.
>
>
> +fs-verity support
> +----------------------
> +
> +During metadata copy up of a lower file, if the source file has
> +fs-verity enabled and overlay verity support is enabled, then the
> +"trusted.overlay.verity" xattr

outdated old format

> is set on the new metacopy file. This
> +specifies the expected fs-verity digest of the lowerdata file, which
> +is used to verify the content of the lower file at the time the
> +metacopy file is opened.
> +
> +When a layer containing verity xattrs is used, it means that any such
> +metacopy file in the upper layer is guaranteed to match the content
> +that was in the lower at the time of the copy-up. If at any time
> +(during a mount, after a remount, etc) such a file in the lower is
> +replaced or modified in any way, access to the corresponding file in
> +overlayfs will result in EIO errors (either on open, due to overlayfs
> +digest check, or from a later read due to fs-verity) and a detailed
> +error is printed to the kernel logs. For more details of how fs-verity
> +file access works, see :ref:`Documentation/filesystems/fsverity.rst
> +<accessing_verity_files>`.
> +
> +Verity can be used as a general robustness check to detect accidental
> +changes in the overlayfs directories in use. But, with additional care
> +it can also give more powerful guarantees. For example, if the upper
> +layer is fully trusted (by using dm-verity or something similar), then
> +an untrusted lower layer can be used to supply validated file content
> +for all metacopy files.  If additionally the untrusted lower
> +directories are specified as "Data-only", then they can only supply
> +such file content, and the entire mount can be trusted to match the
> +upper layer.
> +
> +This feature is controlled by the "verity" mount option, which
> +supports these values:
> +
> +- "off":
> +    The verity xattr is never used. This is the default if verity
> +    option is not specified.
> +- "on":
> +    Whenever a metacopy files specifies an expected digest, the
> +    corresponding data file must match the specified digest.
> +    When generating a metacopy file the verity xattr will be set
> +    from the source file fs-verity digest (if it has one).
> +- "require":
> +    Same as "on", but additionally all metacopy files must specify a
> +    verity xattr. This means metadata copy up will only be used if
> +    the data file has fs-verity enabled, otherwise a full copy-up is
> +    used.
> +
>  Sharing and copying layers
>  --------------------------
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 5b6ac03af192..7414d6d8fb1c 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -70,12 +70,19 @@ enum {
>         OVL_XINO_ON,
>  };
>
> +enum {
> +       OVL_VERITY_OFF,
> +       OVL_VERITY_ON,
> +       OVL_VERITY_REQUIRE,
> +};
> +
>  /* The set of options that user requested explicitly via mount options */
>  struct ovl_opt_set {
>         bool metacopy;
>         bool redirect;
>         bool nfs_export;
>         bool index;
> +       bool verity;

There is no need for that because there is no kconfig/module
option to enable verity and we never implicitly enable verity
because of some other option...

>  };
>
>  /*
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index 306e1ecdc96d..e999c73fb0c3 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -10,6 +10,7 @@ struct ovl_config {
>         char *workdir;
>         bool default_permissions;
>         int redirect_mode;
> +       int verity_mode;
>         bool index;
>         bool uuid;
>         bool nfs_export;
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index ed4b35c9d647..3f8bbd158a2a 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -366,6 +366,23 @@ static inline int ovl_xino_def(void)
>         return ovl_xino_auto_def ? OVL_XINO_AUTO : OVL_XINO_OFF;
>  }
>
> +static const struct constant_table ovl_parameter_verity[] = {
> +       { "off",        OVL_VERITY_OFF     },
> +       { "on",         OVL_VERITY_ON      },
> +       { "require",    OVL_VERITY_REQUIRE },
> +       {}
> +};
> +
> +static const char *ovl_verity_mode(struct ovl_config *config)
> +{
> +       return ovl_parameter_verity[config->verity_mode].name;
> +}
> +
> +static int ovl_verity_mode_def(void)
> +{
> +       return OVL_VERITY_OFF;
> +}
> +
>  /**
>   * ovl_show_options
>   * @m: the seq_file handle
> @@ -414,6 +431,9 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
>                 seq_puts(m, ",volatile");
>         if (ofs->config.userxattr)
>                 seq_puts(m, ",userxattr");
> +       if (ofs->config.verity_mode != ovl_verity_mode_def())
> +               seq_printf(m, ",verity=%s",
> +                          ovl_verity_mode(&ofs->config));
>         return 0;
>  }
>
> @@ -463,6 +483,7 @@ enum {
>         Opt_xino,
>         Opt_metacopy,
>         Opt_volatile,
> +       Opt_verity,
>  };
>
>  static const struct constant_table ovl_parameter_bool[] = {
> @@ -487,6 +508,7 @@ static const struct fs_parameter_spec ovl_parameter_spec[] = {
>         fsparam_enum("xino",                Opt_xino, ovl_parameter_xino),
>         fsparam_enum("metacopy",            Opt_metacopy, ovl_parameter_bool),
>         fsparam_flag("volatile",            Opt_volatile),
> +       fsparam_enum("verity",              Opt_verity, ovl_parameter_verity),
>         {}
>  };
>
> @@ -557,6 +579,10 @@ static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
>         case Opt_userxattr:
>                 config->userxattr = true;
>                 break;
> +       case Opt_verity:
> +               config->verity_mode = result.uint_32;
> +               ctx->set.verity = true;

...so config->verity_mode is enough for all the conflict checks...

> +               break;
>         default:
>                 pr_err("unrecognized mount option \"%s\" or missing value\n",
>                        param->key);
> @@ -596,6 +622,18 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>                 config->ovl_volatile = false;
>         }
>
> +       /* Resolve verity -> metacopy dependency */
> +       if (config->verity_mode && !config->metacopy) {
> +               /* Don't allow explicit specified conflicting combinations */
> +               if (set.metacopy) {
> +                       pr_err("conflicting options: metacopy=off,verity=%s\n",
> +                              ovl_verity_mode(config));
> +                       return -EINVAL;
> +               }
> +               /* Otherwise automatically enable metacopy. */
> +               config->metacopy = true;
> +       }
> +

Nice :)

I think the comments below should be changed to clarify
the indirect dependency that they also check for, e.g.:

        /* Resolve verity -> metacopy -> redirect_dir dependency */


>         /*
>          * This is to make the logic below simpler.  It doesn't make any other
>          * difference, since redirect_dir=on is only used for upper.
> @@ -610,11 +648,12 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>                                ovl_redirect_mode(config));
>                         return -EINVAL;
>                 }
> +               if (set.verity && set.redirect) {
> +                       pr_err("conflicting options: verity=%s,redirect_dir=%s\n",

verity=off,redirect=... is never a conflict
so the correct condition is (config->verity_mode && set.redirect)

> +                              ovl_verity_mode(config), ovl_redirect_mode(config));
> +                       return -EINVAL;
> +               }
>                 if (set.redirect) {
> -                       /*
> -                        * There was an explicit redirect_dir=... that resulted
> -                        * in this conflict.
> -                        */
>                         pr_info("disabling metacopy due to redirect_dir=%s\n",
>                                 ovl_redirect_mode(config));
>                         config->metacopy = false;
> @@ -646,7 +685,7 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>                 }
>         }
>
> -       /* Resolve nfs_export -> !metacopy dependency */
> +       /* Resolve nfs_export -> !metacopy && !verity dependency */
>         if (config->nfs_export && config->metacopy) {
>                 if (set.nfs_export && set.metacopy) {
>                         pr_err("conflicting options: nfs_export=on,metacopy=on\n");
> @@ -659,6 +698,14 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>                          */
>                         pr_info("disabling nfs_export due to metacopy=on\n");
>                         config->nfs_export = false;
> +               } else if (set.verity) {

} else if (config->nfs_export && config->verity_mode) {

> +                       /*
> +                        * There was an explicit verity=.. that resulted
> +                        * in this conflict.
> +                        */
> +                       pr_info("disabling nfs_export due to verity=%s\n",
> +                               ovl_verity_mode(config));
> +                       config->nfs_export = false;
>                 } else {
>                         /*
>                          * There was an explicit nfs_export=on that resulted
> @@ -670,7 +717,7 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>         }
>
>
> -       /* Resolve userxattr -> !redirect && !metacopy dependency */
> +       /* Resolve userxattr -> !redirect && !metacopy && !verity dependency */
>         if (config->userxattr) {
>                 if (set.redirect &&
>                     config->redirect_mode != OVL_REDIRECT_NOFOLLOW) {
> @@ -682,6 +729,11 @@ static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>                         pr_err("conflicting options: userxattr,metacopy=on\n");
>                         return -EINVAL;
>                 }
> +               if (config->verity_mode) {
> +                       pr_err("conflicting options: userxattr,verity=%s\n",
> +                              ovl_verity_mode(config));
> +                       return -EINVAL;
> +               }

This one looks right :)

Thanks,
Amir.

[PATCH v4 2/3] ovl: Validate verity xattr when resolving lowerdata

[Alexander Larsson]

The metacopy xattr is extended from currently empty to a versioned
header with length, flags and an optional digest. During lookup we
record whether the header contained a digest in the OVL_HAS_DIGEST
flags.

When accessing file data the first time, if OVL_HAS_DIGEST is set, we
reload the metadata and that the source lowerdata inode matches the
specified digest in it (according to the enabled verity options). If
the verity check passes we store this info in the inode flags as
OVL_VERIFIED_DIGEST, so that we can avoid doing it again if the inode
remains in memory.

The verification is done in ovl_maybe_validate_verity() which needs to
be called in the same places as ovl_maybe_lookup_lowerdata(), so there
is a new ovl_verify_lowerdata() helper that calls these in the right
order, and all current callers of ovl_maybe_lookup_lowerdata() are
changed to call it instead.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
---
 fs/overlayfs/copy_up.c   |   2 +-
 fs/overlayfs/file.c      |   8 +--
 fs/overlayfs/namei.c     |  97 +++++++++++++++++++++++++++---
 fs/overlayfs/overlayfs.h |  43 ++++++++++---
 fs/overlayfs/super.c     |   5 +-
 fs/overlayfs/util.c      | 126 +++++++++++++++++++++++++++++++++++++--
 6 files changed, 256 insertions(+), 25 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 568f743a5584..68f01fd7f211 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -1078,7 +1078,7 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags)
 	 * not very important to optimize this case, so do lazy lowerdata lookup
 	 * before any copy up, so we can do it before taking ovl_inode_lock().
 	 */
-	err = ovl_maybe_lookup_lowerdata(dentry);
+	err = ovl_verify_lowerdata(dentry);
 	if (err)
 		return err;
 
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 39737c2aaa84..6583d08fdb7a 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -115,8 +115,8 @@ static int ovl_real_fdget_meta(const struct file *file, struct fd *real,
 	if (allow_meta) {
 		ovl_path_real(dentry, &realpath);
 	} else {
-		/* lazy lookup of lowerdata */
-		err = ovl_maybe_lookup_lowerdata(dentry);
+		/* lazy lookup and verify of lowerdata */
+		err = ovl_verify_lowerdata(dentry);
 		if (err)
 			return err;
 
@@ -159,8 +159,8 @@ static int ovl_open(struct inode *inode, struct file *file)
 	struct path realpath;
 	int err;
 
-	/* lazy lookup of lowerdata */
-	err = ovl_maybe_lookup_lowerdata(dentry);
+	/* lazy lookup and verify lowerdata */
+	err = ovl_verify_lowerdata(dentry);
 	if (err)
 		return err;
 
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 57adf911735f..0ba8266a8125 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -26,6 +26,7 @@ struct ovl_lookup_data {
 	bool last;
 	char *redirect;
 	bool metacopy;
+	bool metacopy_digest;
 	/* Referring to last redirect xattr */
 	bool absolute_redirect;
 };
@@ -233,6 +234,7 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
 {
 	struct dentry *this;
 	struct path path;
+	int metacopy_size;
 	int err;
 	bool last_element = !post[0];
 
@@ -270,11 +272,14 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
 			d->stop = true;
 			goto put_and_out;
 		}
-		err = ovl_check_metacopy_xattr(OVL_FS(d->sb), &path);
-		if (err < 0)
+		metacopy_size = ovl_check_metacopy_xattr(OVL_FS(d->sb), &path, NULL);
+		if (metacopy_size < 0) {
+			err = metacopy_size;
 			goto out_err;
+		}
 
-		d->metacopy = err;
+		d->metacopy = metacopy_size;
+		d->metacopy_digest = d->metacopy && metacopy_size > OVL_METACOPY_MIN_SIZE;
 		d->stop = !d->metacopy;
 		if (!d->metacopy || d->last)
 			goto out;
@@ -889,8 +894,59 @@ static int ovl_fix_origin(struct ovl_fs *ofs, struct dentry *dentry,
 	return err;
 }
 
+static int ovl_maybe_validate_verity(struct dentry *dentry)
+{
+	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+	struct inode *inode = d_inode(dentry);
+	struct path datapath, metapath;
+	int err;
+
+	if (!ofs->config.verity_mode ||
+	    !ovl_is_metacopy_dentry(dentry) ||
+	    ovl_test_flag(OVL_VERIFIED_DIGEST, inode))
+		return 0;
+
+	if (!ovl_test_flag(OVL_HAS_DIGEST, inode)) {
+		if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
+			pr_warn_ratelimited("metacopy file '%pd' has no digest specified\n",
+					    dentry);
+			return -EIO;
+		}
+		return 0;
+	}
+
+	ovl_path_lowerdata(dentry, &datapath);
+	if (!datapath.dentry)
+		return -EIO;
+
+	ovl_path_real(dentry, &metapath);
+	if (!metapath.dentry)
+		return -EIO;
+
+	err = ovl_inode_lock_interruptible(inode);
+	if (err)
+		return err;
+
+	if (ovl_test_flag(OVL_HAS_DIGEST, inode) &&
+	    !ovl_test_flag(OVL_VERIFIED_DIGEST, inode)) {
+		const struct cred *old_cred;
+
+		old_cred = ovl_override_creds(dentry->d_sb);
+
+		err = ovl_validate_verity(ofs, &metapath, &datapath);
+		if (err == 0)
+			ovl_set_flag(OVL_VERIFIED_DIGEST, inode);
+
+		revert_creds(old_cred);
+	}
+
+	ovl_inode_unlock(inode);
+
+	return err;
+}
+
 /* Lazy lookup of lowerdata */
-int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
+static int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
 {
 	struct inode *inode = d_inode(dentry);
 	const char *redirect = ovl_lowerdata_redirect(inode);
@@ -935,6 +991,17 @@ int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
 	goto out;
 }
 
+int ovl_verify_lowerdata(struct dentry *dentry)
+{
+	int err;
+
+	err = ovl_maybe_lookup_lowerdata(dentry);
+	if (err)
+		return err;
+
+	return ovl_maybe_validate_verity(dentry);
+}
+
 struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			  unsigned int flags)
 {
@@ -952,9 +1019,11 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	bool upperopaque = false;
 	char *upperredirect = NULL;
 	struct dentry *this;
+	int metacopy_size;
 	unsigned int i;
 	int err;
 	bool uppermetacopy = false;
+	bool metacopy_digest = false;
 	struct ovl_lookup_data d = {
 		.sb = dentry->d_sb,
 		.name = dentry->d_name,
@@ -964,6 +1033,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		.last = ovl_redirect_follow(ofs) ? false : !ovl_numlower(poe),
 		.redirect = NULL,
 		.metacopy = false,
+		.metacopy_digest = false,
 	};
 
 	if (dentry->d_name.len > ofs->namelen)
@@ -997,8 +1067,10 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			if (err)
 				goto out_put_upper;
 
-			if (d.metacopy)
+			if (d.metacopy) {
 				uppermetacopy = true;
+				metacopy_digest = d.metacopy_digest;
+			}
 		}
 
 		if (d.redirect) {
@@ -1076,6 +1148,9 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			origin = this;
 		}
 
+		if (!upperdentry && !d.is_dir && !ctr && d.metacopy)
+			metacopy_digest = d.metacopy_digest;
+
 		if (d.metacopy && ctr) {
 			/*
 			 * Do not store intermediate metacopy dentries in
@@ -1211,10 +1286,13 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			upperredirect = NULL;
 			goto out_free_oe;
 		}
-		err = ovl_check_metacopy_xattr(ofs, &upperpath);
-		if (err < 0)
+		metacopy_size = ovl_check_metacopy_xattr(ofs, &upperpath, NULL);
+		if (metacopy_size < 0) {
+			err = metacopy_size;
 			goto out_free_oe;
-		uppermetacopy = err;
+		}
+		uppermetacopy = metacopy_size;
+		metacopy_digest = metacopy_size >  OVL_METACOPY_MIN_SIZE;
 	}
 
 	if (upperdentry || ctr) {
@@ -1236,6 +1314,9 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			goto out_free_oe;
 		if (upperdentry && !uppermetacopy)
 			ovl_set_flag(OVL_UPPERDATA, inode);
+
+		if ((uppermetacopy || ctr > 1) && metacopy_digest)
+			ovl_set_flag(OVL_HAS_DIGEST, inode);
 	}
 
 	ovl_dentry_init_reval(dentry, upperdentry, OVL_I_E(inode));
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 7414d6d8fb1c..c2213a8ad16e 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -7,6 +7,7 @@
 #include <linux/kernel.h>
 #include <linux/uuid.h>
 #include <linux/fs.h>
+#include <linux/fsverity.h>
 #include <linux/namei.h>
 #include <linux/posix_acl.h>
 #include <linux/posix_acl_xattr.h>
@@ -49,6 +50,8 @@ enum ovl_inode_flag {
 	OVL_UPPERDATA,
 	/* Inode number will remain constant over copy up. */
 	OVL_CONST_INO,
+	OVL_HAS_DIGEST,
+	OVL_VERIFIED_DIGEST,
 };
 
 enum ovl_entry_flag {
@@ -141,6 +144,24 @@ struct ovl_fh {
 #define OVL_FH_FID_OFFSET	(OVL_FH_WIRE_OFFSET + \
 				 offsetof(struct ovl_fb, fid))
 
+/* On-disk format for "metacopy" xattr (if non-zero size) */
+struct ovl_metacopy {
+	u8 version;	/* 0 */
+	u8 len;         /* size of this header, not including unused digest bytes */
+	u8 flags;
+	u8 digest_algo;	/* FS_VERITY_HASH_ALG_* constant, 0 for no digest */
+	u8 digest[FS_VERITY_MAX_DIGEST_SIZE];  /* Only the used part on disk */
+} __packed;
+
+#define OVL_METACOPY_MAX_SIZE (sizeof(struct ovl_metacopy))
+#define OVL_METACOPY_MIN_SIZE (OVL_METACOPY_MAX_SIZE - FS_VERITY_MAX_DIGEST_SIZE)
+#define OVL_METACOPY_INIT { 0, OVL_METACOPY_MIN_SIZE }
+
+static inline int ovl_metadata_digest_size(const struct ovl_metacopy *metacopy)
+{
+	return (int)metacopy->len - OVL_METACOPY_MIN_SIZE;
+}
+
 extern const char *const ovl_xattr_table[][2];
 static inline const char *ovl_xattr(struct ovl_fs *ofs, enum ovl_xattr ox)
 {
@@ -241,7 +262,7 @@ static inline ssize_t ovl_do_getxattr(const struct path *path, const char *name,
 	WARN_ON(path->dentry->d_sb != path->mnt->mnt_sb);
 
 	err = vfs_getxattr(mnt_idmap(path->mnt), path->dentry,
-			       name, value, size);
+			   name, value, size);
 	len = (value && err > 0) ? err : 0;
 
 	pr_debug("getxattr(%pd2, \"%s\", \"%*pE\", %zu, 0) = %i\n",
@@ -263,9 +284,9 @@ static inline ssize_t ovl_getxattr_upper(struct ovl_fs *ofs,
 }
 
 static inline ssize_t ovl_path_getxattr(struct ovl_fs *ofs,
-					 const struct path *path,
-					 enum ovl_xattr ox, void *value,
-					 size_t size)
+					const struct path *path,
+					enum ovl_xattr ox, void *value,
+					size_t size)
 {
 	return ovl_do_getxattr(path, ovl_xattr(ofs, ox), value, size);
 }
@@ -352,7 +373,7 @@ static inline struct file *ovl_do_tmpfile(struct ovl_fs *ofs,
 {
 	struct path path = { .mnt = ovl_upper_mnt(ofs), .dentry = dentry };
 	struct file *file = vfs_tmpfile_open(ovl_upper_mnt_idmap(ofs), &path, mode,
-					O_LARGEFILE | O_WRONLY, current_cred());
+					     O_LARGEFILE | O_WRONLY, current_cred());
 	int err = PTR_ERR_OR_ZERO(file);
 
 	pr_debug("tmpfile(%pd2, 0%o) = %i\n", dentry, mode, err);
@@ -490,9 +511,17 @@ bool ovl_need_index(struct dentry *dentry);
 int ovl_nlink_start(struct dentry *dentry);
 void ovl_nlink_end(struct dentry *dentry);
 int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir);
-int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path);
+int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path,
+			     struct ovl_metacopy *data);
+int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d,
+			   struct ovl_metacopy *metacopy);
 bool ovl_is_metacopy_dentry(struct dentry *dentry);
 char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int padding);
+int ovl_get_verity_xattr(struct ovl_fs *ofs, const struct path *path,
+			 u8 *digest_buf, int *buf_length);
+int ovl_validate_verity(struct ovl_fs *ofs,
+			struct path *metapath,
+			struct path *datapath);
 int ovl_sync_status(struct ovl_fs *ofs);
 
 static inline void ovl_set_flag(unsigned long flag, struct inode *inode)
@@ -612,7 +641,7 @@ struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh);
 struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
 				struct dentry *origin, bool verify);
 int ovl_path_next(int idx, struct dentry *dentry, struct path *path);
-int ovl_maybe_lookup_lowerdata(struct dentry *dentry);
+int ovl_verify_lowerdata(struct dentry *dentry);
 struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			  unsigned int flags);
 bool ovl_lower_positive(struct dentry *dentry);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 3f8bbd158a2a..2175e64d3b64 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -63,6 +63,7 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
 				 const struct inode *inode)
 {
 	struct dentry *real = NULL, *lower;
+	int err;
 
 	/* It's an overlay file */
 	if (inode && d_inode(dentry) == inode)
@@ -89,7 +90,9 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
 	 * uprobes on offset within the file, so lowerdata should be available
 	 * when setting the uprobe.
 	 */
-	ovl_maybe_lookup_lowerdata(dentry);
+	err = ovl_verify_lowerdata(dentry);
+	if (err)
+		goto bug;
 	lower = ovl_dentry_lowerdata(dentry);
 	if (!lower)
 		goto bug;
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 7ef9e13c404a..66448964f753 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -10,6 +10,7 @@
 #include <linux/cred.h>
 #include <linux/xattr.h>
 #include <linux/exportfs.h>
+#include <linux/file.h>
 #include <linux/fileattr.h>
 #include <linux/uuid.h>
 #include <linux/namei.h>
@@ -1054,8 +1055,9 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir)
 	return -EIO;
 }
 
-/* err < 0, 0 if no metacopy xattr, 1 if metacopy xattr found */
-int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path)
+/* err < 0, 0 if no metacopy xattr, metacopy data size if xattr found */
+int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path,
+			     struct ovl_metacopy *data)
 {
 	int res;
 
@@ -1063,7 +1065,8 @@ int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path)
 	if (!S_ISREG(d_inode(path->dentry)->i_mode))
 		return 0;
 
-	res = ovl_path_getxattr(ofs, path, OVL_XATTR_METACOPY, NULL, 0);
+	res = ovl_path_getxattr(ofs, path, OVL_XATTR_METACOPY,
+				data, data ? OVL_METACOPY_MAX_SIZE : 0);
 	if (res < 0) {
 		if (res == -ENODATA || res == -EOPNOTSUPP)
 			return 0;
@@ -1077,12 +1080,48 @@ int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path)
 		goto out;
 	}
 
-	return 1;
+	if (res == 0) {
+		/* Emulate empty data for zero size metacopy xattr */
+		res = OVL_METACOPY_MIN_SIZE;
+		if (data) {
+			memset(data, 0, res);
+			data->len = res;
+		}
+	} else if (res < OVL_METACOPY_MIN_SIZE) {
+		pr_warn_ratelimited("metacopy file '%pd' has too small xattr\n",
+				    path->dentry);
+		return -EIO;
+	} else if (data) {
+		if (data->version != 0) {
+			pr_warn_ratelimited("metacopy file '%pd' has unsupported version\n",
+					    path->dentry);
+			return -EIO;
+		}
+		if (res != data->len) {
+			pr_warn_ratelimited("metacopy file '%pd' has invalid xattr size\n",
+					    path->dentry);
+			return -EIO;
+		}
+	}
+
+	return res;
 out:
 	pr_warn_ratelimited("failed to get metacopy (%i)\n", res);
 	return res;
 }
 
+int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d, struct ovl_metacopy *metacopy)
+{
+	size_t len = metacopy->len;
+
+	/* If no flags or digest fall back to empty metacopy file */
+	if (metacopy->version == 0 && metacopy->flags == 0 && metacopy->digest_algo == 0)
+		len = 0;
+
+	return ovl_check_setxattr(ofs, d, OVL_XATTR_METACOPY,
+				  metacopy, len, -EOPNOTSUPP);
+}
+
 bool ovl_is_metacopy_dentry(struct dentry *dentry)
 {
 	struct ovl_entry *oe = OVL_E(dentry);
@@ -1145,6 +1184,85 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int pa
 	return ERR_PTR(res);
 }
 
+/* Call with mounter creds as it may open the file */
+static int ovl_ensure_verity_loaded(struct path *datapath)
+{
+	struct inode *inode = d_inode(datapath->dentry);
+	const struct fsverity_info *vi;
+	struct file *filp;
+
+	vi = fsverity_get_info(inode);
+	if (vi == NULL && IS_VERITY(inode)) {
+		/*
+		 * If this inode was not yet opened, the verity info hasn't been
+		 * loaded yet, so we need to do that here to force it into memory.
+		 * We use open_with_fake_path to avoid ENFILE.
+		 */
+		filp = open_with_fake_path(datapath, O_RDONLY, inode, current_cred());
+		if (IS_ERR(filp))
+			return PTR_ERR(filp);
+		fput(filp);
+	}
+
+	return 0;
+}
+
+int ovl_validate_verity(struct ovl_fs *ofs,
+			struct path *metapath,
+			struct path *datapath)
+{
+	struct ovl_metacopy metacopy_data;
+	u8 actual_digest[FS_VERITY_MAX_DIGEST_SIZE];
+	u8 verity_algo;
+	int xattr_digest_size;
+	int digest_size;
+	int err;
+
+	if (!ofs->config.verity_mode ||
+	    /* Verity only works on regular files */
+	    !S_ISREG(d_inode(metapath->dentry)->i_mode))
+		return 0;
+
+	err = ovl_check_metacopy_xattr(ofs, metapath, &metacopy_data);
+	if (err < 0)
+		return err;
+
+	if (err == 0 || metacopy_data.digest_algo == 0) {
+		if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
+			pr_warn_ratelimited("metacopy file '%pd' has no digest specified\n",
+					    metapath->dentry);
+			return -EIO;
+		}
+		return 0;
+	}
+
+	xattr_digest_size = ovl_metadata_digest_size(&metacopy_data);
+
+	err = ovl_ensure_verity_loaded(datapath);
+	if (err < 0) {
+		pr_warn_ratelimited("lower file '%pd' failed to load fs-verity info\n",
+				    datapath->dentry);
+		return -EIO;
+	}
+
+	digest_size = fsverity_get_digest(d_inode(datapath->dentry), actual_digest,
+					  &verity_algo, NULL);
+	if (digest_size == 0) {
+		pr_warn_ratelimited("lower file '%pd' has no fs-verity digest\n", datapath->dentry);
+		return -EIO;
+	}
+
+	if (xattr_digest_size != digest_size ||
+	    metacopy_data.digest_algo != verity_algo ||
+	    memcmp(metacopy_data.digest, actual_digest, xattr_digest_size) != 0) {
+		pr_warn_ratelimited("lower file '%pd' has the wrong fs-verity digest\n",
+				    datapath->dentry);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 /*
  * ovl_sync_status() - Check fs sync status for volatile mounts
  *
-- 
2.40.1

Re: [PATCH v4 2/3] ovl: Validate verity xattr when resolving lowerdata

[Amir Goldstein]

On Tue, Jun 20, 2023 at 1:15 PM Alexander Larsson <alexl@redhat.com> wrote:
>
> The metacopy xattr is extended from currently empty to a versioned
> header with length, flags and an optional digest. During lookup we
> record whether the header contained a digest in the OVL_HAS_DIGEST
> flags.
>
> When accessing file data the first time, if OVL_HAS_DIGEST is set, we
> reload the metadata and that the source lowerdata inode matches the
> specified digest in it (according to the enabled verity options). If
> the verity check passes we store this info in the inode flags as
> OVL_VERIFIED_DIGEST, so that we can avoid doing it again if the inode
> remains in memory.
>
> The verification is done in ovl_maybe_validate_verity() which needs to
> be called in the same places as ovl_maybe_lookup_lowerdata(), so there
> is a new ovl_verify_lowerdata() helper that calls these in the right
> order, and all current callers of ovl_maybe_lookup_lowerdata() are
> changed to call it instead.
>
> Signed-off-by: Alexander Larsson <alexl@redhat.com>

Thanks for the on-disk format change.
Overall looking good
some small comments below

> ---
>  fs/overlayfs/copy_up.c   |   2 +-
>  fs/overlayfs/file.c      |   8 +--
>  fs/overlayfs/namei.c     |  97 +++++++++++++++++++++++++++---
>  fs/overlayfs/overlayfs.h |  43 ++++++++++---
>  fs/overlayfs/super.c     |   5 +-
>  fs/overlayfs/util.c      | 126 +++++++++++++++++++++++++++++++++++++--
>  6 files changed, 256 insertions(+), 25 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 568f743a5584..68f01fd7f211 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -1078,7 +1078,7 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags)
>          * not very important to optimize this case, so do lazy lowerdata lookup
>          * before any copy up, so we can do it before taking ovl_inode_lock().
>          */
> -       err = ovl_maybe_lookup_lowerdata(dentry);
> +       err = ovl_verify_lowerdata(dentry);
>         if (err)
>                 return err;
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 39737c2aaa84..6583d08fdb7a 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -115,8 +115,8 @@ static int ovl_real_fdget_meta(const struct file *file, struct fd *real,
>         if (allow_meta) {
>                 ovl_path_real(dentry, &realpath);
>         } else {
> -               /* lazy lookup of lowerdata */
> -               err = ovl_maybe_lookup_lowerdata(dentry);
> +               /* lazy lookup and verify of lowerdata */
> +               err = ovl_verify_lowerdata(dentry);
>                 if (err)
>                         return err;
>
> @@ -159,8 +159,8 @@ static int ovl_open(struct inode *inode, struct file *file)
>         struct path realpath;
>         int err;
>
> -       /* lazy lookup of lowerdata */
> -       err = ovl_maybe_lookup_lowerdata(dentry);
> +       /* lazy lookup and verify lowerdata */
> +       err = ovl_verify_lowerdata(dentry);
>         if (err)
>                 return err;
>
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index 57adf911735f..0ba8266a8125 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -26,6 +26,7 @@ struct ovl_lookup_data {
>         bool last;
>         char *redirect;
>         bool metacopy;
> +       bool metacopy_digest;

This extra bool state is not needed.
What I meant was:
-         bool metacopy;
-         int metacopy;

to be used as tristate:
0 - no metacopy
= OVL_METACOPY_MIN_SIZE - metacopy without digest
> OVL_METACOPY_MIN_SIZE - metacopy with digest

propagated all the way up
and then...

>         /* Referring to last redirect xattr */
>         bool absolute_redirect;
>  };
> @@ -233,6 +234,7 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
>  {
>         struct dentry *this;
>         struct path path;
> +       int metacopy_size;
>         int err;
>         bool last_element = !post[0];
>
> @@ -270,11 +272,14 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
>                         d->stop = true;
>                         goto put_and_out;
>                 }
> -               err = ovl_check_metacopy_xattr(OVL_FS(d->sb), &path);
> -               if (err < 0)
> +               metacopy_size = ovl_check_metacopy_xattr(OVL_FS(d->sb), &path, NULL);
> +               if (metacopy_size < 0) {
> +                       err = metacopy_size;
>                         goto out_err;
> +               }
>
> -               d->metacopy = err;
> +               d->metacopy = metacopy_size;
> +               d->metacopy_digest = d->metacopy && metacopy_size > OVL_METACOPY_MIN_SIZE;

No need for the above changes at all...

>                 d->stop = !d->metacopy;
>                 if (!d->metacopy || d->last)
>                         goto out;
> @@ -889,8 +894,59 @@ static int ovl_fix_origin(struct ovl_fs *ofs, struct dentry *dentry,
>         return err;
>  }
>
> +static int ovl_maybe_validate_verity(struct dentry *dentry)
> +{
> +       struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
> +       struct inode *inode = d_inode(dentry);
> +       struct path datapath, metapath;
> +       int err;
> +
> +       if (!ofs->config.verity_mode ||
> +           !ovl_is_metacopy_dentry(dentry) ||
> +           ovl_test_flag(OVL_VERIFIED_DIGEST, inode))
> +               return 0;
> +
> +       if (!ovl_test_flag(OVL_HAS_DIGEST, inode)) {
> +               if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
> +                       pr_warn_ratelimited("metacopy file '%pd' has no digest specified\n",
> +                                           dentry);
> +                       return -EIO;
> +               }
> +               return 0;
> +       }
> +
> +       ovl_path_lowerdata(dentry, &datapath);
> +       if (!datapath.dentry)
> +               return -EIO;
> +
> +       ovl_path_real(dentry, &metapath);
> +       if (!metapath.dentry)
> +               return -EIO;
> +
> +       err = ovl_inode_lock_interruptible(inode);
> +       if (err)
> +               return err;
> +
> +       if (ovl_test_flag(OVL_HAS_DIGEST, inode) &&
> +           !ovl_test_flag(OVL_VERIFIED_DIGEST, inode)) {
> +               const struct cred *old_cred;
> +
> +               old_cred = ovl_override_creds(dentry->d_sb);
> +
> +               err = ovl_validate_verity(ofs, &metapath, &datapath);
> +               if (err == 0)
> +                       ovl_set_flag(OVL_VERIFIED_DIGEST, inode);
> +
> +               revert_creds(old_cred);
> +       }
> +
> +       ovl_inode_unlock(inode);
> +
> +       return err;
> +}
> +
>  /* Lazy lookup of lowerdata */
> -int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
> +static int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
>  {
>         struct inode *inode = d_inode(dentry);
>         const char *redirect = ovl_lowerdata_redirect(inode);
> @@ -935,6 +991,17 @@ int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
>         goto out;
>  }
>
> +int ovl_verify_lowerdata(struct dentry *dentry)
> +{
> +       int err;
> +
> +       err = ovl_maybe_lookup_lowerdata(dentry);
> +       if (err)
> +               return err;
> +
> +       return ovl_maybe_validate_verity(dentry);
> +}
> +
>  struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                           unsigned int flags)
>  {
> @@ -952,9 +1019,11 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>         bool upperopaque = false;
>         char *upperredirect = NULL;
>         struct dentry *this;
> +       int metacopy_size;
>         unsigned int i;
>         int err;
>         bool uppermetacopy = false;
> +       bool metacopy_digest = false;

I think this can be rebranded as
int metacopy_size = 0;

which refers to any topmost metacopy either lower or upper.

>         struct ovl_lookup_data d = {
>                 .sb = dentry->d_sb,
>                 .name = dentry->d_name,
> @@ -964,6 +1033,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                 .last = ovl_redirect_follow(ofs) ? false : !ovl_numlower(poe),
>                 .redirect = NULL,
>                 .metacopy = false,
> +               .metacopy_digest = false,
>         };
>
>         if (dentry->d_name.len > ofs->namelen)
> @@ -997,8 +1067,10 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                         if (err)
>                                 goto out_put_upper;
>
> -                       if (d.metacopy)
> +                       if (d.metacopy) {
>                                 uppermetacopy = true;
> +                               metacopy_digest = d.metacopy_digest;
> +                       }

... this would just be (no need for if)
metacopy_size = d.metacopy;

>                 }
>
>                 if (d.redirect) {
> @@ -1076,6 +1148,9 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                         origin = this;
>                 }
>
> +               if (!upperdentry && !d.is_dir && !ctr && d.metacopy)
> +                       metacopy_digest = d.metacopy_digest;

Same here also (because it is distinguished from positive upperdentry case):
                            metacopy_size = d.metacopy;

> +
>                 if (d.metacopy && ctr) {
>                         /*
>                          * Do not store intermediate metacopy dentries in
> @@ -1211,10 +1286,13 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                         upperredirect = NULL;
>                         goto out_free_oe;
>                 }
> -               err = ovl_check_metacopy_xattr(ofs, &upperpath);
> -               if (err < 0)
> +               metacopy_size = ovl_check_metacopy_xattr(ofs, &upperpath, NULL);
> +               if (metacopy_size < 0) {
> +                       err = metacopy_size;
>                         goto out_free_oe;
> -               uppermetacopy = err;
> +               }
> +               uppermetacopy = metacopy_size;
> +               metacopy_digest = metacopy_size >  OVL_METACOPY_MIN_SIZE;

Same here.
No need for the rest of the changes.

>         }
>
>         if (upperdentry || ctr) {
> @@ -1236,6 +1314,9 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                         goto out_free_oe;
>                 if (upperdentry && !uppermetacopy)
>                         ovl_set_flag(OVL_UPPERDATA, inode);
> +
> +               if ((uppermetacopy || ctr > 1) && metacopy_digest)

Unless I am mistaken this simply becomes:

                     if (metacopy_size >  OVL_METACOPY_MIN_SIZE)
> +                       ovl_set_flag(OVL_HAS_DIGEST, inode);
>         }
>
>         ovl_dentry_init_reval(dentry, upperdentry, OVL_I_E(inode));
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 7414d6d8fb1c..c2213a8ad16e 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -7,6 +7,7 @@
>  #include <linux/kernel.h>
>  #include <linux/uuid.h>
>  #include <linux/fs.h>
> +#include <linux/fsverity.h>
>  #include <linux/namei.h>
>  #include <linux/posix_acl.h>
>  #include <linux/posix_acl_xattr.h>
> @@ -49,6 +50,8 @@ enum ovl_inode_flag {
>         OVL_UPPERDATA,
>         /* Inode number will remain constant over copy up. */
>         OVL_CONST_INO,
> +       OVL_HAS_DIGEST,
> +       OVL_VERIFIED_DIGEST,
>  };
>
>  enum ovl_entry_flag {
> @@ -141,6 +144,24 @@ struct ovl_fh {
>  #define OVL_FH_FID_OFFSET      (OVL_FH_WIRE_OFFSET + \
>                                  offsetof(struct ovl_fb, fid))
>
> +/* On-disk format for "metacopy" xattr (if non-zero size) */
> +struct ovl_metacopy {
> +       u8 version;     /* 0 */
> +       u8 len;         /* size of this header, not including unused digest bytes */

len is including the used digest bytes, so above it tad confusing
maybe /* size of this header + used digest bytes */ ?

> +       u8 flags;
> +       u8 digest_algo; /* FS_VERITY_HASH_ALG_* constant, 0 for no digest */
> +       u8 digest[FS_VERITY_MAX_DIGEST_SIZE];  /* Only the used part on disk */
> +} __packed;
> +
> +#define OVL_METACOPY_MAX_SIZE (sizeof(struct ovl_metacopy))
> +#define OVL_METACOPY_MIN_SIZE (OVL_METACOPY_MAX_SIZE - FS_VERITY_MAX_DIGEST_SIZE)
> +#define OVL_METACOPY_INIT { 0, OVL_METACOPY_MIN_SIZE }
> +
> +static inline int ovl_metadata_digest_size(const struct ovl_metacopy *metacopy)
> +{
> +       return (int)metacopy->len - OVL_METACOPY_MIN_SIZE;

Please fortify that to return 0 for len < OVL_METACOPY_MIN_SIZE

> +}
> +
>  extern const char *const ovl_xattr_table[][2];
>  static inline const char *ovl_xattr(struct ovl_fs *ofs, enum ovl_xattr ox)
>  {
> @@ -241,7 +262,7 @@ static inline ssize_t ovl_do_getxattr(const struct path *path, const char *name,
>         WARN_ON(path->dentry->d_sb != path->mnt->mnt_sb);
>
>         err = vfs_getxattr(mnt_idmap(path->mnt), path->dentry,
> -                              name, value, size);
> +                          name, value, size);
>         len = (value && err > 0) ? err : 0;
>
>         pr_debug("getxattr(%pd2, \"%s\", \"%*pE\", %zu, 0) = %i\n",
> @@ -263,9 +284,9 @@ static inline ssize_t ovl_getxattr_upper(struct ovl_fs *ofs,
>  }
>
>  static inline ssize_t ovl_path_getxattr(struct ovl_fs *ofs,
> -                                        const struct path *path,
> -                                        enum ovl_xattr ox, void *value,
> -                                        size_t size)
> +                                       const struct path *path,
> +                                       enum ovl_xattr ox, void *value,
> +                                       size_t size)
>  {
>         return ovl_do_getxattr(path, ovl_xattr(ofs, ox), value, size);
>  }
> @@ -352,7 +373,7 @@ static inline struct file *ovl_do_tmpfile(struct ovl_fs *ofs,
>  {
>         struct path path = { .mnt = ovl_upper_mnt(ofs), .dentry = dentry };
>         struct file *file = vfs_tmpfile_open(ovl_upper_mnt_idmap(ofs), &path, mode,
> -                                       O_LARGEFILE | O_WRONLY, current_cred());
> +                                            O_LARGEFILE | O_WRONLY, current_cred());
>         int err = PTR_ERR_OR_ZERO(file);
>
>         pr_debug("tmpfile(%pd2, 0%o) = %i\n", dentry, mode, err);
> @@ -490,9 +511,17 @@ bool ovl_need_index(struct dentry *dentry);
>  int ovl_nlink_start(struct dentry *dentry);
>  void ovl_nlink_end(struct dentry *dentry);
>  int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir);
> -int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path);
> +int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path,
> +                            struct ovl_metacopy *data);
> +int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d,
> +                          struct ovl_metacopy *metacopy);
>  bool ovl_is_metacopy_dentry(struct dentry *dentry);
>  char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int padding);
> +int ovl_get_verity_xattr(struct ovl_fs *ofs, const struct path *path,
> +                        u8 *digest_buf, int *buf_length);
> +int ovl_validate_verity(struct ovl_fs *ofs,
> +                       struct path *metapath,
> +                       struct path *datapath);
>  int ovl_sync_status(struct ovl_fs *ofs);
>
>  static inline void ovl_set_flag(unsigned long flag, struct inode *inode)
> @@ -612,7 +641,7 @@ struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh);
>  struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
>                                 struct dentry *origin, bool verify);
>  int ovl_path_next(int idx, struct dentry *dentry, struct path *path);
> -int ovl_maybe_lookup_lowerdata(struct dentry *dentry);
> +int ovl_verify_lowerdata(struct dentry *dentry);
>  struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                           unsigned int flags);
>  bool ovl_lower_positive(struct dentry *dentry);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 3f8bbd158a2a..2175e64d3b64 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -63,6 +63,7 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
>                                  const struct inode *inode)
>  {
>         struct dentry *real = NULL, *lower;
> +       int err;
>
>         /* It's an overlay file */
>         if (inode && d_inode(dentry) == inode)
> @@ -89,7 +90,9 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
>          * uprobes on offset within the file, so lowerdata should be available
>          * when setting the uprobe.
>          */
> -       ovl_maybe_lookup_lowerdata(dentry);
> +       err = ovl_verify_lowerdata(dentry);
> +       if (err)
> +               goto bug;
>         lower = ovl_dentry_lowerdata(dentry);
>         if (!lower)
>                 goto bug;
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 7ef9e13c404a..66448964f753 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -10,6 +10,7 @@
>  #include <linux/cred.h>
>  #include <linux/xattr.h>
>  #include <linux/exportfs.h>
> +#include <linux/file.h>
>  #include <linux/fileattr.h>
>  #include <linux/uuid.h>
>  #include <linux/namei.h>
> @@ -1054,8 +1055,9 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir)
>         return -EIO;
>  }
>
> -/* err < 0, 0 if no metacopy xattr, 1 if metacopy xattr found */
> -int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path)
> +/* err < 0, 0 if no metacopy xattr, metacopy data size if xattr found */

I guess we should document the emulated size return value.
Something like, "an empty xattr returns OVL_METACOPY_MIN_SIZE to
distinguish from no xattr value".

> +int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path,
> +                            struct ovl_metacopy *data)
>  {
>         int res;
>
> @@ -1063,7 +1065,8 @@ int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path)
>         if (!S_ISREG(d_inode(path->dentry)->i_mode))
>                 return 0;
>
> -       res = ovl_path_getxattr(ofs, path, OVL_XATTR_METACOPY, NULL, 0);
> +       res = ovl_path_getxattr(ofs, path, OVL_XATTR_METACOPY,
> +                               data, data ? OVL_METACOPY_MAX_SIZE : 0);
>         if (res < 0) {
>                 if (res == -ENODATA || res == -EOPNOTSUPP)
>                         return 0;
> @@ -1077,12 +1080,48 @@ int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path)
>                 goto out;
>         }
>
> -       return 1;
> +       if (res == 0) {
> +               /* Emulate empty data for zero size metacopy xattr */
> +               res = OVL_METACOPY_MIN_SIZE;
> +               if (data) {
> +                       memset(data, 0, res);
> +                       data->len = res;
> +               }
> +       } else if (res < OVL_METACOPY_MIN_SIZE) {
> +               pr_warn_ratelimited("metacopy file '%pd' has too small xattr\n",
> +                                   path->dentry);
> +               return -EIO;
> +       } else if (data) {
> +               if (data->version != 0) {
> +                       pr_warn_ratelimited("metacopy file '%pd' has unsupported version\n",
> +                                           path->dentry);
> +                       return -EIO;
> +               }
> +               if (res != data->len) {
> +                       pr_warn_ratelimited("metacopy file '%pd' has invalid xattr size\n",
> +                                           path->dentry);
> +                       return -EIO;
> +               }
> +       }
> +
> +       return res;
>  out:
>         pr_warn_ratelimited("failed to get metacopy (%i)\n", res);
>         return res;
>  }
>
> +int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d, struct ovl_metacopy *metacopy)
> +{
> +       size_t len = metacopy->len;
> +
> +       /* If no flags or digest fall back to empty metacopy file */
> +       if (metacopy->version == 0 && metacopy->flags == 0 && metacopy->digest_algo == 0)
> +               len = 0;
> +
> +       return ovl_check_setxattr(ofs, d, OVL_XATTR_METACOPY,
> +                                 metacopy, len, -EOPNOTSUPP);
> +}
> +
>  bool ovl_is_metacopy_dentry(struct dentry *dentry)
>  {
>         struct ovl_entry *oe = OVL_E(dentry);
> @@ -1145,6 +1184,85 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int pa
>         return ERR_PTR(res);
>  }
>
> +/* Call with mounter creds as it may open the file */
> +static int ovl_ensure_verity_loaded(struct path *datapath)
> +{
> +       struct inode *inode = d_inode(datapath->dentry);
> +       const struct fsverity_info *vi;
> +       struct file *filp;
> +
> +       vi = fsverity_get_info(inode);
> +       if (vi == NULL && IS_VERITY(inode)) {
> +               /*
> +                * If this inode was not yet opened, the verity info hasn't been
> +                * loaded yet, so we need to do that here to force it into memory.
> +                * We use open_with_fake_path to avoid ENFILE.
> +                */
> +               filp = open_with_fake_path(datapath, O_RDONLY, inode, current_cred());

You will also need to rebase on top of Christian's vfs.all branch in linux-next
where this helper is split into two different helpers.
You should use kernel_file_open() (same arguments).

For convenience, you can rebase on top of the 'next' branch in my
github which collects the vfs+fsnotify+fsverity+ovl branches from
linux-next ) or use a local branch of your own that does the same.


> +               if (IS_ERR(filp))
> +                       return PTR_ERR(filp);
> +               fput(filp);
> +       }
> +
> +       return 0;
> +}
> +
> +int ovl_validate_verity(struct ovl_fs *ofs,
> +                       struct path *metapath,
> +                       struct path *datapath)
> +{
> +       struct ovl_metacopy metacopy_data;
> +       u8 actual_digest[FS_VERITY_MAX_DIGEST_SIZE];
> +       u8 verity_algo;
> +       int xattr_digest_size;
> +       int digest_size;
> +       int err;
> +
> +       if (!ofs->config.verity_mode ||
> +           /* Verity only works on regular files */
> +           !S_ISREG(d_inode(metapath->dentry)->i_mode))
> +               return 0;
> +
> +       err = ovl_check_metacopy_xattr(ofs, metapath, &metacopy_data);
> +       if (err < 0)
> +               return err;
> +
> +       if (err == 0 || metacopy_data.digest_algo == 0) {

To me !metacopy_data.digest_algo reads nicer
but I will not insist.

> +               if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
> +                       pr_warn_ratelimited("metacopy file '%pd' has no digest specified\n",
> +                                           metapath->dentry);
> +                       return -EIO;
> +               }
> +               return 0;
> +       }
> +
> +       xattr_digest_size = ovl_metadata_digest_size(&metacopy_data);
> +
> +       err = ovl_ensure_verity_loaded(datapath);
> +       if (err < 0) {
> +               pr_warn_ratelimited("lower file '%pd' failed to load fs-verity info\n",
> +                                   datapath->dentry);
> +               return -EIO;
> +       }
> +
> +       digest_size = fsverity_get_digest(d_inode(datapath->dentry), actual_digest,
> +                                         &verity_algo, NULL);
> +       if (digest_size == 0) {
> +               pr_warn_ratelimited("lower file '%pd' has no fs-verity digest\n", datapath->dentry);
> +               return -EIO;
> +       }
> +
> +       if (xattr_digest_size != digest_size ||
> +           metacopy_data.digest_algo != verity_algo ||
> +           memcmp(metacopy_data.digest, actual_digest, xattr_digest_size) != 0) {
> +               pr_warn_ratelimited("lower file '%pd' has the wrong fs-verity digest\n",
> +                                   datapath->dentry);
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +

As I promised, just some small comments ;-)

Thanks,
Amir.

[PATCH v4 3/3] ovl: Handle verity during copy-up

[Alexander Larsson]

During regular metacopy, if lowerdata file has fs-verity enabled, and
the verity option is enabled, we add the digest to the metacopy xattr.

If verity is required, and lowerdata does not have fs-verity enabled,
fall back to full copy-up (or the generated metacopy would not
validate).

Signed-off-by: Alexander Larsson <alexl@redhat.com>
---
 fs/overlayfs/copy_up.c   | 45 ++++++++++++++++++++++++++++++++++++++--
 fs/overlayfs/overlayfs.h |  3 +++
 fs/overlayfs/util.c      | 32 +++++++++++++++++++++++++++-
 3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 68f01fd7f211..6e6c25836e52 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -544,6 +544,7 @@ struct ovl_copy_up_ctx {
 	bool origin;
 	bool indexed;
 	bool metacopy;
+	bool metacopy_digest;
 };
 
 static int ovl_link_up(struct ovl_copy_up_ctx *c)
@@ -641,8 +642,21 @@ static int ovl_copy_up_metadata(struct ovl_copy_up_ctx *c, struct dentry *temp)
 	}
 
 	if (c->metacopy) {
-		err = ovl_check_setxattr(ofs, temp, OVL_XATTR_METACOPY,
-					 NULL, 0, -EOPNOTSUPP);
+		struct path lowerdatapath;
+		struct ovl_metacopy metacopy_data = OVL_METACOPY_INIT;
+
+		ovl_path_lowerdata(c->dentry, &lowerdatapath);
+		if (WARN_ON_ONCE(lowerdatapath.dentry == NULL))
+			err = -EIO;
+		else
+			err = ovl_set_verity_xattr_from(ofs, &lowerdatapath, &metacopy_data);
+
+		if (metacopy_data.digest_algo != 0)
+			c->metacopy_digest = true;
+
+		if (!err)
+			err = ovl_set_metacopy_xattr(ofs, temp, &metacopy_data);
+
 		if (err)
 			return err;
 	}
@@ -751,6 +765,12 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
 	if (err)
 		goto cleanup;
 
+	if (c->metacopy_digest)
+		ovl_set_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
+	else
+		ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
+	ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry));
+
 	if (!c->metacopy)
 		ovl_set_upperdata(d_inode(c->dentry));
 	inode = d_inode(c->dentry);
@@ -813,6 +833,12 @@ static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c)
 	if (err)
 		goto out_fput;
 
+	if (c->metacopy_digest)
+		ovl_set_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
+	else
+		ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
+	ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry));
+
 	if (!c->metacopy)
 		ovl_set_upperdata(d_inode(c->dentry));
 	ovl_inode_update(d_inode(c->dentry), dget(temp));
@@ -918,6 +944,19 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
 	if (flags && ((OPEN_FMODE(flags) & FMODE_WRITE) || (flags & O_TRUNC)))
 		return false;
 
+	/* Fall back to full copy if no fsverity on source data and we require verity */
+	if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
+		struct path lowerdata;
+
+		ovl_path_lowerdata(dentry, &lowerdata);
+
+		if (WARN_ON_ONCE(lowerdata.dentry == NULL) ||
+		    ovl_ensure_verity_loaded(&lowerdata) ||
+		    !fsverity_get_info(d_inode(lowerdata.dentry))) {
+			return false;
+		}
+	}
+
 	return true;
 }
 
@@ -984,6 +1023,8 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
 	if (err)
 		goto out_free;
 
+	ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
+	ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry));
 	ovl_set_upperdata(d_inode(c->dentry));
 out_free:
 	kfree(capability);
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index c2213a8ad16e..eef4a3243e8a 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -517,11 +517,14 @@ int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d,
 			   struct ovl_metacopy *metacopy);
 bool ovl_is_metacopy_dentry(struct dentry *dentry);
 char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int padding);
+int ovl_ensure_verity_loaded(struct path *path);
 int ovl_get_verity_xattr(struct ovl_fs *ofs, const struct path *path,
 			 u8 *digest_buf, int *buf_length);
 int ovl_validate_verity(struct ovl_fs *ofs,
 			struct path *metapath,
 			struct path *datapath);
+int ovl_set_verity_xattr_from(struct ovl_fs *ofs, struct path *src,
+			      struct ovl_metacopy *metacopy);
 int ovl_sync_status(struct ovl_fs *ofs);
 
 static inline void ovl_set_flag(unsigned long flag, struct inode *inode)
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 66448964f753..3841f04baf35 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1185,7 +1185,7 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int pa
 }
 
 /* Call with mounter creds as it may open the file */
-static int ovl_ensure_verity_loaded(struct path *datapath)
+int ovl_ensure_verity_loaded(struct path *datapath)
 {
 	struct inode *inode = d_inode(datapath->dentry);
 	const struct fsverity_info *vi;
@@ -1263,6 +1263,36 @@ int ovl_validate_verity(struct ovl_fs *ofs,
 	return 0;
 }
 
+int ovl_set_verity_xattr_from(struct ovl_fs *ofs, struct path *src,
+			      struct ovl_metacopy *metacopy)
+{
+	int err, digest_size;
+
+	if (!ofs->config.verity_mode || !S_ISREG(d_inode(src->dentry)->i_mode))
+		return 0;
+
+	err = ovl_ensure_verity_loaded(src);
+	if (err < 0) {
+		pr_warn_ratelimited("lower file '%pd' failed to load fs-verity info\n",
+				    src->dentry);
+		return -EIO;
+	}
+
+	digest_size = fsverity_get_digest(d_inode(src->dentry),
+					  metacopy->digest, &metacopy->digest_algo, NULL);
+	if (digest_size == 0) {
+		if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
+			pr_warn_ratelimited("lower file '%pd' has no fs-verity digest\n",
+					    src->dentry);
+			return -EIO;
+		}
+		return 0;
+	}
+
+	metacopy->len += digest_size;
+	return 0;
+}
+
 /*
  * ovl_sync_status() - Check fs sync status for volatile mounts
  *
-- 
2.40.1

Re: [PATCH v4 3/3] ovl: Handle verity during copy-up

[Amir Goldstein]

On Tue, Jun 20, 2023 at 1:15 PM Alexander Larsson <alexl@redhat.com> wrote:
>
> During regular metacopy, if lowerdata file has fs-verity enabled, and
> the verity option is enabled, we add the digest to the metacopy xattr.
>
> If verity is required, and lowerdata does not have fs-verity enabled,
> fall back to full copy-up (or the generated metacopy would not
> validate).
>
> Signed-off-by: Alexander Larsson <alexl@redhat.com>

Looks good.
Minor comments below

> ---
>  fs/overlayfs/copy_up.c   | 45 ++++++++++++++++++++++++++++++++++++++--
>  fs/overlayfs/overlayfs.h |  3 +++
>  fs/overlayfs/util.c      | 32 +++++++++++++++++++++++++++-
>  3 files changed, 77 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 68f01fd7f211..6e6c25836e52 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -544,6 +544,7 @@ struct ovl_copy_up_ctx {
>         bool origin;
>         bool indexed;
>         bool metacopy;
> +       bool metacopy_digest;
>  };
>
>  static int ovl_link_up(struct ovl_copy_up_ctx *c)
> @@ -641,8 +642,21 @@ static int ovl_copy_up_metadata(struct ovl_copy_up_ctx *c, struct dentry *temp)
>         }
>
>         if (c->metacopy) {
> -               err = ovl_check_setxattr(ofs, temp, OVL_XATTR_METACOPY,
> -                                        NULL, 0, -EOPNOTSUPP);
> +               struct path lowerdatapath;
> +               struct ovl_metacopy metacopy_data = OVL_METACOPY_INIT;
> +
> +               ovl_path_lowerdata(c->dentry, &lowerdatapath);
> +               if (WARN_ON_ONCE(lowerdatapath.dentry == NULL))
> +                       err = -EIO;
> +               else
> +                       err = ovl_set_verity_xattr_from(ofs, &lowerdatapath, &metacopy_data);
> +
> +               if (metacopy_data.digest_algo != 0)

Nit: please drop "!= 0"

> +                       c->metacopy_digest = true;
> +
> +               if (!err)
> +                       err = ovl_set_metacopy_xattr(ofs, temp, &metacopy_data);
> +
>                 if (err)
>                         return err;
>         }
> @@ -751,6 +765,12 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
>         if (err)
>                 goto cleanup;
>
> +       if (c->metacopy_digest)
> +               ovl_set_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
> +       else
> +               ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
> +       ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry));
> +
>         if (!c->metacopy)
>                 ovl_set_upperdata(d_inode(c->dentry));
>         inode = d_inode(c->dentry);
> @@ -813,6 +833,12 @@ static int ovl_copy_up_tmpfile(struct ovl_copy_up_ctx *c)
>         if (err)
>                 goto out_fput;
>
> +       if (c->metacopy_digest)
> +               ovl_set_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
> +       else
> +               ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
> +       ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry));
> +
>         if (!c->metacopy)
>                 ovl_set_upperdata(d_inode(c->dentry));
>         ovl_inode_update(d_inode(c->dentry), dget(temp));
> @@ -918,6 +944,19 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
>         if (flags && ((OPEN_FMODE(flags) & FMODE_WRITE) || (flags & O_TRUNC)))
>                 return false;
>
> +       /* Fall back to full copy if no fsverity on source data and we require verity */
> +       if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
> +               struct path lowerdata;
> +
> +               ovl_path_lowerdata(dentry, &lowerdata);
> +
> +               if (WARN_ON_ONCE(lowerdata.dentry == NULL) ||
> +                   ovl_ensure_verity_loaded(&lowerdata) ||
> +                   !fsverity_get_info(d_inode(lowerdata.dentry))) {
> +                       return false;
> +               }
> +       }
> +
>         return true;
>  }
>
> @@ -984,6 +1023,8 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
>         if (err)
>                 goto out_free;
>
> +       ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
> +       ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry));
>         ovl_set_upperdata(d_inode(c->dentry));
>  out_free:
>         kfree(capability);
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index c2213a8ad16e..eef4a3243e8a 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -517,11 +517,14 @@ int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d,
>                            struct ovl_metacopy *metacopy);
>  bool ovl_is_metacopy_dentry(struct dentry *dentry);
>  char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int padding);
> +int ovl_ensure_verity_loaded(struct path *path);
>  int ovl_get_verity_xattr(struct ovl_fs *ofs, const struct path *path,
>                          u8 *digest_buf, int *buf_length);
>  int ovl_validate_verity(struct ovl_fs *ofs,
>                         struct path *metapath,
>                         struct path *datapath);
> +int ovl_set_verity_xattr_from(struct ovl_fs *ofs, struct path *src,
> +                             struct ovl_metacopy *metacopy);
>  int ovl_sync_status(struct ovl_fs *ofs);
>
>  static inline void ovl_set_flag(unsigned long flag, struct inode *inode)
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 66448964f753..3841f04baf35 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -1185,7 +1185,7 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int pa
>  }
>
>  /* Call with mounter creds as it may open the file */
> -static int ovl_ensure_verity_loaded(struct path *datapath)
> +int ovl_ensure_verity_loaded(struct path *datapath)
>  {
>         struct inode *inode = d_inode(datapath->dentry);
>         const struct fsverity_info *vi;
> @@ -1263,6 +1263,36 @@ int ovl_validate_verity(struct ovl_fs *ofs,
>         return 0;
>  }
>
> +int ovl_set_verity_xattr_from(struct ovl_fs *ofs, struct path *src,
> +                             struct ovl_metacopy *metacopy)
> +{
> +       int err, digest_size;
> +
> +       if (!ofs->config.verity_mode || !S_ISREG(d_inode(src->dentry)->i_mode))
> +               return 0;
> +
> +       err = ovl_ensure_verity_loaded(src);
> +       if (err < 0) {
> +               pr_warn_ratelimited("lower file '%pd' failed to load fs-verity info\n",
> +                                   src->dentry);
> +               return -EIO;
> +       }
> +
> +       digest_size = fsverity_get_digest(d_inode(src->dentry),
> +                                         metacopy->digest, &metacopy->digest_algo, NULL);
> +       if (digest_size == 0) {
|| WARN_ON_ONCE(digest_size > FS_VERITY_MAX_DIGEST_SIZE))

> +               if (ofs->config.verity_mode == OVL_VERITY_REQUIRE) {
> +                       pr_warn_ratelimited("lower file '%pd' has no fs-verity digest\n",
> +                                           src->dentry);
> +                       return -EIO;
> +               }
> +               return 0;
> +       }
> +

ovl and fsverity are different modules, so we should have this
fail safety assertion in place.

It may look ridiculous that fsverity_get_digest() would return a size
larger than their own constant, but people may be loading an overlayfs
module not compiled with exact same fsverity code.

The results of this may be undefined, but at least we can make
the problem heard.

Thanks,
Amir.

Re: [PATCH v4 0/3] ovl: Add support for fs-verity checking of lowerdata

[Eric Biggers]

On Tue, Jun 20, 2023 at 12:15:15PM +0200, Alexander Larsson wrote:
> This series depends on the commit
>   fsverity: rework fsverity_get_digest() again
> Which is in the "for-next" branch of 
>   https://git.kernel.org/pub/scm/fs/fsverity/linux.git/
> 
> This series, plus the above commit are also in git here:
>   https://github.com/alexlarsson/linux/tree/overlay-verity
> 
> I would love to see this go into 6.5. So Eric, could you maybe Ack the
> implementation patches separately from the documentation patches? Then
> maybe we can get this in early, and I promise to try to get the
> documentation up to standard during the 6.5 cycle as needed.

I think it's gotten too late for 6.5.  If there is no 6.4-rc8, then the 6.5
merge window will open just 5 days from now.  This series has recently gone
through some significant changes, including in the version just sent out today
which I haven't had a chance to review yet.

Please don't try to rush things in when they involve UAPI and on-disk format
changes, which will have to be supported forever.  We need to take the time to
get them right.
 
I also see that the overlayfs tree is already very busy in 6.5, with the support
for data-only lower layers, lazy lookup of lowerdata, and the new mount API.

I think 6.6 would be a more realistic target.  That would give time to write
proper documentation as well, which is super important.  (Very often while
writing documentation, I realize that I should do something differently in the
code.  Please don't think of documentation as something can be done "later".)

- Eric

Re: [PATCH v4 0/3] ovl: Add support for fs-verity checking of lowerdata

[Alexander Larsson]

On Tue, Jun 20, 2023 at 6:15 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Jun 20, 2023 at 12:15:15PM +0200, Alexander Larsson wrote:
> > This series depends on the commit
> >   fsverity: rework fsverity_get_digest() again
> > Which is in the "for-next" branch of
> >   https://git.kernel.org/pub/scm/fs/fsverity/linux.git/
> >
> > This series, plus the above commit are also in git here:
> >   https://github.com/alexlarsson/linux/tree/overlay-verity
> >
> > I would love to see this go into 6.5. So Eric, could you maybe Ack the
> > implementation patches separately from the documentation patches? Then
> > maybe we can get this in early, and I promise to try to get the
> > documentation up to standard during the 6.5 cycle as needed.
>
> I think it's gotten too late for 6.5.  If there is no 6.4-rc8, then the 6.5
> merge window will open just 5 days from now.  This series has recently gone
> through some significant changes, including in the version just sent out today
> which I haven't had a chance to review yet.
>
> Please don't try to rush things in when they involve UAPI and on-disk format
> changes, which will have to be supported forever.  We need to take the time to
> get them right.
>
> I also see that the overlayfs tree is already very busy in 6.5, with the support
> for data-only lower layers, lazy lookup of lowerdata, and the new mount API.
>
> I think 6.6 would be a more realistic target.  That would give time to write
> proper documentation as well, which is super important.  (Very often while
> writing documentation, I realize that I should do something differently in the
> code.  Please don't think of documentation as something can be done "later".)

If 6.6 is what ends up happening I'm not gonna protest, it's not a
huge issue for me, only mildly inconvenient. But, for now I'll at
least keep targeting 6.5, and then we will have to see how it works
out wrt reviews and what Miklos decides.

I pushed out a v5 series today too, because the v4 series conflicted
with some other changes in vfs.all that are staged for 6.5. v5 is also
a bit simplified based on Amirs feedback, has some documentation
updates and is refactored into more commits for easier review.

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                Red Hat, Inc
       alexl@redhat.com         alexander.larsson@gmail.com

Re: [PATCH v4 0/3] ovl: Add support for fs-verity checking of lowerdata

[Amir Goldstein]

On Wed, Jun 21, 2023 at 2:27 PM Alexander Larsson <alexl@redhat.com> wrote:
>
> On Tue, Jun 20, 2023 at 6:15 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Tue, Jun 20, 2023 at 12:15:15PM +0200, Alexander Larsson wrote:
> > > This series depends on the commit
> > >   fsverity: rework fsverity_get_digest() again
> > > Which is in the "for-next" branch of
> > >   https://git.kernel.org/pub/scm/fs/fsverity/linux.git/
> > >
> > > This series, plus the above commit are also in git here:
> > >   https://github.com/alexlarsson/linux/tree/overlay-verity
> > >
> > > I would love to see this go into 6.5. So Eric, could you maybe Ack the
> > > implementation patches separately from the documentation patches? Then
> > > maybe we can get this in early, and I promise to try to get the
> > > documentation up to standard during the 6.5 cycle as needed.
> >
> > I think it's gotten too late for 6.5.  If there is no 6.4-rc8, then the 6.5
> > merge window will open just 5 days from now.  This series has recently gone
> > through some significant changes, including in the version just sent out today
> > which I haven't had a chance to review yet.
> >
> > Please don't try to rush things in when they involve UAPI and on-disk format
> > changes, which will have to be supported forever.  We need to take the time to
> > get them right.
> >
> > I also see that the overlayfs tree is already very busy in 6.5, with the support
> > for data-only lower layers, lazy lookup of lowerdata, and the new mount API.
> >
> > I think 6.6 would be a more realistic target.  That would give time to write
> > proper documentation as well, which is super important.  (Very often while
> > writing documentation, I realize that I should do something differently in the
> > code.  Please don't think of documentation as something can be done "later".)
>
> If 6.6 is what ends up happening I'm not gonna protest, it's not a
> huge issue for me, only mildly inconvenient. But, for now I'll at
> least keep targeting 6.5, and then we will have to see how it works
> out wrt reviews and what Miklos decides.
>
> I pushed out a v5 series today too, because the v4 series conflicted
> with some other changes in vfs.all that are staged for 6.5. v5 is also
> a bit simplified based on Amirs feedback, has some documentation
> updates and is refactored into more commits for easier review.
>

I reviewed v5 and it is all fine by me, but I do agree with Eric that
it has become quite late for 6.5 and other reviewers need to get
enough time to review v5, so no need to rush.

I also need some time to test verity feature which I hadn't had
the chance to do yet, so it looks like the stars are aligned for 6.6.
I am planning to be on vacation around 6.5-rc2..6.5-rc6 -
because of your efforts to get the patches ready in time for 6.5,
I will now have time to test your patches before -rc6, so your
efforts have not been in vain...

Thanks,
Amir.