All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] vsprintf: Add command line option debug_boot_weak_hash
@ 2018-05-15  4:38 Tobin C. Harding
  2018-05-15 14:33 ` Steven Rostedt
  2018-05-15 15:29 ` Randy Dunlap
  0 siblings, 2 replies; 4+ messages in thread
From: Tobin C. Harding @ 2018-05-15  4:38 UTC (permalink / raw)
  To: Steven Rostedt, Andrew Morton
  Cc: Tobin C. Harding, Linus Torvalds, Randy Dunlap, Kees Cook,
	Anna-Maria Gleixner, Theodore Ts'o, Greg Kroah-Hartman,
	Arnd Bergmann, linux-kernel

Currently printing [hashed] pointers requires enough entropy to be
available.  Early in the boot sequence this may not be the case
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_boot_weak_hash is enabled use
cryptographically insecure hashing and hash pointer value immediately.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---

This patch was previously submitted as the last in the set

	[PATCH v3 0/4] enable early printing of hashed pointers

Helps debugging using ftrace.  Original problem reported by Anna-Maria,
solution requested by Steve.

Changes since above mentioned patch set
 - change option name from debug_early_boot -> debug_boot_weak_hash
   (suggested by Steve).


I have only tested this by enabling the option and printing some
pointers.  This does not _prove_ that it fixes the ftrace issue.

thanks,
Tobin.


 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 3b8032431585..c95dd6704592 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_boot_weak_hash
+			[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 b82f0c6c2aec..5ff18f8fe3bd 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_boot_weak_hash __ro_after_init;
+EXPORT_SYMBOL(debug_boot_weak_hash);
+
+static int __init debug_boot_weak_hash_enable(char *str)
+{
+	debug_boot_weak_hash = 1;
+	pr_info("debug_boot_weak_hash enabled\n");
+	return 0;
+}
+early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable);
+
 static bool have_filled_random_ptr_key __read_mostly;
 static siphash_key_t ptr_key __read_mostly;
 
@@ -1694,6 +1706,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_boot_weak_hash)) {
+		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 */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] vsprintf: Add command line option debug_boot_weak_hash
  2018-05-15  4:38 [PATCH] vsprintf: Add command line option debug_boot_weak_hash Tobin C. Harding
@ 2018-05-15 14:33 ` Steven Rostedt
  2018-05-15 15:29 ` Randy Dunlap
  1 sibling, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2018-05-15 14:33 UTC (permalink / raw)
  To: Anna-Maria Gleixner
  Cc: Tobin C. Harding, Andrew Morton, Linus Torvalds, Randy Dunlap,
	Kees Cook, Theodore Ts'o, Greg Kroah-Hartman, Arnd Bergmann,
	linux-kernel


Anna-Maria,

Can you test this patch on that kernel you were having issues with (the
one triggering the RCU stalls, and not having entropy to show the
pointers of the timer events).

And report back what you find.

Thanks!

-- Steve


On Tue, 15 May 2018 14:38:35 +1000
"Tobin C. Harding" <me@tobin.cc> wrote:

> Currently printing [hashed] pointers requires enough entropy to be
> available.  Early in the boot sequence this may not be the case
> 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_boot_weak_hash is enabled use
> cryptographically insecure hashing and hash pointer value immediately.
> 
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
> 
> This patch was previously submitted as the last in the set
> 
> 	[PATCH v3 0/4] enable early printing of hashed pointers
> 
> Helps debugging using ftrace.  Original problem reported by Anna-Maria,
> solution requested by Steve.
> 
> Changes since above mentioned patch set
>  - change option name from debug_early_boot -> debug_boot_weak_hash
>    (suggested by Steve).
> 
> 
> I have only tested this by enabling the option and printing some
> pointers.  This does not _prove_ that it fixes the ftrace issue.
> 
> thanks,
> Tobin.
> 
> 
>  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 3b8032431585..c95dd6704592 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_boot_weak_hash
> +			[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 b82f0c6c2aec..5ff18f8fe3bd 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_boot_weak_hash __ro_after_init;
> +EXPORT_SYMBOL(debug_boot_weak_hash);
> +
> +static int __init debug_boot_weak_hash_enable(char *str)
> +{
> +	debug_boot_weak_hash = 1;
> +	pr_info("debug_boot_weak_hash enabled\n");
> +	return 0;
> +}
> +early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable);
> +
>  static bool have_filled_random_ptr_key __read_mostly;
>  static siphash_key_t ptr_key __read_mostly;
>  
> @@ -1694,6 +1706,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_boot_weak_hash)) {
> +		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 */

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] vsprintf: Add command line option debug_boot_weak_hash
  2018-05-15  4:38 [PATCH] vsprintf: Add command line option debug_boot_weak_hash Tobin C. Harding
  2018-05-15 14:33 ` Steven Rostedt
@ 2018-05-15 15:29 ` Randy Dunlap
  2018-05-16 22:26   ` Tobin C. Harding
  1 sibling, 1 reply; 4+ messages in thread
From: Randy Dunlap @ 2018-05-15 15:29 UTC (permalink / raw)
  To: Tobin C. Harding, Steven Rostedt, Andrew Morton
  Cc: Linus Torvalds, Kees Cook, Anna-Maria Gleixner,
	Theodore Ts'o, Greg Kroah-Hartman, Arnd Bergmann,
	linux-kernel

On 05/14/2018 09:38 PM, Tobin C. Harding wrote:

>  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 3b8032431585..c95dd6704592 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_boot_weak_hash
> +			[KNL] Enable debugging early in the boot sequence.  If

That makes it sound like "debug_boot_weak_hash" turns on early debugging.

> +			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

			                          i.e.

> +			of '(___ptrval___)') - cryptographically insecure,
> +			please do not use on production kernels.
> +
>  	debug_locks_verbose=
>  			[KNL] verbose self-tests
>  			Format=<0|1>


-- 
~Randy

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] vsprintf: Add command line option debug_boot_weak_hash
  2018-05-15 15:29 ` Randy Dunlap
@ 2018-05-16 22:26   ` Tobin C. Harding
  0 siblings, 0 replies; 4+ messages in thread
From: Tobin C. Harding @ 2018-05-16 22:26 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Steven Rostedt, Andrew Morton, Linus Torvalds, Kees Cook,
	Anna-Maria Gleixner, Theodore Ts'o, Greg Kroah-Hartman,
	Arnd Bergmann, linux-kernel

On Tue, May 15, 2018 at 08:29:47AM -0700, Randy Dunlap wrote:
> On 05/14/2018 09:38 PM, Tobin C. Harding wrote:
> 
> >  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 3b8032431585..c95dd6704592 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_boot_weak_hash
> > +			[KNL] Enable debugging early in the boot sequence.  If
> 
> That makes it sound like "debug_boot_weak_hash" turns on early debugging.

Thanks Randy, I'll have another go at writing this.


	Tobin

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-05-16 22:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-15  4:38 [PATCH] vsprintf: Add command line option debug_boot_weak_hash Tobin C. Harding
2018-05-15 14:33 ` Steven Rostedt
2018-05-15 15:29 ` Randy Dunlap
2018-05-16 22:26   ` Tobin C. Harding

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.