2011-02-17 22:23:40

by Jarod Wilson

[permalink] [raw]
Subject: [PATCH] random: update interface comments to reflect reality

At present, the comment header in random.c makes no mention of
add_disk_randomness, and instead, suggests that disk activity adds to the
random pool by way of add_interrupt_randomness, which appears to not have
been the case since sometime prior to the existence of git, and even prior
to bitkeeper. Didn't look any further back. At least, as far as I can
tell, there are no storage drivers setting IRQF_SAMPLE_RANDOM, which is a
requirement for add_interrupt_randomness to trigger, so the only way for a
disk to contribute entropy is by way of add_disk_randomness. Update
comments accordingly, complete with special mention about solid state
drives being a crappy source of entropy (see e2e1a148bc for reference).

Signed-off-by: Jarod Wilson <[email protected]>

---
drivers/char/random.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 908ac1f..3dba627 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -128,6 +128,7 @@
* void add_input_randomness(unsigned int type, unsigned int code,
* unsigned int value);
* void add_interrupt_randomness(int irq);
+ * void add_disk_randomness(struct gendisk *disk);
*
* add_input_randomness() uses the input layer interrupt timing, as well as
* the event type information from the hardware.
@@ -136,9 +137,15 @@
* inputs to the entropy pool. Note that not all interrupts are good
* sources of randomness! For example, the timer interrupts is not a
* good choice, because the periodicity of the interrupts is too
- * regular, and hence predictable to an attacker. Disk interrupts are
- * a better measure, since the timing of the disk interrupts are more
- * unpredictable.
+ * regular, and hence predictable to an attacker. Network Interface
+ * Controller interrupts are a better measure, since the timing of the
+ * NIC interrupts are more unpredictable.
+ *
+ * add_disk_randomness() uses what amounts to the seek time of block
+ * layer request events, on a per-disk_devt basis, as input to the
+ * entropy pool. Note that high-speed solid state drives with very low
+ * seek times do not make for good sources of entropy, as their seek
+ * times are usually fairly consistent.
*
* All of these routines try to estimate how many bits of randomness a
* particular randomness source. They do this by keeping track of the


--
Jarod Wilson
[email protected]


2011-02-17 22:29:20

by Matt Mackall

[permalink] [raw]
Subject: Re: [PATCH] random: update interface comments to reflect reality

On Thu, 2011-02-17 at 17:23 -0500, Jarod Wilson wrote:
> At present, the comment header in random.c makes no mention of
> add_disk_randomness, and instead, suggests that disk activity adds to the
> random pool by way of add_interrupt_randomness, which appears to not have
> been the case since sometime prior to the existence of git, and even prior
> to bitkeeper. Didn't look any further back. At least, as far as I can
> tell, there are no storage drivers setting IRQF_SAMPLE_RANDOM, which is a
> requirement for add_interrupt_randomness to trigger, so the only way for a
> disk to contribute entropy is by way of add_disk_randomness. Update
> comments accordingly, complete with special mention about solid state
> drives being a crappy source of entropy (see e2e1a148bc for reference).
>
> Signed-off-by: Jarod Wilson <[email protected]>

Sure. Herbert, let's route this through your crypto tree.

Acked-by: Matt Mackall <[email protected]>

> ---
> drivers/char/random.c | 13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 908ac1f..3dba627 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -128,6 +128,7 @@
> * void add_input_randomness(unsigned int type, unsigned int code,
> * unsigned int value);
> * void add_interrupt_randomness(int irq);
> + * void add_disk_randomness(struct gendisk *disk);
> *
> * add_input_randomness() uses the input layer interrupt timing, as well as
> * the event type information from the hardware.
> @@ -136,9 +137,15 @@
> * inputs to the entropy pool. Note that not all interrupts are good
> * sources of randomness! For example, the timer interrupts is not a
> * good choice, because the periodicity of the interrupts is too
> - * regular, and hence predictable to an attacker. Disk interrupts are
> - * a better measure, since the timing of the disk interrupts are more
> - * unpredictable.
> + * regular, and hence predictable to an attacker. Network Interface
> + * Controller interrupts are a better measure, since the timing of the
> + * NIC interrupts are more unpredictable.
> + *
> + * add_disk_randomness() uses what amounts to the seek time of block
> + * layer request events, on a per-disk_devt basis, as input to the
> + * entropy pool. Note that high-speed solid state drives with very low
> + * seek times do not make for good sources of entropy, as their seek
> + * times are usually fairly consistent.
> *
> * All of these routines try to estimate how many bits of randomness a
> * particular randomness source. They do this by keeping track of the
>
>


--
Mathematics is the supreme nostalgia of our time.

2011-02-21 10:43:35

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] random: update interface comments to reflect reality

On Thu, Feb 17, 2011 at 04:29:14PM -0600, Matt Mackall wrote:
> On Thu, 2011-02-17 at 17:23 -0500, Jarod Wilson wrote:
> > At present, the comment header in random.c makes no mention of
> > add_disk_randomness, and instead, suggests that disk activity adds to the
> > random pool by way of add_interrupt_randomness, which appears to not have
> > been the case since sometime prior to the existence of git, and even prior
> > to bitkeeper. Didn't look any further back. At least, as far as I can
> > tell, there are no storage drivers setting IRQF_SAMPLE_RANDOM, which is a
> > requirement for add_interrupt_randomness to trigger, so the only way for a
> > disk to contribute entropy is by way of add_disk_randomness. Update
> > comments accordingly, complete with special mention about solid state
> > drives being a crappy source of entropy (see e2e1a148bc for reference).
> >
> > Signed-off-by: Jarod Wilson <[email protected]>
>
> Sure. Herbert, let's route this through your crypto tree.
>
> Acked-by: Matt Mackall <[email protected]>

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt