linux-kernel - Re: [RFC PATCH 4/8] vfs: Convert pstore to fs

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]

Date:   Thu, 21 Mar 2019 09:31:46 -0700
From:   Kees Cook <keescook@...omium.org>
To:     David Howells <dhowells@...hat.com>
Cc:     Al Viro <viro@...iv.linux.org.uk>,
        Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 4/8] vfs: Convert pstore to fs_context

On Thu, Mar 21, 2019 at 4:48 AM David Howells <dhowells@...hat.com> wrote:
>
> Signed-off-by: David Howells <dhowells@...hat.com>
> cc: Kees Cook <keescook@...omium.org>
> cc: Anton Vorontsov <anton@...msg.org>
> cc: Colin Cross <ccross@...roid.com>
> cc: Tony Luck <tony.luck@...el.com>

Thanks for doing this conversion!

Acked-by: Kees Cook <keescook@...omium.org>

Some questions/nits below:

> +static int pstore_parse_param(struct fs_context *fc, struct fs_parameter *param)
> +{
> +       struct pstore_fs_context *ctx = fc->fs_private;
> +       struct fs_parse_result result;
> +       int opt;
> +
> +       opt = fs_parse(fc, &pstore_fs_parameters, param, &result);
> +       if (opt < 0)
> +               return opt;
> +
> +       switch (opt) {
> +       case Opt_kmsg_bytes:
> +               ctx->kmsg_bytes = result.uint_32;
> +               break;
> +       }
>
> -       while ((p = strsep(&options, ",")) != NULL) {
> -               int token;
> +       return 0;
> +}
>
> -               if (!*p)
> -                       continue;
> +static void pstore_apply_param(struct fs_context *fc)
> +{
> +       struct pstore_fs_context *ctx = fc->fs_private;
>
> -               token = match_token(p, tokens, args);
> -               switch (token) {
> -               case Opt_kmsg_bytes:
> -                       if (!match_int(&args[0], &option))
> -                               pstore_set_kmsg_bytes(option);
> -                       break;
> -               }
> -       }
> +       pstore_set_kmsg_bytes(ctx->kmsg_bytes);
>  }

Why the separation between parse and apply now? Is this due to the
reconfigure calls? (i.e. why not call pstore_set_kmsg_bytes() in
pstore_parse_param()?

> @@ -416,14 +423,38 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent)
>                 return -ENOMEM;
>
>         pstore_get_records(0);
> -
>         return 0;
>  }

I prefer to keep a blank before "return", but no need to drop this
unless it gets a respin.

Thanks again!

-- 
Kees Cook