| 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
| ||
|
Date: Fri, 4 Aug 2017 08:36:18 -0700
From: Mark Salyzyn <salyzyn@...roid.com>
To: Prarit Bhargava <prarit@...hat.com>, linux-kernel@...r.kernel.org
Cc: Jonathan Corbet <corbet@....net>, Petr Mladek <pmladek@...e.com>,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
Steven Rostedt <rostedt@...dmis.org>,
John Stultz <john.stultz@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Stephen Boyd <sboyd@...eaurora.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Christoffer Dall <cdall@...aro.org>,
Deepa Dinamani <deepa.kernel@...il.com>,
Ingo Molnar <mingo@...nel.org>,
Joel Fernandes <joelaf@...gle.com>,
Kees Cook <keescook@...omium.org>,
Peter Zijlstra <peterz@...radead.org>,
Geert Uytterhoeven <geert+renesas@...der.be>,
"Luis R. Rodriguez" <mcgrof@...nel.org>,
Nicholas Piggin <npiggin@...il.com>,
"Jason A. Donenfeld" <Jason@...c4.com>,
Olof Johansson <olof@...om.net>,
Josh Poimboeuf <jpoimboe@...hat.com>, linux-doc@...r.kernel.org
Subject: Re: [PATCH v3] printk: Add boottime and real timestamps
On 08/03/2017 06:18 PM, Prarit Bhargava wrote:
> diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
> index cfc2465e8b77..6c73c305ad17 100644
> --- a/arch/arm/configs/aspeed_g4_defconfig
> +++ b/arch/arm/configs/aspeed_g4_defconfig
> @@ -162,7 +162,9 @@ CONFIG_JFFS2_FS_XATTR=y
> CONFIG_UBIFS_FS=y
> CONFIG_SQUASHFS=y
> CONFIG_SQUASHFS_XZ=y
> -CONFIG_PRINTK_TIME=y
> +# CONFIG_PRINTK_TIME_DISABLE is not set
> +CONFIG_PRINTK_TIME_LOCAL=Y
> +CONFIG_PRINTK_TIME=1
> CONFIG_DYNAMIC_DEBUG=y
> CONFIG_STRIP_ASM_SYMS=y
> CONFIG_DEBUG_FS=y
savedefconfig tells me otherwise:
-CONFIG_PRINTK_TIME=y
+CONFIG_PRINTK_TIME_LOCAL=y
is all that is needed, the rest is fluff and will cause a savedefconfig
mismatch error.
> +static int printk_time_set(const char *val, const struct kernel_param *kp)
> +{
> + char *param = strstrip((char *)val);
> + int _printk_time;
> +
> + if (strlen(param) != 1)
> + return -EINVAL;
(see below) strlen is so restrictive, wouldn't it be nice(tm) to allow
"boot", "monotonic" or "realtime" strings? Certainly KISS can handle the
first letters for next to zero cost. (switch statement optimization)
> +
> + switch (param[0]) {
> + case '0':
> + case 'n':
> + case 'N':
I would wish for a few more 'convenience' cases:
case 'd': case 'D': /* Disable */
> + _printk_time = PRINTK_TIME_DISABLE;
> + break;
> + case '1':
> + case 'y':
> + case 'Y':
> + _printk_time = PRINTK_TIME_LOCAL;
> + break;
> + case '2':
case 'b': case 'B': /* Boottime */
> + _printk_time = PRINTK_TIME_BOOT;
> + break;
> + case '3':
case 'm': case 'M': /* Monotonic */
> + _printk_time = PRINTK_TIME_MONO;
> + break;
> + case '4':
case 'r': case 'R': /* Realtime */
> + _printk_time = PRINTK_TIME_REAL;
> + break;
> + default:
> + pr_warn("printk: invalid timestamp value\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Only allow enabling and disabling of the current printk_time
> + * setting. Changing it from one setting to another confuses
> + * userspace.
> + */
> + if (printk_time_setting == PRINTK_TIME_DISABLE) {
> + printk_time_setting = _printk_time;
> + } else if ((printk_time_setting != _printk_time) &&
> + (_printk_time != 0)) {
> + pr_warn("printk: timestamp can only be set to 0(disabled) or %d(%s) ",
> + printk_time_setting,
> + printk_time_str[printk_time_setting]);
> + return -EINVAL;
> + }
> +
> + printk_time = _printk_time;
> + pr_info("printk: timestamp set to %d(%s).",
> + printk_time, printk_time_str[printk_time]);
> + return 0;
> +}
> . . .
> /* time in seconds when suspend began for persistent clock */
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 98fe715522e8..a71ba5689814 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1,8 +1,46 @@
> menu "printk and dmesg options"
>
> +choice
> + prompt "printk default clock"
> + config PRINTK_TIME_DISABLE
> + bool "Disabled"
> + help
> + Selecting this option disables the time stamps of printk().
> +
> + config PRINTK_TIME_LOCAL
> + bool "Local Clock"
> + help
> + Selecting this option causes the time stamps of printk() to be
> + stamped with the unadjusted hardware clock.
> +
> + config PRINTK_TIME_BOOT
> + bool "CLOCK_BOOTTIME"
> + help
> + Selecting this option causes the time stamps of printk() to be
> + stamped with the adjusted boottime clock.
> +
> + config PRINTK_TIME_MONO
> + bool "CLOCK_MONOTONIC"
> + help
> + Selecting this option causes the time stamps of printk() to be
> + stamped with the adjusted monotonic clock.
> +
> + config PRINTK_TIME_REAL
> + bool "CLOCK_REALTIME"
> + help
> + Selecting this option causes the time stamps of printk() to be
> + stamped with the adjusted realtime clock.
> +
> +endchoice
This will ensure mutual exclusivity, no need to add #
CONFIG_PRINTK_TIME_DISABLE is not set to configs.
> +
> config PRINTK_TIME
> - bool "Show timing information on printks"
> + int "Show time stamp information on printks"
> depends on PRINTK
> + default 0 if PRINTK_TIME_DISABLE
> + default 1 if PRINTK_TIME_LOCAL
> + default 2 if PRINTK_TIME_BOOT
> + default 3 if PRINTK_TIME_MONO
> + default 4 if PRINTK_TIME_REAL
> help
> Selecting this option causes time stamps of the printk()
> messages to be added to the output of the syslog() system
This "dummy" config will be automated by the select, so no need to add
CONFIG_PRINTK_TIME=<x> to the defconfigs.
-- Mark
Powered by blists - more mailing lists