Re: [PATCH v2 1/2] pstore: Split pstore fragile flags

[Kees Cook]

On Wed, Jul 20, 2016 at 7:26 AM, Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
> This patch adds new PSTORE_FLAGS for each pstore type so that they can
> be enabled separately.  This is a preparation for ongoing virtio-pstore
> work to support those types flexibly.

Thanks! I was thinking about doing this too, but there wasn't a strong
motivation yet. Now we have one! :)

> The PSTORE_FLAGS_FRAGILE is mapped to PSTORE_FLAGS_DMESG to preserve the
> original behavior.

There are very few users, so I'd rather just remove
PSTORE_FLAGS_FRAGILE entirely and replace it with _DMESG in the
backends that were using _FRAGILE.

-Kees

>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> ---
> v2: check flags on the unregister path also
>
>  fs/pstore/platform.c   | 21 +++++++++++++--------
>  fs/pstore/ram.c        |  2 ++
>  include/linux/pstore.h |  8 +++++++-
>  3 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 588461bb2dd4..6e86b7ea095d 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -467,13 +467,14 @@ int pstore_register(struct pstore_info *psi)
>         if (pstore_is_mounted())
>                 pstore_get_records(0);
>
> -       pstore_register_kmsg();
> -
> -       if ((psi->flags & PSTORE_FLAGS_FRAGILE) == 0) {
> +       if (psi->flags & PSTORE_FLAGS_DMESG)
> +               pstore_register_kmsg();
> +       if (psi->flags & PSTORE_FLAGS_CONSOLE)
>                 pstore_register_console();
> +       if (psi->flags & PSTORE_FLAGS_FTRACE)
>                 pstore_register_ftrace();
> +       if (psi->flags & PSTORE_FLAGS_PMSG)
>                 pstore_register_pmsg();
> -       }
>
>         if (pstore_update_ms >= 0) {
>                 pstore_timer.expires = jiffies +
> @@ -497,10 +498,14 @@ EXPORT_SYMBOL_GPL(pstore_register);
>
>  void pstore_unregister(struct pstore_info *psi)
>  {
> -       pstore_unregister_pmsg();
> -       pstore_unregister_ftrace();
> -       pstore_unregister_console();
> -       pstore_unregister_kmsg();
> +       if (psi->flags & PSTORE_FLAGS_PMSG)
> +               pstore_unregister_pmsg();
> +       if (psi->flags & PSTORE_FLAGS_FTRACE)
> +               pstore_unregister_ftrace();
> +       if (psi->flags & PSTORE_FLAGS_CONSOLE)
> +               pstore_unregister_console();
> +       if (psi->flags & PSTORE_FLAGS_DMESG)
> +               pstore_unregister_kmsg();
>
>         free_buf_for_compression();
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index bd9812e83461..d08bfd611b11 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -539,6 +539,8 @@ static int ramoops_probe(struct platform_device *pdev)
>                 goto fail_clear;
>         }
>
> +       cxt->pstore.flags = PSTORE_FLAGS_ALL;
> +
>         err = pstore_register(&cxt->pstore);
>         if (err) {
>                 pr_err("registering with pstore failed\n");
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index 831479f8df8f..6c291be41e84 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -73,7 +73,13 @@ struct pstore_info {
>         void            *data;
>  };
>
> -#define        PSTORE_FLAGS_FRAGILE    1
> +#define PSTORE_FLAGS_DMESG     (1 << 0)
> +#define PSTORE_FLAGS_CONSOLE   (1 << 1)
> +#define PSTORE_FLAGS_FTRACE    (1 << 2)
> +#define PSTORE_FLAGS_PMSG      (1 << 3)
> +
> +#define        PSTORE_FLAGS_FRAGILE    PSTORE_FLAGS_DMESG
> +#define PSTORE_FLAGS_ALL       ((1 << 4) - 1)
>
>  extern int pstore_register(struct pstore_info *);
>  extern void pstore_unregister(struct pstore_info *);
> --
> 2.8.0
>



-- 
Kees Cook
Chrome OS & Brillo Security