Re: /dev/urandom uses uninit bytes, leaks user data

[John Reiser <jreiser@BitWagon.com>]

Matt Mackall wrote:
> Yes, we use uninitialized data. But it's not a leak in any useful
> sense. To the extent the previous data is secret, this actually
> improves our entropy.
> 
> It's getting folded into the random number pool, where it will be
> impossible to recover it unless you already know what was in the
> pool.  And if you know what's in the pool, you've already broken into
> the kernel.

The combination of capturing data from other users, plus seeding
the pool with your own data, just might be powerful enough to help
steal secrets, sometime in the next five years, from data that is
recorded today.

> But I'm sympathetic to making Valgrind happy.  ...

> [The code in John's patch is] hideous.  How about a memset instead ...

> And [John's] change is broken..   We have to add precisely the
> number of bytes returned by extract_entropy to keep the books
> balanced.

Matt is correct.  The rolled-up result follows.

Signed off by: jreiser@BitWagon.com

--- ./drivers/char/random.c.orig	2007-12-14 11:06:03.000000000 -0800
+++ ./drivers/char/random.c	2007-12-14 12:27:23.000000000 -0800
@@ -708,6 +708,8 @@

 		bytes=extract_entropy(r->pull, tmp, bytes,
 				      random_read_wakeup_thresh / 8, rsvd);
+		/* clear uninitialized bytes at the end to make valgrind happy */
+		memset((char *)tmp + bytes, 0, -bytes & 3);
 		add_entropy_words(r, tmp, (bytes + 3) / 4);
 		credit_entropy_store(r, bytes*8);
 	}
-- 
John Reiser, jreiser@BitWagon.com