Re: [PATCH v2 2/5] pstore/platform: pass max_reason to kmesg dump

[Kees Cook <keescook@chromium.org>]

On Tue, May 05, 2020 at 11:45:07AM -0400, Pavel Tatashin wrote:
> Add a new field to pstore_info that passes information about kmesg dump
> maximum reason.
> 
> This allows a finer control of what kmesg dumps are stored on pstore
> device.
> 
> Those clients that do not explicitly set this field (keep it equal to 0),
> get the default behavior: dump only Oops and Panics, and dump everything
> if printk.always_kmsg_dump is provided.
> 
> Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> ---
>  fs/pstore/platform.c   | 4 +++-
>  include/linux/pstore.h | 3 +++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 408277ee3cdb..75bf8a43f92a 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -602,8 +602,10 @@ int pstore_register(struct pstore_info *psi)
>  	if (pstore_is_mounted())
>  		pstore_get_records(0);
>  
> -	if (psi->flags & PSTORE_FLAGS_DMESG)
> +	if (psi->flags & PSTORE_FLAGS_DMESG) {
> +		pstore_dumper.max_reason = psinfo->max_reason;
>  		pstore_register_kmsg();
> +	}

I haven't finished reading the whole series carefully, but I think
something we can do here to make things a bit more user-friendly is to
do the KMSG_DUMP_UNDEF value here to get us the "all" instead of INT_MAX:

	if (psi->flags & PSTORE_FLAGS_DMESG) {
		pstore_dumper.max_reason = psinfo->max_reason;
		if (pstore_dumper.max_reason == KMSG_DUMP_UNDEF)
			pstore_dumper.max_reason = KMSG_DUMP_MAX;
		pstore_register_kmsg();
	}

That way setting max_reason to 0 without setting dump_oops at all will
get "all". I think it'll need some tweaks to the next patch.

>  	if (psi->flags & PSTORE_FLAGS_CONSOLE)
>  		pstore_register_console();
>  	if (psi->flags & PSTORE_FLAGS_FTRACE)
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index e779441e6d26..45ae424bfeb5 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -97,6 +97,8 @@ struct pstore_record {
>   * @read_mutex:	serializes @open, @read, @close, and @erase callbacks
>   * @flags:	bitfield of frontends the backend can accept writes for
>   * @data:	backend-private pointer passed back during callbacks
> + * @max_reason: Used when PSTORE_FLAGS_DMESG is set. Contains the
> + *              kmsg_dump_reason enum value.

Nit: please move this above @data since it has a @flags dependency.

>   *
>   * Callbacks:
>   *
> @@ -180,6 +182,7 @@ struct pstore_info {
>  
>  	int		flags;
>  	void		*data;
> +	int		max_reason;
>  
>  	int		(*open)(struct pstore_info *psi);
>  	int		(*close)(struct pstore_info *psi);
> -- 
> 2.25.1
> 

Looking good!

-- 
Kees Cook