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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <10800323.FrV6YrZeG5@positron.chronox.de>
Date:   Thu, 09 Feb 2017 10:32:18 +0100
From:   Stephan Müller <smueller@...onox.de>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     David Daney <david.daney@...ium.com>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        Matt Mackall <mpm@...enic.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Revert "hwrng: core - zeroize buffers with random data"

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