Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1577253 > unrolled thread

Re: [PATCH] Revert "hwrng: core - zeroize buffers with random data"

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] Revert "hwrng: core - zeroize buffers with random data" Linus Torvalds <torvalds@linux-foundation.org> - 2017-02-09 03:00 +0100
    Re: [PATCH] Revert "hwrng: core - zeroize buffers with random data" Stephan Müller <smueller@chronox.de> - 2017-02-09 10:40 +0100

#1577253 — Re: [PATCH] Revert "hwrng: core - zeroize buffers with random data"

Stephan, Herbert? The zeroes in /dev/hwrng output are obviously
complete crap, so there's something badly wrong somewhere.

The locking, for example, is completely buggered. There's even a
comment about it, but that comment makes the correct observation of
"but y'know: randomness". But the memset() also being outside the lock
makes a complete joke of the whole thing.

Is the hwrng thing even worth maintaining? Compared to something like
/dev/urandom, it clearly does not do a very good job.

So I'm inclined to take the revert, but I'm also somewhat inclined to
simply mark this crud broken when we have other things that clearly do
a lot better.

              Linus

On Tue, Feb 7, 2017 at 4:23 PM, David Daney <david.daney@cavium.com> wrote:
> This reverts commit 2cc751545854d7bd7eedf4d7e377bb52e176cd07.

[toc] | [next] | [standalone]

#1577418

Am Mittwoch, 8. Februar 2017, 17:57:23 CET schrieb Linus Torvalds:

Hi Linus,

> Stephan, Herbert? The zeroes in /dev/hwrng output are obviously
> complete crap, so there's something badly wrong somewhere.
> 
> The locking, for example, is completely buggered. There's even a
> comment about it, but that comment makes the correct observation of
> "but y'know: randomness". But the memset() also being outside the lock
> makes a complete joke of the whole thing.

That is correct, the patch is broken and should be reverted.

May I ask, however, why the add_device_randomness is invoked outside the lock 
as well. Shouldn't it be moved into the lock?

Besides, I still would think that a memset(0) is needed because we have long-
living memory locations (rng_buffer and rng_fillbuf) which may be overwritten 
sporadically. As these memory locations are expected to hold entropy, they 
should be overwritten as soon as the data is processed. Obviously, such memset 
must be done within the lock.

Ciao
Stephan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web