linux-kernel - Re: [PATCH v4] printk: Add monotonic, boottime, and realtime timestamps

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]

Message-ID: <ff8a31d8-cebe-88bf-fe05-82b0b8d4e3da@android.com>
Date:   Mon, 7 Aug 2017 09:58:31 -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 v4] printk: Add monotonic, boottime, and realtime
 timestamps

On 08/07/2017 08:52 AM, Prarit Bhargava wrote:
> diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
> index cfc2465e8b77..5f3c50914e92 100644
> --- a/arch/arm/configs/aspeed_g4_defconfig
> +++ b/arch/arm/configs/aspeed_g4_defconfig
> @@ -162,7 +162,7 @@ CONFIG_JFFS2_FS_XATTR=y
>   CONFIG_UBIFS_FS=y
>   CONFIG_SQUASHFS=y
>   CONFIG_SQUASHFS_XZ=y
> -CONFIG_PRINTK_TIME=y
> +CONFIG_PRINTK_TIME_LOCAL=y
>   CONFIG_DYNAMIC_DEBUG=y
>   CONFIG_STRIP_ASM_SYMS=y
>   CONFIG_DEBUG_FS=y
Many have had misgivings, let me try another pass at this.

We (royal we) should really look into adjusting configuration parsing to 
allow an easy transition from boolean to selection ... I am sure this is 
not the first time bistate/tristate was moved to a number.

An idea? Maybe look into a way to deal with this to use something 
_other_ than CONFIG_PRINTK_TIME to hold the selection, and keep a 
(hidden/legacy?) CONFIG_PRINTK_TIME that when selected sets 
CONFIG_PRINTK_TIME_LOCAL, and switch to _not_ CONFIG_PRINTK_TIME_DISABLE 
as the internal mechanical replacement for it. I do not know how 
disruptive this will be, but is worth it if the codebase supports it, 
and legacy config retained?
> +
> +static int printk_time_set(const char *val, const struct kernel_param *kp)
> +{
> +	char *param = strstrip((char *)val);
> +	int _printk_time = -1;
> +	int stamp;
> +
> +	if (strlen(param) == 1) {
> +		/* Preserve legacy boolean settings */
> +		if (!strcmp("0", param) || !strcmp("n", param) ||
if strlen(param) == 1, then param[0] == '0' etc works fine and is KISS.
> +	/*
> +	 * 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 %s\n",
> +			printk_time_str[printk_time_setting]);
> +		return -EINVAL;
> +	}
I agree with the restriction in the general case.  However (as hinted at 
before() #ifdef CONFIG_PRINTK_TIME_RESTRICT (default y, or #ifndef 
CONFIG_PRINTK_TIME_DEBUG default n) around this will allow us users to 
choose if we are confused or not. I can see being able to change it on 
the fly as an option. Especially since we have 
/sys/module/printk/parameters/time.

-- Mark