[PATCH v5 0/3] enable early printing of hashed pointers

[PATCH v5 0/3] enable early printing of hashed pointers

[Tobin C. Harding]

Currently if an attempt is made to print a pointer before there is
enough entropy then '(____ptrval____)' is printed.  This makes debugging
stack traces during early boot difficult.

One partial solution to this problem is to use the hw RNG if it is
available.

Patch 1 - Whitespace fixes.
Patch 2 - Fix get_random_bytes_arch()
Patch 3 - Use hw RNG for pointer hashing if available (by default).


thanks,
Tobin.

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

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

v2 -> v3
 - Add __ro_after_init (suggested by Kees).

v1 -> 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)



*** BLURB HERE ***

Tobin C. Harding (3):
  random: Fix whitespace pre random-bytes work
  random: Return nbytes filled from hw RNG
  vsprintf: Use hw RNG for ptr_key

 drivers/char/random.c  | 19 ++++++++++---------
 include/linux/random.h |  2 +-
 lib/vsprintf.c         | 19 ++++++++++++++++---
 3 files changed, 27 insertions(+), 13 deletions(-)

-- 
2.7.4

[PATCH v5 1/3] 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 v5 2/3] 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 v5 3/3] 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.

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

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index b82f0c6c2aec..a1567dd7066c 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1657,9 +1657,8 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 static bool have_filled_random_ptr_key __read_mostly;
 static siphash_key_t ptr_key __read_mostly;
 
-static void fill_random_ptr_key(struct random_ready_callback *unused)
+static void ptr_key_ready(void)
 {
-	get_random_bytes(&ptr_key, sizeof(ptr_key));
 	/*
 	 * have_filled_random_ptr_key==true is dependent on get_random_bytes().
 	 * ptr_to_id() needs to see have_filled_random_ptr_key==true
@@ -1669,14 +1668,28 @@ static void fill_random_ptr_key(struct random_ready_callback *unused)
 	WRITE_ONCE(have_filled_random_ptr_key, true);
 }
 
+static void fill_random_ptr_key(struct random_ready_callback *unused)
+{
+	get_random_bytes(&ptr_key, sizeof(ptr_key));
+	ptr_key_ready();
+}
+
 static struct random_ready_callback random_ready = {
 	.func = fill_random_ptr_key
 };
 
 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) {
+		ptr_key_ready();
+		return 0;
+	}
 
+	ret = add_random_ready_callback(&random_ready);
 	if (!ret) {
 		return 0;
 	} else if (ret == -EALREADY) {
-- 
2.7.4

Re: [PATCH v5 0/3] enable early printing of hashed pointers

[Steven Rostedt]

I think the series looks good, although if Linus takes my last patch,
it may conflict badly with the third patch. I'll have to look into that.

Anyway, Andrew, you want to take this in your tree?

-- Steve


On Wed, 16 May 2018 09:17:36 +1000
"Tobin C. Harding" <me@tobin.cc> wrote:

> Currently if an attempt is made to print a pointer before there is
> enough entropy then '(____ptrval____)' is printed.  This makes debugging
> stack traces during early boot difficult.
> 
> One partial solution to this problem is to use the hw RNG if it is
> available.
> 
> Patch 1 - Whitespace fixes.
> Patch 2 - Fix get_random_bytes_arch()
> Patch 3 - Use hw RNG for pointer hashing if available (by default).
> 
> 
> thanks,
> Tobin.
> 
> v4 -> v5
>  - Use 'upside-down-xmas-tree' style to declare local variables (Steve)
>  - Added Reviewed-by tag from Steve (patch 2 and 3).
> 
> v3 -> v4
>  - remove last patch of series (command line option patch)
> 
> v2 -> v3
>  - Add __ro_after_init (suggested by Kees).
> 
> v1 -> 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)
> 
> 
> 
> *** BLURB HERE ***
> 
> Tobin C. Harding (3):
>   random: Fix whitespace pre random-bytes work
>   random: Return nbytes filled from hw RNG
>   vsprintf: Use hw RNG for ptr_key
> 
>  drivers/char/random.c  | 19 ++++++++++---------
>  include/linux/random.h |  2 +-
>  lib/vsprintf.c         | 19 ++++++++++++++++---
>  3 files changed, 27 insertions(+), 13 deletions(-)
> 

Re: [PATCH v5 0/3] enable early printing of hashed pointers

[Tobin C. Harding]

On Wed, May 16, 2018 at 11:13:48AM -0400, Steven Rostedt wrote:
> 
> I think the series looks good, although if Linus takes my last patch,
> it may conflict badly with the third patch. I'll have to look into that.

I applied your PR patch ontop of mainline (imitating Linus pulling) any
yes patch 3 of this set has a merge conflict.

> Anyway, Andrew, you want to take this in your tree?

I split this set up into it's current form because Ted offered to take
it through his tree.  There has been no response from him though since
doing so.  He has ack'ed the first 2 patches (that touch
drivers/char/random.c and include/linux/random.c).

Since this is not overly urgent stuff is it easier for everyone involved
if I wait until the PR to Linus goes through then rebase this, add the
other vsprintf patch back onto the set and send it all to Andrew?

Usual disclaimer about being a massive noob, please correct me if I am
out of line.  My aim is to make the job of others more simple.

thanks,
Tobin.

Re: [PATCH v5 0/3] enable early printing of hashed pointers

[Steven Rostedt]

[ Steve is back from vacation ]

On Thu, 17 May 2018 08:41:58 +1000
"Tobin C. Harding" <me@tobin.cc> wrote:

> On Wed, May 16, 2018 at 11:13:48AM -0400, Steven Rostedt wrote:
> > 
> > I think the series looks good, although if Linus takes my last patch,
> > it may conflict badly with the third patch. I'll have to look into that.  
> 
> I applied your PR patch ontop of mainline (imitating Linus pulling) any
> yes patch 3 of this set has a merge conflict.
> 
> > Anyway, Andrew, you want to take this in your tree?  
> 
> I split this set up into it's current form because Ted offered to take
> it through his tree.  There has been no response from him though since
> doing so.  He has ack'ed the first 2 patches (that touch
> drivers/char/random.c and include/linux/random.c).

Hopefully Ted takes this then.

> 
> Since this is not overly urgent stuff is it easier for everyone involved
> if I wait until the PR to Linus goes through then rebase this, add the
> other vsprintf patch back onto the set and send it all to Andrew?

Linus already pulled my commit in his tree. You can base your latest
work on top of it and post it again. I'm sure Andrew could take it
after that.

> 
> Usual disclaimer about being a massive noob, please correct me if I am
> out of line.  My aim is to make the job of others more simple.

No worries, you're doing a fine job ;-)

-- Steve