2014-04-14 16:02:20

by Torsten Duwe

[permalink] [raw]
Subject: [PATCH v3 00/03]: hwrng: an in-kernel rngd

More or less a resend of v2.

On Wed, Mar 26, 2014 at 06:03:37PM -0700, H. Peter Anvin wrote:
> I'm wondering more about the default. We default to 50% for arch_get_random_seed, and this is supposed to be the default for in effect unverified hwrngs...

Done. 50% is now the default, that's the only change from v2.

Andy: the printk you pointed out already limits itself to 1/10s,
which is half the default rate limit. Also, as Peter already
wrote, we're dealing with true HWRNGs here; if such a device
does not produce a single byte within 10 seconds something _is_
severely broken and, like a dying disk, worth to be logged.
Here's one of the better circuits I found:
http://www.maximintegrated.com/app-notes/index.mvp/id/3469
or offline:
http://pdfserv.maximintegrated.com/en/an/AN3469.pdf
Disclaimer: I'm not endorsing Maxim, it's just that paper
that hits the spot IMHO.

Anything wrong with feeding those bits into the input pool?
Any other comments on the code?

Torsten


2014-04-14 16:04:44

by Torsten Duwe

[permalink] [raw]
Subject: [PATCH v3 01/03]: hwrng: provide an injection point for pure hardware randomness


This patch adds an interface to the random pool for feeding entropy in-kernel.
It may serve as a destination for dedicated HWRNGs.

It resembles -- and could be merged with -- the ioctl(RNDADDENTROPY) code, plus
a sleep condition for eager writers.

Signed-off-by: Torsten Duwe <[email protected]>

---
include/linux/hw_random.h | 2 ++
drivers/char/random.c | 23 +++++++++++++++++++++++
2 files changed, 25 insertions(+)

--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -47,5 +47,7 @@ struct hwrng {
extern int hwrng_register(struct hwrng *rng);
/** Unregister a Hardware Random Number Generator driver. */
extern void hwrng_unregister(struct hwrng *rng);
+/** Feed random bits into the pool. */
+extern void add_hwgenerator_randomness(const char *buffer, size_t count, size_t entropy);

#endif /* LINUX_HWRANDOM_H_ */
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -250,6 +250,7 @@
#include <linux/interrupt.h>
#include <linux/mm.h>
#include <linux/spinlock.h>
+#include <linux/kthread.h>
#include <linux/percpu.h>
#include <linux/cryptohash.h>
#include <linux/fips.h>
@@ -1347,3 +1347,25 @@ randomize_range(unsigned long start, uns
return 0;
return PAGE_ALIGN(get_random_int() % range + start);
}
+
+/* Interface for in-kernel drivers of true hardware RNGs.
+ * Those devices may produce endless random bits and will be throttled
+ * when our pool is full.
+ */
+void add_hwgenerator_randomness(const char *buffer, size_t count,
+ size_t entropy)
+{
+ struct entropy_store *poolp = &input_pool;
+
+ /* Suspend writing if we're above the trickle threshold.
+ * We'll be woken up again once below random_write_wakeup_thresh,
+ * or when the calling thread is about to terminate.
+ */
+ wait_event_interruptible(random_write_wait, kthread_should_stop() ||
+ input_pool.entropy_count
+ <= random_write_wakeup_thresh);
+ mix_pool_bytes(poolp, buffer, count, NULL);
+ credit_entropy_bits(poolp, entropy);
+}
+EXPORT_SYMBOL_GPL(add_hwgenerator_randomness);
+

2014-04-14 16:05:52

by Torsten Duwe

[permalink] [raw]
Subject: [PATCH v3 02/03]: hwrng: create filler thread


This can be viewed as the in-kernel equivalent of hwrngd;
like FUSE it is a good thing to have a mechanism in user land,
but for some reasons (simplicity, secrecy, integrity, speed)
it may be better to have it in kernel space.

This patch creates a thread once a hwrng registers, and uses
the previously established add_hwgenerator_randomness() to feed
its data to the input pool as long as needed. A derating factor
is used to bias the entropy estimation and to disable this
mechanism entirely when set to zero.

Signed-off-by: Torsten Duwe <[email protected]>

---
drivers/char/hw_random/core.c | 65 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 62 insertions(+), 3 deletions(-)

--- linux/drivers/char/hw_random/core.c.orig
+++ linux/drivers/char/hw_random/core.c
@@ -39,6 +39,7 @@
#include <linux/sched.h>
#include <linux/init.h>
#include <linux/miscdevice.h>
+#include <linux/kthread.h>
#include <linux/delay.h>
#include <linux/slab.h>
#include <asm/uaccess.h>
@@ -50,10 +51,18 @@


static struct hwrng *current_rng;
+static struct task_struct *hwrng_fill;
static LIST_HEAD(rng_list);
static DEFINE_MUTEX(rng_mutex);
static int data_avail;
-static u8 *rng_buffer;
+static u8 *rng_buffer, *rng_fillbuf;
+static unsigned short derating_current = 700; /* an arbitrary 70% */
+
+module_param(derating_current, ushort, 0644);
+MODULE_PARM_DESC(derating_current,
+ "current hwrng entropy estimation per mill");
+
+static void start_khwrngd(void);

static size_t rng_buffer_size(void)
{
@@ -62,9 +71,18 @@ static size_t rng_buffer_size(void)

static inline int hwrng_init(struct hwrng *rng)
{
+ int err;
+
if (!rng->init)
return 0;
- return rng->init(rng);
+ err = rng->init(rng);
+ if (err)
+ return err;
+
+ if (derating_current > 0 && !hwrng_fill)
+ start_khwrngd();
+
+ return 0;
}

static inline void hwrng_cleanup(struct hwrng *rng)
@@ -300,6 +318,36 @@ err_misc_dereg:
goto out;
}

+static int hwrng_fillfn(void *unused)
+{
+ long rc;
+
+ while (!kthread_should_stop()) {
+ if (!current_rng)
+ break;
+ rc = rng_get_data(current_rng, rng_fillbuf,
+ rng_buffer_size(), 1);
+ if (rc <= 0) {
+ pr_warn("hwrng: no data available\n");
+ msleep_interruptible(10000);
+ continue;
+ }
+ add_hwgenerator_randomness((void *)rng_fillbuf, rc,
+ (rc*derating_current)>>10);
+ }
+ hwrng_fill = 0;
+ return 0;
+}
+
+static void start_khwrngd(void)
+{
+ hwrng_fill = kthread_run(hwrng_fillfn, NULL, "hwrng");
+ if (hwrng_fill == ERR_PTR(-ENOMEM)) {
+ pr_err("hwrng_fill thread creation failed");
+ hwrng_fill = NULL;
+ }
+}
+
int hwrng_register(struct hwrng *rng)
{
int must_register_misc;
@@ -319,6 +367,13 @@ int hwrng_register(struct hwrng *rng)
if (!rng_buffer)
goto out_unlock;
}
+ if (!rng_fillbuf) {
+ rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL);
+ if (!rng_fillbuf) {
+ kfree(rng_buffer);
+ goto out_unlock;
+ }
+ }

/* Must not register two RNGs with the same name. */
err = -EEXIST;
@@ -373,8 +428,11 @@ void hwrng_unregister(struct hwrng *rng)
current_rng = NULL;
}
}
- if (list_empty(&rng_list))
+ if (list_empty(&rng_list)) {
unregister_miscdev();
+ if (hwrng_fill)
+ kthread_stop(hwrng_fill);
+ }

mutex_unlock(&rng_mutex);
}
@@ -385,6 +443,7 @@ static void __exit hwrng_exit(void)
mutex_lock(&rng_mutex);
BUG_ON(current_rng);
kfree(rng_buffer);
+ kfree(rng_fillbuf);
mutex_unlock(&rng_mutex);
}

2014-04-14 16:06:57

by Torsten Duwe

[permalink] [raw]
Subject: [PATCH v3 03/03]: hwrng: khwrngd derating per device


This patch introduces a derating factor to struct hwrng for
the random bits going into the kernel input pool, and a common
default derating for drivers which do not specify one.

Signed-off-by: Torsten Duwe <[email protected]>

---
drivers/char/hw_random/core.c | 11 ++++++++++-
include/linux/hw_random.h | 3 +++
2 files changed, 13 insertions(+), 1 deletion(-)

--- linux/include/linux/hw_random.h.orig
+++ linux/include/linux/hw_random.h
@@ -29,6 +29,8 @@
* @read: New API. drivers can fill up to max bytes of data
* into the buffer. The buffer is aligned for any type.
* @priv: Private data, for use by the RNG driver.
+ * @derating: Estimation of true entropy in RNG's bitstream
+ * (per mill).
*/
struct hwrng {
const char *name;
@@ -38,6 +39,7 @@ struct hwrng {
int (*data_read)(struct hwrng *rng, u32 *data);
int (*read)(struct hwrng *rng, void *data, size_t max, bool wait);
unsigned long priv;
+ unsigned short derating;

/* internal. */
struct list_head list;
--- linux/drivers/char/hw_random/core.c.orig
+++ linux/drivers/char/hw_random/core.c
@@ -56,11 +56,15 @@ static LIST_HEAD(rng_list);
static DEFINE_MUTEX(rng_mutex);
static int data_avail;
static u8 *rng_buffer, *rng_fillbuf;
-static unsigned short derating_current = 700; /* an arbitrary 70% */
+static unsigned short derating_current;
+static unsigned short derating_default = 500; /* an arbitrary 50% */

module_param(derating_current, ushort, 0644);
MODULE_PARM_DESC(derating_current,
"current hwrng entropy estimation per mill");
+module_param(derating_default, ushort, 0644);
+MODULE_PARM_DESC(derating_default,
+ "default entropy content of hwrng per mill");

static void start_khwrngd(void);

@@ -79,6 +83,11 @@ static inline int hwrng_init(struct hwrn
if (err)
return err;

+ derating_current = rng->derating ? : derating_default;
+ derating_current &= 1023;
+
+ if (derating_current == 0 && hwrng_fill)
+ kthread_stop(hwrng_fill);
if (derating_current > 0 && !hwrng_fill)
start_khwrngd();

2014-04-14 16:10:17

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v3 00/03]: hwrng: an in-kernel rngd

I think the default should be zero, so each hwrng driver maintainer would have to consider what guarantees that particular driver can give. If anything 50% ought to be the maximum.

On April 14, 2014 9:02:11 AM PDT, Torsten Duwe <[email protected]> wrote:
>More or less a resend of v2.
>
>On Wed, Mar 26, 2014 at 06:03:37PM -0700, H. Peter Anvin wrote:
>> I'm wondering more about the default. We default to 50% for
>arch_get_random_seed, and this is supposed to be the default for in
>effect unverified hwrngs...
>
>Done. 50% is now the default, that's the only change from v2.
>
>Andy: the printk you pointed out already limits itself to 1/10s,
>which is half the default rate limit. Also, as Peter already
>wrote, we're dealing with true HWRNGs here; if such a device
>does not produce a single byte within 10 seconds something _is_
>severely broken and, like a dying disk, worth to be logged.
>Here's one of the better circuits I found:
>http://www.maximintegrated.com/app-notes/index.mvp/id/3469
>or offline:
>http://pdfserv.maximintegrated.com/en/an/AN3469.pdf
>Disclaimer: I'm not endorsing Maxim, it's just that paper
>that hits the spot IMHO.
>
>Anything wrong with feeding those bits into the input pool?
>Any other comments on the code?
>
> Torsten

--
Sent from my mobile phone. Please pardon brevity and lack of formatting.

2014-04-14 16:30:23

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v3 00/03]: hwrng: an in-kernel rngd

On 04/14/2014 09:24 AM, Torsten Duwe wrote:
> On Mon, Apr 14, 2014 at 09:09:14AM -0700, H. Peter Anvin wrote:
>> I think the default should be zero, so each hwrng driver maintainer would have to consider what guarantees that particular driver can give. If anything 50% ought to be the maximum.
>
> Repost of v4 just for patch 03/03 is on its way then, defaulting to 0
> (off), which happens to be the old behaviour, which I think I've also
> expressed my sympathy for already.
>

Sounds good.

-hpa

2014-04-14 16:27:14

by Torsten Duwe

[permalink] [raw]
Subject: [PATCH v4 03/03]: hwrng: khwrngd derating per device


This patch introduces a derating factor to struct hwrng for
the random bits going into the kernel input pool, and a common
default derating for drivers which do not specify one.

Signed-off-by: Torsten Duwe <[email protected]>

---
drivers/char/hw_random/core.c | 11 ++++++++++-
include/linux/hw_random.h | 3 +++
2 files changed, 13 insertions(+), 1 deletion(-)

--- linux/include/linux/hw_random.h.orig
+++ linux/include/linux/hw_random.h
@@ -29,6 +29,8 @@
* @read: New API. drivers can fill up to max bytes of data
* into the buffer. The buffer is aligned for any type.
* @priv: Private data, for use by the RNG driver.
+ * @derating: Estimation of true entropy in RNG's bitstream
+ * (per mill).
*/
struct hwrng {
const char *name;
@@ -38,6 +39,7 @@ struct hwrng {
int (*data_read)(struct hwrng *rng, u32 *data);
int (*read)(struct hwrng *rng, void *data, size_t max, bool wait);
unsigned long priv;
+ unsigned short derating;

/* internal. */
struct list_head list;
--- linux/drivers/char/hw_random/core.c.orig
+++ linux/drivers/char/hw_random/core.c
@@ -56,11 +56,15 @@ static LIST_HEAD(rng_list);
static DEFINE_MUTEX(rng_mutex);
static int data_avail;
static u8 *rng_buffer, *rng_fillbuf;
-static unsigned short derating_current = 700; /* an arbitrary 70% */
+static unsigned short derating_current;
+static unsigned short derating_default = 0; /* default to "off" */

module_param(derating_current, ushort, 0644);
MODULE_PARM_DESC(derating_current,
"current hwrng entropy estimation per mill");
+module_param(derating_default, ushort, 0644);
+MODULE_PARM_DESC(derating_default,
+ "default entropy content of hwrng per mill");

static void start_khwrngd(void);

@@ -79,6 +83,11 @@ static inline int hwrng_init(struct hwrn
if (err)
return err;

+ derating_current = rng->derating ? : derating_default;
+ derating_current &= 1023;
+
+ if (derating_current == 0 && hwrng_fill)
+ kthread_stop(hwrng_fill);
if (derating_current > 0 && !hwrng_fill)
start_khwrngd();

2014-04-14 16:41:34

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v3 03/03]: hwrng: khwrngd derating per device

On Mon, Apr 14, 2014 at 9:06 AM, Torsten Duwe <[email protected]> wrote:
>
> This patch introduces a derating factor to struct hwrng for
> the random bits going into the kernel input pool, and a common
> default derating for drivers which do not specify one.
>
> Signed-off-by: Torsten Duwe <[email protected]>
>
> ---
> drivers/char/hw_random/core.c | 11 ++++++++++-
> include/linux/hw_random.h | 3 +++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> --- linux/include/linux/hw_random.h.orig
> +++ linux/include/linux/hw_random.h
> @@ -29,6 +29,8 @@
> * @read: New API. drivers can fill up to max bytes of data
> * into the buffer. The buffer is aligned for any type.
> * @priv: Private data, for use by the RNG driver.
> + * @derating: Estimation of true entropy in RNG's bitstream
> + * (per mill).

I'll bikeshed again: this is a rating, not a *de*rating. Higher =
more confidence, at least assuming the comment is right.

--Andy

2014-04-14 16:44:14

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v3 00/03]: hwrng: an in-kernel rngd

On Mon, Apr 14, 2014 at 9:24 AM, Torsten Duwe <[email protected]> wrote:
> On Mon, Apr 14, 2014 at 09:09:14AM -0700, H. Peter Anvin wrote:
>> I think the default should be zero, so each hwrng driver maintainer would have to consider what guarantees that particular driver can give. If anything 50% ought to be the maximum.
>
> Repost of v4 just for patch 03/03 is on its way then, defaulting to 0
> (off), which happens to be the old behaviour, which I think I've also
> expressed my sympathy for already.
>

To be clear, I like your patches, bikeshedding aside. Having reads of
/dev/random automatically pull in hwrng data would be *great*. My
drbg patches are not meant to replace them at all. I just think that
there's a need for the nonblocking pool to be well-seeded as quickly
as possible, regardless of configuration, and I don't think your code
is meant to do that.

--Andy

2014-04-14 16:25:00

by Torsten Duwe

[permalink] [raw]
Subject: Re: [PATCH v3 00/03]: hwrng: an in-kernel rngd

On Mon, Apr 14, 2014 at 09:09:14AM -0700, H. Peter Anvin wrote:
> I think the default should be zero, so each hwrng driver maintainer would have to consider what guarantees that particular driver can give. If anything 50% ought to be the maximum.

Repost of v4 just for patch 03/03 is on its way then, defaulting to 0
(off), which happens to be the old behaviour, which I think I've also
expressed my sympathy for already.

Torsten

2014-04-15 08:51:35

by Torsten Duwe

[permalink] [raw]
Subject: Re: [PATCH v3 03/03]: hwrng: khwrngd derating per device

On Mon, Apr 14, 2014 at 09:41:10AM -0700, Andy Lutomirski wrote:
> On Mon, Apr 14, 2014 at 9:06 AM, Torsten Duwe <[email protected]> wrote:
> >
> > This patch introduces a derating factor to struct hwrng for
> > the random bits going into the kernel input pool, and a common
> > default derating for drivers which do not specify one.
> >
> > Signed-off-by: Torsten Duwe <[email protected]>
> >
> > ---
> > drivers/char/hw_random/core.c | 11 ++++++++++-
> > include/linux/hw_random.h | 3 +++
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > --- linux/include/linux/hw_random.h.orig
> > +++ linux/include/linux/hw_random.h
> > @@ -29,6 +29,8 @@
> > * @read: New API. drivers can fill up to max bytes of data
> > * into the buffer. The buffer is aligned for any type.
> > * @priv: Private data, for use by the RNG driver.
> > + * @derating: Estimation of true entropy in RNG's bitstream
> > + * (per mill).
>
> I'll bikeshed again: this is a rating, not a *de*rating. Higher =
> more confidence, at least assuming the comment is right.
>
You're right. Would anyone object to call it "quality", as in RX signal quality?
In context of a random source that is pretty accurate, I'd say. Other opinions?

Torsten

2014-04-15 16:54:17

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v3 03/03]: hwrng: khwrngd derating per device

On Tue, Apr 15, 2014 at 1:51 AM, Torsten Duwe <[email protected]> wrote:
> On Mon, Apr 14, 2014 at 09:41:10AM -0700, Andy Lutomirski wrote:
>> On Mon, Apr 14, 2014 at 9:06 AM, Torsten Duwe <[email protected]> wrote:
>> >
>> > This patch introduces a derating factor to struct hwrng for
>> > the random bits going into the kernel input pool, and a common
>> > default derating for drivers which do not specify one.
>> >
>> > Signed-off-by: Torsten Duwe <[email protected]>
>> >
>> > ---
>> > drivers/char/hw_random/core.c | 11 ++++++++++-
>> > include/linux/hw_random.h | 3 +++
>> > 2 files changed, 13 insertions(+), 1 deletion(-)
>> >
>> > --- linux/include/linux/hw_random.h.orig
>> > +++ linux/include/linux/hw_random.h
>> > @@ -29,6 +29,8 @@
>> > * @read: New API. drivers can fill up to max bytes of data
>> > * into the buffer. The buffer is aligned for any type.
>> > * @priv: Private data, for use by the RNG driver.
>> > + * @derating: Estimation of true entropy in RNG's bitstream
>> > + * (per mill).
>>
>> I'll bikeshed again: this is a rating, not a *de*rating. Higher =
>> more confidence, at least assuming the comment is right.
>>
> You're right. Would anyone object to call it "quality", as in RX signal quality?
> In context of a random source that is pretty accurate, I'd say. Other opinions?

I'm okay with "quality", although I'm still partial to "entropy_per_1000bits".

--Andy