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

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

[Tobin C. Harding]

Currently printing pointers early in the boot sequence can result in a
dummy string '(____ptrval____)' being printed.  While resolving this
issue it was noticed that we can use the hw RNG if available for hashing
pointers.

Patch one and two do the ground work to be able to use hw RNG removing
from get_random_bytes_arch() the call to get_random_bytes() and
returning the number of bytes of random material successfully returned. 

Patch three uses the hw RNG to get keying material if it is available.

Patch four further assists debugging early in the boot sequence for
machines that do not have a hw RNG by adding a command line option
'debug_boot_weak_hash'.  If enabled, non-cryptographically secure hashing
is used instead of siphash so we can hash at any time. 

During the versions of this set I have been totally confused about which
patches go through which tree.  This version again puts all 4 patches
together in the hope they will go through Andrew's tree.


Steve,

Could you please take a quick squiz at the final 2 patches if you get a
chance.  I assumed we are in preemptible context during early_init based
on your code (and code comment) and called static_branch_disable()
directly if hw RNG returned keying material.  It's a pretty simple
change but I'd love to get someone else to check I've not noob'ed it.


thanks,
Tobin.

v6
 - Rebase on top of Steve's patch (fixing race condition).  Uses static
   branch instead of memory barrier.

v5
 - Use 'upside-down-xmas-tree' style to declare local variables (Steve)
 - Added Reviewed-by tag from Steve (patch 2 and 3).

v4
 - remove last patch of series (command line option patch)

v3
 - Add __ro_after_init (suggested by Kees).

v2
 - Use min_t() instead of min() (thanks checkpatch).
 - Add __must_check to function declaration (thanks Steve).
 - Use hw RNG by default if available (as originally suggested by Kees).
 - Add command line option to use cryptographically insecure hashing.
   If debug_early_boot is enabled use hash_long() instead of siphash
   (as requested by Steve, and solves original problem for Anna-Maria).
 - Added Acked-by tag from Ted (patch 1 and 2)



Tobin C. Harding (4):
  random: Fix whitespace pre random-bytes work
  random: Return nbytes filled from hw RNG
  vsprintf: Use hw RNG for ptr_key
  vsprintf: Add command line option debug_boot_weak_hash

 Documentation/admin-guide/kernel-parameters.txt |  9 ++++++++
 drivers/char/random.c                           | 19 ++++++++--------
 include/linux/random.h                          |  2 +-
 lib/vsprintf.c                                  | 30 ++++++++++++++++++++++++-
 4 files changed, 49 insertions(+), 11 deletions(-)

-- 
2.7.4

[PATCH v6 1/4] random: Fix whitespace pre random-bytes work

[Tobin C. Harding]

There are a couple of whitespace issues around the function
get_random_bytes_arch().  In preparation for patching this function
let's clean them up.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
Acked-by: Theodore Ts'o <tytso@mit.edu>
---
 drivers/char/random.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index cd888d4ee605..031d18b31e0f 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1737,7 +1737,7 @@ void get_random_bytes_arch(void *buf, int nbytes)
 
 		if (!arch_get_random_long(&v))
 			break;
-		
+
 		memcpy(p, &v, chunk);
 		p += chunk;
 		nbytes -= chunk;
@@ -1748,7 +1748,6 @@ void get_random_bytes_arch(void *buf, int nbytes)
 }
 EXPORT_SYMBOL(get_random_bytes_arch);
 
-
 /*
  * init_std_data - initialize pool with system data
  *
-- 
2.7.4

[PATCH v6 2/4] random: Return nbytes filled from hw RNG

[Tobin C. Harding]

Currently the function get_random_bytes_arch() has return value 'void'.
If the hw RNG fails we currently fall back to using get_random_bytes().
This defeats the purpose of requesting random material from the hw RNG
in the first place.

There are currently no intree users of get_random_bytes_arch().

Only get random bytes from the hw RNG, make function return the number
of bytes retrieved from the hw RNG.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
Acked-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 drivers/char/random.c  | 16 +++++++++-------
 include/linux/random.h |  2 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 031d18b31e0f..8b9b537ae634 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1725,26 +1725,28 @@ EXPORT_SYMBOL(del_random_ready_callback);
  * key known by the NSA).  So it's useful if we need the speed, but
  * only if we're willing to trust the hardware manufacturer not to
  * have put in a back door.
+ *
+ * Return number of bytes filled in.
  */
-void get_random_bytes_arch(void *buf, int nbytes)
+int __must_check get_random_bytes_arch(void *buf, int nbytes)
 {
+	int left = nbytes;
 	char *p = buf;
 
-	trace_get_random_bytes_arch(nbytes, _RET_IP_);
-	while (nbytes) {
+	trace_get_random_bytes_arch(left, _RET_IP_);
+	while (left) {
 		unsigned long v;
-		int chunk = min(nbytes, (int)sizeof(unsigned long));
+		int chunk = min_t(int, left, (int)sizeof(unsigned long));
 
 		if (!arch_get_random_long(&v))
 			break;
 
 		memcpy(p, &v, chunk);
 		p += chunk;
-		nbytes -= chunk;
+		left -= chunk;
 	}
 
-	if (nbytes)
-		get_random_bytes(p, nbytes);
+	return nbytes - left;
 }
 EXPORT_SYMBOL(get_random_bytes_arch);
 
diff --git a/include/linux/random.h b/include/linux/random.h
index 2ddf13b4281e..f1c9bc5cd231 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -38,7 +38,7 @@ extern void get_random_bytes(void *buf, int nbytes);
 extern int wait_for_random_bytes(void);
 extern int add_random_ready_callback(struct random_ready_callback *rdy);
 extern void del_random_ready_callback(struct random_ready_callback *rdy);
-extern void get_random_bytes_arch(void *buf, int nbytes);
+extern int __must_check get_random_bytes_arch(void *buf, int nbytes);
 
 #ifndef MODULE
 extern const struct file_operations random_fops, urandom_fops;
-- 
2.7.4

[PATCH v6 3/4] vsprintf: Use hw RNG for ptr_key

[Tobin C. Harding]

Currently we must wait for enough entropy to become available before
hashed pointers can be printed.  We can remove this wait by using the
hw RNG if available.

Use hw RNG to get keying material.

Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Suggested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 lib/vsprintf.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 23920c5ff728..1545a8aa26a9 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1693,8 +1693,16 @@ static struct random_ready_callback random_ready = {
 
 static int __init initialize_ptr_random(void)
 {
-	int ret = add_random_ready_callback(&random_ready);
+	int key_size = sizeof(ptr_key);
+	int ret;
+
+	/* Use hw RNG if available */
+	if (get_random_bytes_arch(&ptr_key, key_size) == key_size) {
+		static_branch_disable(&not_filled_random_ptr_key);
+		return 0;
+	}
 
+	ret = add_random_ready_callback(&random_ready);
 	if (!ret) {
 		return 0;
 	} else if (ret == -EALREADY) {
-- 
2.7.4

Re: [PATCH v6 3/4] vsprintf: Use hw RNG for ptr_key

[Steven Rostedt]

On Mon, 28 May 2018 11:46:41 +1000
"Tobin C. Harding" <me@tobin.cc> wrote:

> Currently we must wait for enough entropy to become available before
> hashed pointers can be printed.  We can remove this wait by using the
> hw RNG if available.
> 
> Use hw RNG to get keying material.
> 
> Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> Suggested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
>  lib/vsprintf.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 23920c5ff728..1545a8aa26a9 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1693,8 +1693,16 @@ static struct random_ready_callback random_ready = {
>  
>  static int __init initialize_ptr_random(void)
>  {
> -	int ret = add_random_ready_callback(&random_ready);
> +	int key_size = sizeof(ptr_key);
> +	int ret;
> +
> +	/* Use hw RNG if available */
> +	if (get_random_bytes_arch(&ptr_key, key_size) == key_size) {
> +		static_branch_disable(&not_filled_random_ptr_key);
> +		return 0;
> +	}
>  
> +	ret = add_random_ready_callback(&random_ready);
>  	if (!ret) {
>  		return 0;
>  	} else if (ret == -EALREADY) {

[PATCH v6 4/4] vsprintf: Add command line option debug_boot_weak_hash

[Tobin C. Harding]

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.

Cc: Anna-Maria Gleixner <anna-maria@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 Documentation/admin-guide/kernel-parameters.txt |  9 +++++++++
 lib/vsprintf.c                                  | 20 ++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f2040d46f095..8a86d895343e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -753,6 +753,15 @@
 
 	debug		[KNL] Enable kernel debugging (events log level).
 
+	debug_boot_weak_hash
+			[KNL] Enable printing pointers 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 1545a8aa26a9..369623205e2c 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1670,6 +1670,20 @@ char *pointer_string(char *buf, char *end, const void *ptr,
 }
 
 static DEFINE_STATIC_KEY_TRUE(not_filled_random_ptr_key);
+
+/* 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;
 
 static void enable_ptr_key_workfn(struct work_struct *work)
@@ -1721,6 +1735,12 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
 	unsigned long hashval;
 	const int default_width = 2 * sizeof(ptr);
 
+	/* 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 (static_branch_unlikely(&not_filled_random_ptr_key)) {
 		spec.field_width = default_width;
 		/* string length must be less than default_width */
-- 
2.7.4

Re: [PATCH v6 4/4] vsprintf: Add command line option debug_boot_weak_hash

[Randy Dunlap]

On 05/27/2018 06:46 PM, Tobin C. Harding 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.
> 
> Cc: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  9 +++++++++
>  lib/vsprintf.c                                  | 20 ++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f2040d46f095..8a86d895343e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -753,6 +753,15 @@
>  
>  	debug		[KNL] Enable kernel debugging (events log level).
>  

Hi,
This is much more readable than the previous version.  Thanks.

> +	debug_boot_weak_hash
> +			[KNL] Enable printing pointers 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

			                                         i.e.

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


-- 
~Randy

Re: [PATCH v6 4/4] vsprintf: Add command line option debug_boot_weak_hash

[Tobin C. Harding]

On Mon, May 28, 2018 at 10:40:43AM -0700, Randy Dunlap wrote:
> On 05/27/2018 06:46 PM, Tobin C. Harding 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.
> > 
> > Cc: Anna-Maria Gleixner <anna-maria@linutronix.de>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Randy Dunlap <rdunlap@infradead.org>
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> >  Documentation/admin-guide/kernel-parameters.txt |  9 +++++++++
> >  lib/vsprintf.c                                  | 20 ++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index f2040d46f095..8a86d895343e 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -753,6 +753,15 @@
> >  
> >  	debug		[KNL] Enable kernel debugging (events log level).
> >  
> 
> Hi,
> This is much more readable than the previous version.  Thanks.

Thanks for following up with this one Randy.

	Tobin

Re: [PATCH v6 4/4] vsprintf: Add command line option debug_boot_weak_hash

[Steven Rostedt]

On Mon, 28 May 2018 11:46:42 +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.
> 

I was able to play with this. It did start showing real pointers after
the early parameters are parsed. Hopefully we don't need anything
before that. But that is still much earlier than what we had before.


> Cc: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  9 +++++++++
>  lib/vsprintf.c                                  | 20 ++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f2040d46f095..8a86d895343e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -753,6 +753,15 @@
>  
>  	debug		[KNL] Enable kernel debugging (events log level).
>  
> +	debug_boot_weak_hash
> +			[KNL] Enable printing pointers 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 1545a8aa26a9..369623205e2c 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1670,6 +1670,20 @@ char *pointer_string(char *buf, char *end, const void *ptr,
>  }
>  
>  static DEFINE_STATIC_KEY_TRUE(not_filled_random_ptr_key);
> +
> +/* 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;
>  
>  static void enable_ptr_key_workfn(struct work_struct *work)
> @@ -1721,6 +1735,12 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
>  	unsigned long hashval;
>  	const int default_width = 2 * sizeof(ptr);
>  
> +	/* When debugging early boot use non-cryptographically secure hash */
> +	if (unlikely(debug_boot_weak_hash)) {

Perhaps we should make the debug_boot_weak_hash into a static key too.
That way, it's constant jump instead of a compare.

-- Steve


> +		hashval = hash_long((unsigned long)ptr, 32);
> +		return pointer_string(buf, end, (const void *)hashval, spec);
> +	}
> +
>  	if (static_branch_unlikely(&not_filled_random_ptr_key)) {
>  		spec.field_width = default_width;
>  		/* string length must be less than default_width */

Re: [PATCH v6 4/4] vsprintf: Add command line option debug_boot_weak_hash

[Tobin C. Harding]

On Thu, May 31, 2018 at 05:35:15PM -0400, Steven Rostedt wrote:
> On Mon, 28 May 2018 11:46:42 +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.
> > 
> 
> I was able to play with this. It did start showing real pointers after
> the early parameters are parsed. Hopefully we don't need anything
> before that. But that is still much earlier than what we had before.

Cool, thanks for testing.

> > Cc: Anna-Maria Gleixner <anna-maria@linutronix.de>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Randy Dunlap <rdunlap@infradead.org>
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> >  Documentation/admin-guide/kernel-parameters.txt |  9 +++++++++
> >  lib/vsprintf.c                                  | 20 ++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index f2040d46f095..8a86d895343e 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -753,6 +753,15 @@
> >  
> >  	debug		[KNL] Enable kernel debugging (events log level).
> >  
> > +	debug_boot_weak_hash
> > +			[KNL] Enable printing pointers 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 1545a8aa26a9..369623205e2c 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1670,6 +1670,20 @@ char *pointer_string(char *buf, char *end, const void *ptr,
> >  }
> >  
> >  static DEFINE_STATIC_KEY_TRUE(not_filled_random_ptr_key);
> > +
> > +/* 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;
> >  
> >  static void enable_ptr_key_workfn(struct work_struct *work)
> > @@ -1721,6 +1735,12 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
> >  	unsigned long hashval;
> >  	const int default_width = 2 * sizeof(ptr);
> >  
> > +	/* When debugging early boot use non-cryptographically secure hash */
> > +	if (unlikely(debug_boot_weak_hash)) {
> 
> Perhaps we should make the debug_boot_weak_hash into a static key too.
> That way, it's constant jump instead of a compare.

Happy to do it but isn't that a bit of an over optimization, is printing
ever that performance critical?

Of course, if on second thoughts you still think it's worth doing I'll
code it up.

thanks,
Tobin.

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Theodore Y. Ts'o]

On Mon, May 28, 2018 at 11:46:38AM +1000, Tobin C. Harding wrote:
> 
> During the versions of this set I have been totally confused about which
> patches go through which tree.  This version again puts all 4 patches
> together in the hope they will go through Andrew's tree.

I'm also happy to take the whole patch series through the random tree
if everyone else is OK with it.

Thoughts?

					- Ted

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Tobin C. Harding]

On Mon, May 28, 2018 at 09:59:15AM -0400, Theodore Y. Ts'o wrote:
> On Mon, May 28, 2018 at 11:46:38AM +1000, Tobin C. Harding wrote:
> > 
> > During the versions of this set I have been totally confused about which
> > patches go through which tree.  This version again puts all 4 patches
> > together in the hope they will go through Andrew's tree.
> 
> I'm also happy to take the whole patch series through the random tree
> if everyone else is OK with it.
> 
> Thoughts?

Looks like every one is happy (or silent) now Ted, can you please take
this version through your tree.

thanks,
Tobin.

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Theodore Y. Ts'o]

On Thu, Jun 07, 2018 at 09:31:04AM +1000, Tobin C. Harding wrote:
> > I'm also happy to take the whole patch series through the random tree
> > if everyone else is OK with it.
> 
> Looks like every one is happy (or silent) now Ted, can you please take
> this version through your tree.

Ok, it's now on the random.git tree.  I'm going to give it a few days
of soak time in linux-next, and then I'll push it to Linus before the
end of the merge window.

							- Ted

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Tobin C. Harding]

On Thu, Jun 07, 2018 at 10:00:02AM -0400, Theodore Y. Ts'o wrote:
> On Thu, Jun 07, 2018 at 09:31:04AM +1000, Tobin C. Harding wrote:
> > > I'm also happy to take the whole patch series through the random tree
> > > if everyone else is OK with it.
> > 
> > Looks like every one is happy (or silent) now Ted, can you please take
> > this version through your tree.
> 
> Ok, it's now on the random.git tree.  I'm going to give it a few days
> of soak time in linux-next, and then I'll push it to Linus before the
> end of the merge window.

Great, thanks.

	Tobin.

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Steven Rostedt]

On Mon, 28 May 2018 11:46:38 +1000
"Tobin C. Harding" <me@tobin.cc> wrote:

> Steve,

Hi Tobin,

Sorry for the late reply, I'm currently at a conference and have had
little time to read email.

> 
> Could you please take a quick squiz at the final 2 patches if you get a
> chance.  I assumed we are in preemptible context during early_init based
> on your code (and code comment) and called static_branch_disable()
> directly if hw RNG returned keying material.  It's a pretty simple
> change but I'd love to get someone else to check I've not noob'ed it.

I can take a look, and perhaps do some tests. But it was Anna-Maria
that originally triggered the issue. She's on Cc, perhaps she can try
this and see if it works.

I'll still pull the patches and add some injections to see how it works.

Thanks!

-- Steve

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Anna-Maria Gleixner]

On Thu, 31 May 2018, Steven Rostedt wrote:

> On Mon, 28 May 2018 11:46:38 +1000
> "Tobin C. Harding" <me@tobin.cc> wrote:
> 
> > Steve,
> 
> Hi Tobin,
> 
> Sorry for the late reply, I'm currently at a conference and have had
> little time to read email.
> 
> > 
> > Could you please take a quick squiz at the final 2 patches if you get a
> > chance.  I assumed we are in preemptible context during early_init based
> > on your code (and code comment) and called static_branch_disable()
> > directly if hw RNG returned keying material.  It's a pretty simple
> > change but I'd love to get someone else to check I've not noob'ed it.
> 
> I can take a look, and perhaps do some tests. But it was Anna-Maria
> that originally triggered the issue. She's on Cc, perhaps she can try
> this and see if it works.

I'll test it today - sorry for the delay.

Anna-Maria

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Anna-Maria Gleixner]

On Tue, 5 Jun 2018, Anna-Maria Gleixner wrote:

> On Thu, 31 May 2018, Steven Rostedt wrote:
> 
> > On Mon, 28 May 2018 11:46:38 +1000
> > "Tobin C. Harding" <me@tobin.cc> wrote:
> > 
> > > Steve,
> > 
> > Hi Tobin,
> > 
> > Sorry for the late reply, I'm currently at a conference and have had
> > little time to read email.
> > 
> > > 
> > > Could you please take a quick squiz at the final 2 patches if you get a
> > > chance.  I assumed we are in preemptible context during early_init based
> > > on your code (and code comment) and called static_branch_disable()
> > > directly if hw RNG returned keying material.  It's a pretty simple
> > > change but I'd love to get someone else to check I've not noob'ed it.
> > 
> > I can take a look, and perhaps do some tests. But it was Anna-Maria
> > that originally triggered the issue. She's on Cc, perhaps she can try
> > this and see if it works.
> 
> I'll test it today - sorry for the delay.
> 

I tested it with command line option enabled. This works early enough
because it works right after early_trace_init().

Thanks,

	Anna-Maria

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Tobin C. Harding]

On Wed, Jun 06, 2018 at 03:08:25PM +0200, Anna-Maria Gleixner wrote:
> On Tue, 5 Jun 2018, Anna-Maria Gleixner wrote:
> 
> > On Thu, 31 May 2018, Steven Rostedt wrote:
> > 
> > > On Mon, 28 May 2018 11:46:38 +1000
> > > "Tobin C. Harding" <me@tobin.cc> wrote:
> > > 
> > > > Steve,
> > > 
> > > Hi Tobin,
> > > 
> > > Sorry for the late reply, I'm currently at a conference and have had
> > > little time to read email.
> > > 
> > > > 
> > > > Could you please take a quick squiz at the final 2 patches if you get a
> > > > chance.  I assumed we are in preemptible context during early_init based
> > > > on your code (and code comment) and called static_branch_disable()
> > > > directly if hw RNG returned keying material.  It's a pretty simple
> > > > change but I'd love to get someone else to check I've not noob'ed it.
> > > 
> > > I can take a look, and perhaps do some tests. But it was Anna-Maria
> > > that originally triggered the issue. She's on Cc, perhaps she can try
> > > this and see if it works.
> > 
> > I'll test it today - sorry for the delay.
> > 
> 
> I tested it with command line option enabled. This works early enough
> because it works right after early_trace_init().

Awesome, thanks Anna-Maria!

	Tobin

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Thomas Gleixner]

On Mon, 28 May 2018, Tobin C. Harding wrote:

> Currently printing pointers early in the boot sequence can result in a
> dummy string '(____ptrval____)' being printed.  While resolving this
> issue it was noticed that we can use the hw RNG if available for hashing
> pointers.
> 
> Patch one and two do the ground work to be able to use hw RNG removing
> from get_random_bytes_arch() the call to get_random_bytes() and
> returning the number of bytes of random material successfully returned. 
> 
> Patch three uses the hw RNG to get keying material if it is available.
> 
> Patch four further assists debugging early in the boot sequence for
> machines that do not have a hw RNG by adding a command line option
> 'debug_boot_weak_hash'.  If enabled, non-cryptographically secure hashing
> is used instead of siphash so we can hash at any time. 
> 
> During the versions of this set I have been totally confused about which
> patches go through which tree.  This version again puts all 4 patches
> together in the hope they will go through Andrew's tree.
> 
> 
> Steve,
> 
> Could you please take a quick squiz at the final 2 patches if you get a
> chance.  I assumed we are in preemptible context during early_init based
> on your code (and code comment) and called static_branch_disable()
> directly if hw RNG returned keying material.  It's a pretty simple
> change but I'd love to get someone else to check I've not noob'ed it.

early_initcalls() are not that early :) They run in thread context fully
preemtible so calling static_branch_disable() is perfectly fine.

Thanks,

	tglx

Re: [PATCH v6 0/4] enable early printing of hashed pointers

[Tobin C. Harding]

On Wed, Jun 06, 2018 at 03:02:20PM +0200, Thomas Gleixner wrote:
> On Mon, 28 May 2018, Tobin C. Harding wrote:
> 
> > Currently printing pointers early in the boot sequence can result in a
> > dummy string '(____ptrval____)' being printed.  While resolving this
> > issue it was noticed that we can use the hw RNG if available for hashing
> > pointers.
> > 
> > Patch one and two do the ground work to be able to use hw RNG removing
> > from get_random_bytes_arch() the call to get_random_bytes() and
> > returning the number of bytes of random material successfully returned. 
> > 
> > Patch three uses the hw RNG to get keying material if it is available.
> > 
> > Patch four further assists debugging early in the boot sequence for
> > machines that do not have a hw RNG by adding a command line option
> > 'debug_boot_weak_hash'.  If enabled, non-cryptographically secure hashing
> > is used instead of siphash so we can hash at any time. 
> > 
> > During the versions of this set I have been totally confused about which
> > patches go through which tree.  This version again puts all 4 patches
> > together in the hope they will go through Andrew's tree.
> > 
> > 
> > Steve,
> > 
> > Could you please take a quick squiz at the final 2 patches if you get a
> > chance.  I assumed we are in preemptible context during early_init based
> > on your code (and code comment) and called static_branch_disable()
> > directly if hw RNG returned keying material.  It's a pretty simple
> > change but I'd love to get someone else to check I've not noob'ed it.
> 
> early_initcalls() are not that early :) They run in thread context fully
> preemtible so calling static_branch_disable() is perfectly fine.

Thanks for the explanation.

	Tobin.