diff mbox series

[4/5] ovl: don't require metacopy=on for lower -> data redirect

Message ID 20250210194512.417339-4-mszeredi@redhat.com (mailing list archive)
State New
Headers show
Series [1/5] ovl: don't allow datadir only | expand

Commit Message

Miklos Szeredi Feb. 10, 2025, 7:45 p.m. UTC
Allow the special case of a redirect from a lower layer to a data layer
without having to turn on metacopy.  This makes the feature work with
userxattr, which in turn allows data layers to be usable in user
namespaces.

Minimize the risk by only enabling redirect from a single lower layer to a
data layer iff a data layer is specified.  The only way to access a data
layer is to enable this, so there's really no reason no to enable this.

This can be used safely if the lower layer is read-only and the
user.overlay.redirect xattr cannot be modified.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 Documentation/filesystems/overlayfs.rst |  7 ++++++
 fs/overlayfs/namei.c                    | 32 ++++++++++++++-----------
 fs/overlayfs/params.c                   |  5 ----
 3 files changed, 25 insertions(+), 19 deletions(-)

Comments

Amir Goldstein Feb. 11, 2025, 12:08 p.m. UTC | #1
> Subject: [PATCH] ovl: don't require metacopy=on for lower -> data redirect

Nit: the change does not require metacopy=on nor redirect_dir=follow
so maybe some short title like:

ovl: relax redirect/metacopy requirements for lower -> data redirect


> Allow the special case of a redirect from a lower layer to a data layer
> without having to turn on metacopy.  This makes the feature work with
> userxattr, which in turn allows data layers to be usable in user
> namespaces.
>
> Minimize the risk by only enabling redirect from a single lower layer to a
> data layer iff a data layer is specified.  The only way to access a data
> layer is to enable this, so there's really no reason no to enable this.
>
> This can be used safely if the lower layer is read-only and the
> user.overlay.redirect xattr cannot be modified.
>

Just need to verify that those assumptions are not broken by
upper index with metacopy/redirect -
I think the safety of redirect still holds, but not the safety
of the verity digest.

Thanks,
Amir.

> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  Documentation/filesystems/overlayfs.rst |  7 ++++++
>  fs/overlayfs/namei.c                    | 32 ++++++++++++++-----------
>  fs/overlayfs/params.c                   |  5 ----
>  3 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index 6245b67ae9e0..5d277d79cf2f 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -429,6 +429,13 @@ Only the data of the files in the "data-only" lower layers may be visible
>  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.
>
> +Instead of explicitly enabling "metacopy=on" it is sufficient to specify at
> +least one data-only layer to enable redirection of data to a data-only layer.
> +In this case other forms of metacopy are rejected.  Note: this way data-only
> +layers may be used toghether with "userxattr", in which case careful attention
> +must be given to privileges needed to change the "user.overlay.redirect" xattr
> +to prevent misuse.
> +
>  Since kernel version v6.8, "data-only" lower layers can also be added using
>  the "datadir+" mount options and the fsconfig syscall from new mount api.
>  For example::
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index da322e9768d1..f9dc71b70beb 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -1042,6 +1042,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>         char *upperredirect = NULL;
>         bool nextredirect = false;
>         bool nextmetacopy = false;
> +       bool check_redirect = (ovl_redirect_follow(ofs) || ofs->numdatalayer);
>         struct dentry *this;
>         unsigned int i;
>         int err;
> @@ -1053,7 +1054,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                 .is_dir = false,
>                 .opaque = false,
>                 .stop = false,
> -               .last = ovl_redirect_follow(ofs) ? false : !ovl_numlower(poe),
> +               .last = check_redirect ? false : !ovl_numlower(poe),
>                 .redirect = NULL,
>                 .metacopy = 0,
>         };
> @@ -1141,7 +1142,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                         goto out_put;
>                 }
>
> -               if (!ovl_redirect_follow(ofs))
> +               if (!check_redirect)
>                         d.last = i == ovl_numlower(poe) - 1;
>                 else if (d.is_dir || !ofs->numdatalayer)
>                         d.last = lower.layer->idx == ovl_numlower(roe);
> @@ -1222,21 +1223,24 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                 }
>         }
>
> -       /* Defer lookup of lowerdata in data-only layers to first access */
> +       /*
> +        * Defer lookup of lowerdata in data-only layers to first access.
> +        * Don't require redirect=follow and metacopy=on in this case.
> +        */
>         if (d.metacopy && ctr && ofs->numdatalayer && d.absolute_redirect) {
>                 d.metacopy = 0;
>                 ctr++;
> -       }
> -
> -       if (nextmetacopy && !ofs->config.metacopy) {
> -               pr_warn_ratelimited("refusing to follow metacopy origin for (%pd2)\n", dentry);
> -               err = -EPERM;
> -               goto out_put;
> -       }
> -       if (nextredirect && !ovl_redirect_follow(ofs)) {
> -               pr_warn_ratelimited("refusing to follow redirect for (%pd2)\n", dentry);
> -               err = -EPERM;
> -               goto out_put;
> +       } else {
> +               if (nextmetacopy && !ofs->config.metacopy) {
> +                       pr_warn_ratelimited("refusing to follow metacopy origin for (%pd2)\n", dentry);
> +                       err = -EPERM;
> +                       goto out_put;
> +               }
> +               if (nextredirect && !ovl_redirect_follow(ofs)) {
> +                       pr_warn_ratelimited("refusing to follow redirect for (%pd2)\n", dentry);
> +                       err = -EPERM;
> +                       goto out_put;
> +               }
>         }
>
>         /*
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> index 1115c22deca0..54468b2b0fba 100644
> --- a/fs/overlayfs/params.c
> +++ b/fs/overlayfs/params.c
> @@ -1000,11 +1000,6 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>                  */
>         }
>
> -       if (ctx->nr_data > 0 && !config->metacopy) {
> -               pr_err("lower data-only dirs require metacopy support.\n");
> -               return -EINVAL;
> -       }
> -
>         return 0;
>  }
>
> --
> 2.48.1
>
diff mbox series

Patch

diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
index 6245b67ae9e0..5d277d79cf2f 100644
--- a/Documentation/filesystems/overlayfs.rst
+++ b/Documentation/filesystems/overlayfs.rst
@@ -429,6 +429,13 @@  Only the data of the files in the "data-only" lower layers may be visible
 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.
 
+Instead of explicitly enabling "metacopy=on" it is sufficient to specify at
+least one data-only layer to enable redirection of data to a data-only layer.
+In this case other forms of metacopy are rejected.  Note: this way data-only
+layers may be used toghether with "userxattr", in which case careful attention
+must be given to privileges needed to change the "user.overlay.redirect" xattr
+to prevent misuse.
+
 Since kernel version v6.8, "data-only" lower layers can also be added using
 the "datadir+" mount options and the fsconfig syscall from new mount api.
 For example::
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index da322e9768d1..f9dc71b70beb 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -1042,6 +1042,7 @@  struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	char *upperredirect = NULL;
 	bool nextredirect = false;
 	bool nextmetacopy = false;
+	bool check_redirect = (ovl_redirect_follow(ofs) || ofs->numdatalayer);
 	struct dentry *this;
 	unsigned int i;
 	int err;
@@ -1053,7 +1054,7 @@  struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		.is_dir = false,
 		.opaque = false,
 		.stop = false,
-		.last = ovl_redirect_follow(ofs) ? false : !ovl_numlower(poe),
+		.last = check_redirect ? false : !ovl_numlower(poe),
 		.redirect = NULL,
 		.metacopy = 0,
 	};
@@ -1141,7 +1142,7 @@  struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			goto out_put;
 		}
 
-		if (!ovl_redirect_follow(ofs))
+		if (!check_redirect)
 			d.last = i == ovl_numlower(poe) - 1;
 		else if (d.is_dir || !ofs->numdatalayer)
 			d.last = lower.layer->idx == ovl_numlower(roe);
@@ -1222,21 +1223,24 @@  struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		}
 	}
 
-	/* Defer lookup of lowerdata in data-only layers to first access */
+	/*
+	 * Defer lookup of lowerdata in data-only layers to first access.
+	 * Don't require redirect=follow and metacopy=on in this case.
+	 */
 	if (d.metacopy && ctr && ofs->numdatalayer && d.absolute_redirect) {
 		d.metacopy = 0;
 		ctr++;
-	}
-
-	if (nextmetacopy && !ofs->config.metacopy) {
-		pr_warn_ratelimited("refusing to follow metacopy origin for (%pd2)\n", dentry);
-		err = -EPERM;
-		goto out_put;
-	}
-	if (nextredirect && !ovl_redirect_follow(ofs)) {
-		pr_warn_ratelimited("refusing to follow redirect for (%pd2)\n", dentry);
-		err = -EPERM;
-		goto out_put;
+	} else {
+		if (nextmetacopy && !ofs->config.metacopy) {
+			pr_warn_ratelimited("refusing to follow metacopy origin for (%pd2)\n", dentry);
+			err = -EPERM;
+			goto out_put;
+		}
+		if (nextredirect && !ovl_redirect_follow(ofs)) {
+			pr_warn_ratelimited("refusing to follow redirect for (%pd2)\n", dentry);
+			err = -EPERM;
+			goto out_put;
+		}
 	}
 
 	/*
diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
index 1115c22deca0..54468b2b0fba 100644
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -1000,11 +1000,6 @@  int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
 		 */
 	}
 
-	if (ctx->nr_data > 0 && !config->metacopy) {
-		pr_err("lower data-only dirs require metacopy support.\n");
-		return -EINVAL;
-	}
-
 	return 0;
 }