diff mbox series

[v3,3/3] ovl: don't require "metacopy=on" for "verity"

Message ID 20250408154011.673891-4-mszeredi@redhat.com (mailing list archive)
State New
Headers show
Series ovl: metacopy/verity fixes and improvements | expand

Commit Message

Miklos Szeredi April 8, 2025, 3:40 p.m. UTC
This allows the "verity" mount option to be used with "userxattr" data-only
layer(s).

Also it allows dropping the "metacopy=on" option when the "datadir+" option
is to be used.  This cleanly separates the two features that have been
lumped together under "metacopy=on":

 - data-redirect: data access is redirected to the data-only layer

 - meta-copy: copy up metadata only if possible

Previous patches made sure that with "userxattr" metacopy only works in the
lower -> data scenario.

In this scenario the lower (metadata) layer must be secured against
tampering, in which case the verity checksums contained in this layer can
ensure integrity of data even in the case of an untrusted data layer.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/overlayfs/params.c | 26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

Comments

Amir Goldstein April 9, 2025, 6:12 a.m. UTC | #1
On Tue, Apr 8, 2025 at 5:40 PM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> This allows the "verity" mount option to be used with "userxattr" data-only
> layer(s).
>
> Also it allows dropping the "metacopy=on" option when the "datadir+" option
> is to be used.  This cleanly separates the two features that have been
> lumped together under "metacopy=on":
>
>  - data-redirect: data access is redirected to the data-only layer
>
>  - meta-copy: copy up metadata only if possible
>
> Previous patches made sure that with "userxattr" metacopy only works in the
> lower -> data scenario.
>
> In this scenario the lower (metadata) layer must be secured against
> tampering, in which case the verity checksums contained in this layer can
> ensure integrity of data even in the case of an untrusted data layer.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>


> ---
>  fs/overlayfs/params.c | 26 ++------------------------
>  1 file changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> index 2468b436bb13..e297681ecac7 100644
> --- a/fs/overlayfs/params.c
> +++ b/fs/overlayfs/params.c
> @@ -871,18 +871,6 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>                 config->uuid = OVL_UUID_NULL;
>         }
>
> -       /* 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.
> @@ -890,18 +878,13 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>         if (!config->upperdir && config->redirect_mode == OVL_REDIRECT_FOLLOW)
>                 config->redirect_mode = OVL_REDIRECT_ON;
>
> -       /* Resolve verity -> metacopy -> redirect_dir dependency */
> +       /* metacopy -> redirect_dir dependency */
>         if (config->metacopy && config->redirect_mode != OVL_REDIRECT_ON) {
>                 if (set.metacopy && set.redirect) {
>                         pr_err("conflicting options: metacopy=on,redirect_dir=%s\n",
>                                ovl_redirect_mode(config));
>                         return -EINVAL;
>                 }
> -               if (config->verity_mode && 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
> @@ -970,7 +953,7 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>         }
>
>
> -       /* Resolve userxattr -> !redirect && !metacopy && !verity dependency */
> +       /* Resolve userxattr -> !redirect && !metacopy dependency */
>         if (config->userxattr) {
>                 if (set.redirect &&
>                     config->redirect_mode != OVL_REDIRECT_NOFOLLOW) {
> @@ -982,11 +965,6 @@ 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.49.0
>
diff mbox series

Patch

diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
index 2468b436bb13..e297681ecac7 100644
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -871,18 +871,6 @@  int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 		config->uuid = OVL_UUID_NULL;
 	}
 
-	/* 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.
@@ -890,18 +878,13 @@  int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 	if (!config->upperdir && config->redirect_mode == OVL_REDIRECT_FOLLOW)
 		config->redirect_mode = OVL_REDIRECT_ON;
 
-	/* Resolve verity -> metacopy -> redirect_dir dependency */
+	/* metacopy -> redirect_dir dependency */
 	if (config->metacopy && config->redirect_mode != OVL_REDIRECT_ON) {
 		if (set.metacopy && set.redirect) {
 			pr_err("conflicting options: metacopy=on,redirect_dir=%s\n",
 			       ovl_redirect_mode(config));
 			return -EINVAL;
 		}
-		if (config->verity_mode && 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
@@ -970,7 +953,7 @@  int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 	}
 
 
-	/* Resolve userxattr -> !redirect && !metacopy && !verity dependency */
+	/* Resolve userxattr -> !redirect && !metacopy dependency */
 	if (config->userxattr) {
 		if (set.redirect &&
 		    config->redirect_mode != OVL_REDIRECT_NOFOLLOW) {
@@ -982,11 +965,6 @@  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