Re: [PATCH 1/6] pstore: Add console log messages support

From: Kees Cook
Date: Wed May 16 2012 - 12:49:09 EST


On Wed, May 16, 2012 at 5:56 AM, Anton Vorontsov
<anton.vorontsov@xxxxxxxxxx> wrote:
> Pstore doesn't support logging kernel messages in run-time, it only
> dumps dmesg when kernel oopses/panics. This makes pstore useless for
> debugging hangs caused by HW issues or improper use of HW (e.g.
> weird device inserted -> driver tried to write a reserved bits ->
> SoC hanged. In that case we don't get any messages in the pstore.
>
> Therefore, let's add a runtime logging support: PSTORE_TYPE_CONSOLE.
>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@xxxxxxxxxx>
> ---
>  fs/pstore/Kconfig      |    7 +++++++
>  fs/pstore/inode.c      |    3 +++
>  fs/pstore/platform.c   |   25 +++++++++++++++++++++++++
>  include/linux/pstore.h |    1 +
>  4 files changed, 36 insertions(+)
>
> diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
> index 23ade26..d044de6 100644
> --- a/fs/pstore/Kconfig
> +++ b/fs/pstore/Kconfig
> @@ -12,6 +12,13 @@ config PSTORE
>           If you don't have a platform persistent store driver,
>           say N.
>
> +config PSTORE_CONSOLE
> +       bool "Log kernel console messages"
> +       depends on PSTORE
> +       help
> +         When the option is enabled, pstore will log all kernel
> +         messages, even if no oops or panic happened.
> +
>  config PSTORE_RAM
>        tristate "Log panic/oops to a RAM buffer"
>        depends on PSTORE
> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index 1950788..97d35ee 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -212,6 +212,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id,
>        case PSTORE_TYPE_DMESG:
>                sprintf(name, "dmesg-%s-%lld", psname, id);
>                break;
> +       case PSTORE_TYPE_CONSOLE:
> +               sprintf(name, "console-%s", psname);
> +               break;
>        case PSTORE_TYPE_MCE:
>                sprintf(name, "mce-%s-%lld", psname, id);
>                break;
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 82c585f..3a384c8 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -1,6 +1,7 @@
>  /*
>  * Persistent Storage - platform driver interface parts.
>  *
> + * Copyright (C) 2007-2008 Google, Inc.
>  * Copyright (C) 2010 Intel Corporation <tony.luck@xxxxxxxxx>
>  *
>  *  This program is free software; you can redistribute it and/or modify
> @@ -22,6 +23,7 @@
>  #include <linux/errno.h>
>  #include <linux/init.h>
>  #include <linux/kmsg_dump.h>
> +#include <linux/console.h>
>  #include <linux/module.h>
>  #include <linux/pstore.h>
>  #include <linux/string.h>
> @@ -156,6 +158,28 @@ static struct kmsg_dumper pstore_dumper = {
>        .dump = pstore_dump,
>  };
>
> +#ifdef CONFIG_PSTORE_CONSOLE
> +static void pstore_console_write(struct console *con, const char *s, unsigned c)
> +{
> +       strncpy(psinfo->buf, s, c);

The size of psinfo->buf needs to be the length argument to strncpy,
not the size of "s". If "s" isn't NULL terminated, then the min of c
and buf's size should be used. And if this should be NULL terminated
in buf, that needs to be added manually since strncpy won't do it if
it hits the max length argument.

-Kees

--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/