linux-kernel - Re: [PATCH 01/13] vfs: verify param type in vfs_parse_sb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJfpegv_ezsXOLV2f7yd07=T3MenJoMKhu=MBac1-80s0BFg9A@mail.gmail.com>
Date:   Mon, 1 Jul 2019 10:45:53 +0200
From:   Miklos Szeredi <miklos@...redi.hu>
To:     David Howells <dhowells@...hat.com>
Cc:     Al Viro <viro@...iv.linux.org.uk>,
        Miklos Szeredi <mszeredi@...hat.com>,
        Ian Kent <raven@...maw.net>,
        Linux API <linux-api@...r.kernel.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/13] vfs: verify param type in vfs_parse_sb_flag()

Hi David,

Ping?  Have you had a chance of looking at this series?

Köszi,
Miklos

On Wed, Jun 19, 2019 at 2:30 PM Miklos Szeredi <mszeredi@...hat.com> wrote:
>
> vfs_parse_sb_flag() accepted any kind of param with a matching key, not
> just a flag.  This is wrong, only allow flag type and return -EINVAL
> otherwise.
>
> Signed-off-by: Miklos Szeredi <mszeredi@...hat.com>
> ---
>  fs/fs_context.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/fs/fs_context.c b/fs/fs_context.c
> index 103643c68e3f..e56310fd8c75 100644
> --- a/fs/fs_context.c
> +++ b/fs/fs_context.c
> @@ -81,30 +81,29 @@ static const char *const forbidden_sb_flag[] = {
>  /*
>   * Check for a common mount option that manipulates s_flags.
>   */
> -static int vfs_parse_sb_flag(struct fs_context *fc, const char *key)
> +static int vfs_parse_sb_flag(struct fs_context *fc, struct fs_parameter *param)
>  {
> -       unsigned int token;
> +       const char *key = param->key;
> +       unsigned int set, clear;
>         unsigned int i;
>
>         for (i = 0; i < ARRAY_SIZE(forbidden_sb_flag); i++)
>                 if (strcmp(key, forbidden_sb_flag[i]) == 0)
>                         return -EINVAL;
>
> -       token = lookup_constant(common_set_sb_flag, key, 0);
> -       if (token) {
> -               fc->sb_flags |= token;
> -               fc->sb_flags_mask |= token;
> -               return 0;
> -       }
> +       set = lookup_constant(common_set_sb_flag, key, 0);
> +       clear = lookup_constant(common_clear_sb_flag, key, 0);
> +       if (!set && !clear)
> +               return -ENOPARAM;
>
> -       token = lookup_constant(common_clear_sb_flag, key, 0);
> -       if (token) {
> -               fc->sb_flags &= ~token;
> -               fc->sb_flags_mask |= token;
> -               return 0;
> -       }
> +       if (param->type != fs_value_is_flag)
> +               return invalf(fc, "%s: Unexpected value for '%s'",
> +                             fc->fs_type->name, param->key);
>
> -       return -ENOPARAM;
> +       fc->sb_flags |= set;
> +       fc->sb_flags &= ~clear;
> +       fc->sb_flags_mask |= set | clear;
> +       return 0;
>  }
>
>  /**
> @@ -130,7 +129,7 @@ int vfs_parse_fs_param(struct fs_context *fc, struct fs_parameter *param)
>         if (!param->key)
>                 return invalf(fc, "Unnamed parameter\n");
>
> -       ret = vfs_parse_sb_flag(fc, param->key);
> +       ret = vfs_parse_sb_flag(fc, param);
>         if (ret != -ENOPARAM)
>                 return ret;
>
> --
> 2.21.0
>