Re: [PATCH v3 4/4] vsprintf: Add command line option debug_early_boot

[Steven Rostedt]

On Fri,  4 May 2018 09:07:41 +1000
"Tobin C. Harding" <me@xxxxxxxx> wrote:

> Currently printing [hashed] pointers requires either a hw RNG or enough
> entropy to be available.  Early in the boot sequence these conditions
> may not be met resulting in a dummy string '(____ptrval____)' being
> printed.  This makes debugging the early boot sequence difficult.  We
> can relax the requirement to use cryptographically secure hashing during
> debugging.  This enables debugging while keeping development/production
> kernel behaviour the same.
> 
> If new command line option debug_early_boot is enabled use
> cryptographically insecure hashing and hash pointer value immediately.
> 
> Signed-off-by: Tobin C. Harding <me@xxxxxxxx>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  8 ++++++++
>  lib/vsprintf.c                                  | 18 ++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index b8d1379aa039..ab619c4ccbf2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -748,6 +748,14 @@
>  
>  	debug		[KNL] Enable kernel debugging (events log level).
>  
> +	debug_early_boot

Honestly, I think this is a bad name for what it does. It doesn't
suggest anything about hashing algorithms. And looks like something to
set if you want to debug early in the boot process but you are not
printing pointers.

Maybe something like "debug_boot_weak_hash"? 

-- Steve


> +			[KNL] Enable debugging early in the boot sequence.  If
> +			enabled, we use a weak hash instead of siphash to hash
> +			pointers.  Use this option if you need to see pointer
> +			values during early boot (i.e you are seeing instances
> +			of '(___ptrval___)') - cryptographically insecure,
> +			please do not use on production kernels.
> +
>  	debug_locks_verbose=
>  			[KNL] verbose self-tests
>  			Format=<0|1>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3697a19c2b25..2c48884f00eb 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1654,6 +1654,18 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
>  	return widen_string(buf, buf - buf_start, end, spec);
>  }
>  
> +/* Make pointers available for printing early in the boot sequence. */
> +static int debug_early_boot __ro_after_init;
> +EXPORT_SYMBOL(debug_early_boot);
> +
> +static int __init debug_early_boot_enable(char *str)
> +{
> +	debug_early_boot = 1;
> +	pr_info("debug_early_boot enabled\n");
> +	return 0;
> +}
> +early_param("debug_early_boot", debug_early_boot_enable);
> +
>  static bool have_filled_random_ptr_key __read_mostly;
>  static siphash_key_t ptr_key __read_mostly;
>  
> @@ -1707,6 +1719,12 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
>  	const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)";
>  	unsigned long hashval;
>  
> +	/* When debugging early boot use non-cryptographically secure hash */
> +	if (unlikely(debug_early_boot)) {
> +		hashval = hash_long((unsigned long)ptr, 32);
> +		return pointer_string(buf, end, (const void *)hashval, spec);
> +	}
> +
>  	if (unlikely(!have_filled_random_ptr_key)) {
>  		spec.field_width = 2 * sizeof(ptr);
>  		/* string length must be less than default_width */