Re: [RFC PATCH v12 2/4] random: conditionally compile code depending on LRNG

2017-07-18 Thread Stephan Müller
Am Dienstag, 18. Juli 2017, 10:47:00 CEST schrieb Arnd Bergmann:

Hi Arnd,

> On Tue, Jul 18, 2017 at 10:37 AM, Stephan Müller  
wrote:
> > Am Dienstag, 18. Juli 2017, 10:13:55 CEST schrieb Arnd Bergmann:
> >> On Tue, Jul 18, 2017 at 9:58 AM, Stephan Müller  
wrote:
> >> > When selecting the LRNG for compilation, disable add_disk_randomness
> >> > and
> >> > its supporting function.
> >> > 
> >> > CC: Greg Kroah-Hartman 
> >> > CC: Arnd Bergmann 
> >> > CC: Jason A. Donenfeld 
> >> > Signed-off-by: Stephan Mueller 
> >> 
> >> I think this needs a better explanation. Why do we ignore the extra
> >> entropy here?
> > 
> > I was not sure whether to add all the details about the reason into the
> > patch submission.
> > 
> > The reason is explained here in [1] page 3 and re-iterated in [2].
> 
> Ok, got it. A half-sentence summary of that ("... to avoid adding the
> same event twice from interrupt and block") would be sufficient for
> the patch description, longer is also fine.

Perfect, thank you for that hint. I will add this information to a next 
iteration.
> 
> Generally speaking, each patch description should describe why
> that particular patch is required rather than describe what it does
> (which in cases like this is plain to see from looking a few lines
> down).
> 
> Arnd



Ciao
Stephan


Re: [RFC PATCH v12 2/4] random: conditionally compile code depending on LRNG

2017-07-18 Thread Arnd Bergmann
On Tue, Jul 18, 2017 at 10:37 AM, Stephan Müller  wrote:
> Am Dienstag, 18. Juli 2017, 10:13:55 CEST schrieb Arnd Bergmann:
>> On Tue, Jul 18, 2017 at 9:58 AM, Stephan Müller  wrote:
>> > When selecting the LRNG for compilation, disable add_disk_randomness and
>> > its supporting function.
>> >
>> > CC: Greg Kroah-Hartman 
>> > CC: Arnd Bergmann 
>> > CC: Jason A. Donenfeld 
>> > Signed-off-by: Stephan Mueller 
>>
>> I think this needs a better explanation. Why do we ignore the extra
>> entropy here?
>
> I was not sure whether to add all the details about the reason into the patch
> submission.
>
> The reason is explained here in [1] page 3 and re-iterated in [2].
>

Ok, got it. A half-sentence summary of that ("... to avoid adding the
same event twice from interrupt and block") would be sufficient for
the patch description, longer is also fine.

Generally speaking, each patch description should describe why
that particular patch is required rather than describe what it does
(which in cases like this is plain to see from looking a few lines
down).

Arnd


Re: [RFC PATCH v12 2/4] random: conditionally compile code depending on LRNG

2017-07-18 Thread Stephan Müller
Am Dienstag, 18. Juli 2017, 10:13:55 CEST schrieb Arnd Bergmann:

Hi Arnd,

> On Tue, Jul 18, 2017 at 9:58 AM, Stephan Müller  wrote:
> > When selecting the LRNG for compilation, disable add_disk_randomness and
> > its supporting function.
> > 
> > CC: Greg Kroah-Hartman 
> > CC: Arnd Bergmann 
> > CC: Jason A. Donenfeld 
> > Signed-off-by: Stephan Mueller 
> 
> I think this needs a better explanation. Why do we ignore the extra
> entropy here?

I was not sure whether to add all the details about the reason into the patch 
submission.

The reason is explained here in [1] page 3 and re-iterated in [2].

The gist is the following:

A HID or block device event providing entropy to the respective individual 
noise sources processing generates an interrupt. These interrupts are also 
processed by the interrupt noise source. The majority of entropy is delivered 
by the high-resolution time stamp of the occurrence of such an event. Now, 
that event is processed twice in the legacy /dev/random implementation: once 
by the HID or block device noise source and once by the interrupt noise 
source. Thus, the two time stamps of the one event (HID noise source and 
interrupt noise source, or block device noise source and interrupt noise 
source) used as a basis for entropy are highly correlated. Correlation or even 
a possible reuse of the same random value diminishes entropy significantly.

The additional data provided via the block noise source (block device number) 
has no real entropy.

Bottom line: for entropy, the HID and block device noise sources are just a 
derivative of the interrupt noise source. Thus, discarding the block device 
noise source will not lose any entropy. Regarding the HID noise source, only 
the key/mouse event numbers are injected into the LRNG without attributing any 
entropy to them.

[1] http://www.chronox.de/lrng/doc/lrng.pdf

[2] https://www.spinics.net/lists/linux-crypto/msg26316.html

Ciao
Stephan


Re: [RFC PATCH v12 2/4] random: conditionally compile code depending on LRNG

2017-07-18 Thread Arnd Bergmann
On Tue, Jul 18, 2017 at 9:58 AM, Stephan Müller  wrote:
> When selecting the LRNG for compilation, disable add_disk_randomness and
> its supporting function.
>
> CC: Greg Kroah-Hartman 
> CC: Arnd Bergmann 
> CC: Jason A. Donenfeld 
> Signed-off-by: Stephan Mueller 

I think this needs a better explanation. Why do we ignore the extra
entropy here?

   Arnd


[RFC PATCH v12 2/4] random: conditionally compile code depending on LRNG

2017-07-18 Thread Stephan Müller
When selecting the LRNG for compilation, disable add_disk_randomness and
its supporting function.

CC: Greg Kroah-Hartman 
CC: Arnd Bergmann 
CC: Jason A. Donenfeld 
Signed-off-by: Stephan Mueller 
---
 include/linux/genhd.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index e619fae..7e08ebc 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -422,8 +422,13 @@ extern void disk_flush_events(struct gendisk *disk, 
unsigned int mask);
 extern unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask);
 
 /* drivers/char/random.c */
+#ifdef CONFIG_LRNG
+#define add_disk_randomness(disk) do {} while (0)
+#define rand_initialize_disk(disk) do {} while (0)
+#else
 extern void add_disk_randomness(struct gendisk *disk) __latent_entropy;
 extern void rand_initialize_disk(struct gendisk *disk);
+#endif
 
 static inline sector_t get_start_sect(struct block_device *bdev)
 {
-- 
2.9.4