On Sat, Jul 07, 2012 at 02:23:16PM -0400, Theodore Ts'o wrote:

On Sat, Jul 07, 2012 at 10:11:22AM -0700, Kees Cook wrote:

quoted

While very unlikely, it is possible for arch_get_random_long() to fail
in the middle of the loop in xfer_secondary_pool(), which would mean
that the loop could stop with only part of u.hwrand populated, leading
to mix_pool_bytes() injecting uninitialized or already injected bytes
instead of fresh bytes. This changes the mix_pool_bytes() call to only
inject the successfully gathered bytes.

I don't believe there is a major problem with injecting uninitialized
or even known bytes into the pool; worst case we're wastiing a tiny
amount of CPU in this unlikely case (versus the CPU costs of doing the
multiplication each time).  Not that I think really matters one way or
the other... 

Is there a reason why you're particularly concerned about what might
happen in the case where arch_get_random_long() fails mid-loop (which
can happen if RDRAND returns an error for whatever reason, granted)?

Not really, but it seems like poor form to try to mix in data that wasn't
actually what you were expecting. I don't exactly see a problem with what
you've already got, but it seems like it's better to not have the bug at
all.

-Kees

-- 
Kees Cook
Chrome OS Security