On Thu, Jul 11, 2024 at 7:05 AM Mike Baynton <mike@xxxxxxxxxxxx> wrote:
>
> Some overlayfs features require permission to read/write trusted.*
> xattrs. These include redirect_dir, verity, metacopy, and data-only
> layers. This patch adds additional validations at mount time to stop
> overlays from mounting in certain cases where the resulting mount would
> not function according to the user's expectations because they lack
> permission to access trusted.* xattrs (for example, not global root.)
>
> Similar checks in ovl_make_workdir() that disable features instead of
> failing are still relevant and used in cases where the resulting mount
> can still work "reasonably well." Generally, if the feature was enabled
> through kernel config or module option, any mount that worked before
> will still work the same; this applies to redirect_dir and metacopy. The
> user must explicitly request these features in order to generate a mount
> failure. Verity and data-only layers on the other hand must be explictly
> requested and have no "reasonable" disabled or degraded alternative, so
> mounts attempting either always fail.
>
> "lower data-only dirs require metacopy support" moved down in case
> userxattr is set, which disables metacopy.
>
> Signed-off-by: Mike Baynton <mike@xxxxxxxxxxxx>
Looks nice
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
>
> v1 -> v2 not specific to data-only layers, punt on metacopy disable
> due to xattr write errors creating a conflicting configuration
> when data-only layers are present.
>
> fs/overlayfs/params.c | 39 +++++++++++++++++++++++++++++++++------
> 1 file changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> index 4860fcc4611b..107c43e5e4cb 100644
> --- a/fs/overlayfs/params.c
> +++ b/fs/overlayfs/params.c
> @@ -782,11 +782,6 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
> {
> struct ovl_opt_set set = ctx->set;
>
> - if (ctx->nr_data > 0 && !config->metacopy) {
> - pr_err("lower data-only dirs require metacopy support.\n");
> - return -EINVAL;
> - }
> -
> /* Workdir/index are useless in non-upper mount */
> if (!config->upperdir) {
> if (config->workdir) {
> @@ -910,7 +905,6 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
> }
> }
>
> -
> /* Resolve userxattr -> !redirect && !metacopy && !verity dependency */
> if (config->userxattr) {
> if (set.redirect &&
> @@ -938,6 +932,39 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
> config->metacopy = false;
> }
>
> + /*
> + * Fail if we don't have trusted xattr capability and a feature was
> + * explicitly requested that requires them.
> + */
> + if (!config->userxattr && !capable(CAP_SYS_ADMIN)) {
> + if (set.redirect &&
> + config->redirect_mode != OVL_REDIRECT_NOFOLLOW) {
> + pr_err("redirect_dir requires permission to access trusted xattrs\n");
> + return -EPERM;
> + }
> + if (config->metacopy && set.metacopy) {
> + pr_err("metacopy requires permission to access trusted xattrs\n");
> + return -EPERM;
> + }
> + if (config->verity_mode) {
> + pr_err("verity requires permission to access trusted xattrs\n");
> + return -EPERM;
> + }
> + if (ctx->nr_data > 0) {
> + pr_err("lower data-only dirs require permission to access trusted xattrs\n");
> + return -EPERM;
> + }
> + /*
> + * Other xattr-dependent features should be disabled without
> + * great disturbance to the user in ovl_make_workdir().
> + */
> + }
> +
> + if (ctx->nr_data > 0 && !config->metacopy) {
> + pr_err("lower data-only dirs require metacopy support.\n");
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> --
> 2.34.1
>
>
