On Mon, Feb 10, 2025 at 8:45 PM Miklos Szeredi <mszeredi@xxxxxxxxxx> wrote:
>
> Allow the "verity" mount option to be used with "userxattr" data-only
> layer(s).
This standalone sentence sounds like a security risk,
because unpriv users could change the verity digest.
I suggest explaining it better.
>
> Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
> ---
> fs/overlayfs/params.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> index 54468b2b0fba..7300ed904e6d 100644
> --- a/fs/overlayfs/params.c
> +++ b/fs/overlayfs/params.c
> @@ -846,8 +846,8 @@ 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) {
> + /* Resolve verity -> metacopy dependency (unless used with userxattr) */
> + if (config->verity_mode && !config->metacopy && !config->userxattr) {
> /* Don't allow explicit specified conflicting combinations */
> if (set.metacopy) {
> pr_err("conflicting options: metacopy=off,verity=%s\n",
> @@ -945,7 +945,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) {
> @@ -957,11 +957,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
> @@ -986,10 +981,6 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
> 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;
> - }
This looks wrong.
I don't think you meant to change the case of
(!config->userxattr && !capable(CAP_SYS_ADMIN))
Thanks,
Amir.
