Skip to contentSkip to conversation

: khwrngd

PATCH Superseded Torsten Duwe 6 participants 41 messages Mar 21, 2014 – Jul 11, 2014 2 messages not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover : khwrngd

Here is version 2 of the khwrngd patch set.

The first patch is unchanged.

The second one now introduces an initial derating factor, as suggested by hpa. It's called derating_current to simplify patch#3, and the thread creation has moved into hwrng_init, because it may later depend on the hwrng's derating property.

The third patch only introduces the derating member to struct hwrng and provides a configurable default.

I could imagine to further add a derating_extra parameter for conservative admins, in order to diminish the entropy estimation given from the driver author even more. OTOH too many knobs might cause confusion.

Torsten

#1 lore raw

PATCH v2 1/3 : 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.

---
 include/linux/hw_random.h |    2 ++
 drivers/char/random.c     |   23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)
include/linux/hw_random.h +2 -0
@@ -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_ */
drivers/char/random.c +23 -0
@@ -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);
+
#2 lore raw

PATCH v2 2/3 : hwrng: create filler thread

In reply to Torsten Duwe

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.

---
 drivers/char/hw_random/core.c |   65 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 3 deletions(-)
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +62 -3
@@ -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);
 }
 
#3 lore raw

Re: [PATCH v2 02/03]: hwrng: create filler thread

Quoted 4 lines · Torsten Duwe

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.

Nice.

[...]

drivers/char/hw_random/core.c line 63

Quoted 13 lines

static struct hwrng *current_rng;

drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +7 -1
+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");

As an electrical engineer (sort of), I can't read this without thinking you're talking about the amount by which the current is derated. For example, a 14-50 electrical outlet is rated to 50 Amps. If you use it continuously for a long time, though, the current is derated to 40 Amps.

Shouldn't this be called credit_derating or, even better, credit_per_1000bits?

Also, "per mill" is just obscure enough that someone might think it means "per million".

Quoted 11 lines
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +4 -0
+
+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;
+

drivers/char/hw_random/core.c static size_t rng_buffer_size(void) · line 84

Quoted 10 lines
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +7 -1
 	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();
+

Why the check for derating > 0? Paranoid users may want zero credit, but they probably still want the thing to run.

Quoted 9 lines
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +2 -0
+	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)

drivers/char/hw_random/core.c err_misc_dereg: · line 331

Quoted 10 lines
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +10 -0
+{
+	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");

ratelimit (heavily), please.

Also, would it make sense to round-robin all hwrngs? Even better:
collect entropy from each one and add them to the pool all at once. If so, would it make sense for the derating to be a per-rng parameter. For example, if there's a sysfs class, it could go in there.

Finally, there may be hwrngs like TPMs that are amazingly slow. What happens if the RNG is so slow that it becomes the bottleneck? Should this thing back off? Using the TPM at 100% utilization seems silly when there's a heavy entropy consumer, especially since reading 256 bits from the TPM once is probably just about as secure as reading from it continuously.

Also, with my quantum hat on, thanks for doing this in a way that isn't gratuitously insecure against quantum attack. 128-bit reseeds are simply too small if your adversary has a large quantum computer :)

--Andy

#4 lore raw

Re: [PATCH v2 02/03]: hwrng: create filler thread

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...

Quoted 107 lines · Andy Lutomirski
Quoted 4 lines · Torsten Duwe

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.

Nice.

[...]

drivers/char/hw_random/core.c line 63

Quoted 13 lines

static struct hwrng *current_rng;

drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +7 -1
+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");

As an electrical engineer (sort of), I can't read this without thinking you're talking about the amount by which the current is derated. For example, a 14-50 electrical outlet is rated to 50 Amps. If you use it continuously for a long time, though, the current is derated to 40 Amps.

Shouldn't this be called credit_derating or, even better, credit_per_1000bits?

Also, "per mill" is just obscure enough that someone might think it means "per million".

Quoted 11 lines
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +4 -0
+
+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;
+

drivers/char/hw_random/core.c static size_t rng_buffer_size(void) · line 84

Quoted 10 lines
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +7 -1
 	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();
+

Why the check for derating > 0? Paranoid users may want zero credit, but they probably still want the thing to run.

Quoted 9 lines
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +2 -0
+	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)

drivers/char/hw_random/core.c err_misc_dereg: · line 331

Quoted 10 lines
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +10 -0
+{
+	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");

ratelimit (heavily), please.

Also, would it make sense to round-robin all hwrngs? Even better:
collect entropy from each one and add them to the pool all at once. If so, would it make sense for the derating to be a per-rng parameter. For
example, if there's a sysfs class, it could go in there.

Finally, there may be hwrngs like TPMs that are amazingly slow. What happens if the RNG is so slow that it becomes the bottleneck? Should this thing back off? Using the TPM at 100% utilization seems silly when
there's a heavy entropy consumer, especially since reading 256 bits from
the TPM once is probably just about as secure as reading from it continuously.

Also, with my quantum hat on, thanks for doing this in a way that isn't gratuitously insecure against quantum attack. 128-bit reseeds are simply too small if your adversary has a large quantum computer :)

--Andy

Signature · 1 line
Sent from my mobile phone.  Please pardon brevity and lack of formatting.
#5 lore raw

Re: [PATCH v2 02/03]: hwrng: create filler thread

[cc: Greg Price, might be working on this stuff]

Quoted 1 line · H. Peter Anvin

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...

TBH I'm highly skeptical of this kind of entropy estimation. /dev/random is IMO just silly, since you need to have very conservative entropy estimates for the concept to really work, and that ends up being hideously slow. Also, in the /dev/random sense, most hardware RNGs have no entropy at all, since they're likely to be FIPS-approved DRBGs that don't have a real non-deterministic source.

For the kernel's RNG to be secure, I think it should have the property that it still works if you rescale all the entropy estimates by any constant that's decently close to 1.

If entropy estimates are systematically too low, then a naive implementation results in an excessively long window during early bootup in which /dev/urandom is completely insecure.

If entropy estimates are systematically too high, then a naive implementation fails to do a catastrophic reseed, and the RNG can be brute-forced.

So I think that the core code should do something along the lines of using progressively larger reseeds. Since I think that /dev/random is silly, this means that we only really care about the extent to which "entropy" measures entropy conditioned on whatever an attacker can actually compute. Since this could vary widely between devices (e.g. if your TPM is malicious), I think that the best we can do is to collect ~256 bits from everything available, shove it all in to the core together, and repeat. For all I know, the core code already does this.

The upshot is that the actual rescaling factor should barely matter. 50% is probably fine. So is 100% and 25%. 10% is probably asking for trouble during early boot if all you have is a TPM.

--Andy

#6 lore raw

Re: [PATCH v2 02/03]: hwrng: create filler thread

There are a number of things wrong with this post, but I'll respond in detail when I get to a keyboard.

Quoted 41 lines · Andy Lutomirski

[cc: Greg Price, might be working on this stuff]

Quoted 1 line · H. Peter Anvin

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...

TBH I'm highly skeptical of this kind of entropy estimation. /dev/random is IMO just silly, since you need to have very conservative entropy estimates for the concept to really work, and that ends up being hideously slow. Also, in the /dev/random sense, most hardware RNGs have no entropy at all, since they're likely to be FIPS-approved DRBGs that don't have a real non-deterministic source.

For the kernel's RNG to be secure, I think it should have the property that it still works if you rescale all the entropy estimates by any constant that's decently close to 1.

If entropy estimates are systematically too low, then a naive implementation results in an excessively long window during early bootup in which /dev/urandom is completely insecure.

If entropy estimates are systematically too high, then a naive implementation fails to do a catastrophic reseed, and the RNG can be brute-forced.

So I think that the core code should do something along the lines of using progressively larger reseeds. Since I think that /dev/random is silly, this means that we only really care about the extent to which "entropy" measures entropy conditioned on whatever an attacker can actually compute. Since this could vary widely between devices (e.g. if your TPM is malicious), I think that the best we can do is to collect ~256 bits from everything available, shove it all in to the core together, and repeat. For all I know, the core code already does this.

The upshot is that the actual rescaling factor should barely matter. 50% is probably fine. So is 100% and 25%. 10% is probably asking for trouble during early boot if all you have is a TPM.

--Andy

Signature · 1 line
Sent from my mobile phone.  Please pardon brevity and lack of formatting.
#7 lore raw

Re: [PATCH v2 02/03]: hwrng: create filler thread

In reply to Andy Lutomirski
Quoted 5 lines · Andy Lutomirski

TBH I'm highly skeptical of this kind of entropy estimation. /dev/random is IMO just silly, since you need to have very conservative entropy estimates for the concept to really work, and that ends up being hideously slow.

In the absence of a hardware entropy source, it is, but for long-lived keys, delay is better than bad key generation.

A major reason for entropy estimation is to control the amount of backpressure. If you don't have backpressure, you only have generation pressure, and you can't put your system to sleep when the hwrng keeps outputting data. Worse, if your entropy source is inexhaustible, you might end up spending all your CPU time processing its output.

Quoted 3 lines

Also, in the /dev/random sense,
most hardware RNGs have no entropy at all, since they're likely to be FIPS-approved DRBGs that don't have a real non-deterministic source.

Such a device has no business being a Linux hwrng device. We already have a PRNG (DRBG) in the kernel, the *only* purpose for a hwrng device is to be an entropy source.

Quoted 3 lines

For the kernel's RNG to be secure, I think it should have the property that it still works if you rescale all the entropy estimates by any constant that's decently close to 1.

That is correct.

Quoted 3 lines

If entropy estimates are systematically too low, then a naive implementation results in an excessively long window during early bootup in which /dev/urandom is completely insecure.

Eh? What mechanism would make /dev/urandom any less secure due to entropy underestimation? The whole *point* is that we should systematically underestimate entropy -- and we do, according to research papers which have analyzed the state of things we do by orders of magnitude, which is the only possible way to do it for non-hwrng sources.

Quoted 3 lines

If entropy estimates are systematically too high, then a naive implementation fails to do a catastrophic reseed, and the RNG can be brute-forced.

This again is unacceptable. We really should not overestimate.

Quoted 10 lines

So I think that the core code should do something along the lines of using progressively larger reseeds. Since I think that /dev/random is silly, this means that we only really care about the extent to which "entropy" measures entropy conditioned on whatever an attacker can actually compute. Since this could vary widely between devices (e.g. if your TPM is malicious), I think that the best we can do is to collect ~256 bits from everything available, shove it all in to the core together, and repeat. For all I know, the core code already does this.

Quoted 3 lines

The upshot is that the actual rescaling factor should barely matter. 50% is probably fine. So is 100% and 25%. 10% is probably asking for trouble during early boot if all you have is a TPM.

I don't see why small factors should be a problem at all (except that it discourages /dev/random usage.) Keep in mind we still add the entropy -- we just don't credit its existence.

TPMs, in particular, should almost certainly be massively derated based on what little we know about TPM.

As a concrete example: RDRAND is a hardware entropy source that is architecturally allowed to be diluted by a DRBG up to 512 times. As far as I know of the hardware, no shipping piece of hardware is anywhere near 512 in this aspect. rngd currently does 512:1 data reduction, but injecting the raw output at 1/512 credit ought to give a much better result in terms of entropy.

-hpa

#8 lore raw

Re: [PATCH v2 02/03]: hwrng: create filler thread

Quoted 8 lines · H. Peter Anvin
Quoted 3 lines · Andy Lutomirski

Also, in the /dev/random sense,
most hardware RNGs have no entropy at all, since they're likely to be FIPS-approved DRBGs that don't have a real non-deterministic source.

Such a device has no business being a Linux hwrng device. We already have a PRNG (DRBG) in the kernel, the *only* purpose for a hwrng device is to be an entropy source.

That's also my understanding. The shot noise from a Schottky-diode seems to be quite popular, for example.

Quoted 2 lines

TPMs, in particular, should almost certainly be massively derated based on what little we know about TPM.

Backdoors, anyone? :-)

Torsten

#9 lore raw

Re: [PATCH v2 02/03]: hwrng: create filler thread

In reply to H. Peter Anvin
Quoted 7 lines · H. Peter Anvin
Quoted 5 lines · Andy Lutomirski

TBH I'm highly skeptical of this kind of entropy estimation. /dev/random is IMO just silly, since you need to have very conservative entropy estimates for the concept to really work, and that ends up being hideously slow.

Quoted 8 lines

In the absence of a hardware entropy source, it is, but for long-lived keys, delay is better than bad key generation.

A major reason for entropy estimation is to control the amount of backpressure. If you don't have backpressure, you only have generation pressure, and you can't put your system to sleep when the hwrng keeps outputting data. Worse, if your entropy source is inexhaustible, you might end up spending all your CPU time processing its output.

Fair enough. I'll shut up about /dev/random (except for the fact that I think that the reseed logic should be considered very carefully). Please consider the rest of my comments as being specific to urandom. :)

Quoted 8 lines
Quoted 4 lines

Also, in the /dev/random sense,
most hardware RNGs have no entropy at all, since they're likely to be FIPS-approved DRBGs that don't have a real non-deterministic source.

Such a device has no business being a Linux hwrng device. We already have a PRNG (DRBG) in the kernel, the *only* purpose for a hwrng device is to be an entropy source.

See the very end.

Quoted 11 lines
Quoted 4 lines

For the kernel's RNG to be secure, I think it should have the property that it still works if you rescale all the entropy estimates by any constant that's decently close to 1.

That is correct.

Quoted 3 lines

If entropy estimates are systematically too low, then a naive implementation results in an excessively long window during early bootup in which /dev/urandom is completely insecure.

Quoted 5 lines

Eh? What mechanism would make /dev/urandom any less secure due to entropy underestimation? The whole *point* is that we should systematically underestimate entropy -- and we do, according to research papers which have analyzed the state of things we do by orders of magnitude, which is the only possible way to do it for non-hwrng sources.

Lack of an initial reseed. If the core code decides that it's only received three bits of entropy and shouldn't reseed, then the system might go for a very long time with no entropy at all making it all the way to urandom. I don't know what the current code does, but it's changed quite a few times recently.

Quoted 6 lines
Quoted 4 lines

If entropy estimates are systematically too high, then a naive implementation fails to do a catastrophic reseed, and the RNG can be brute-forced.

This again is unacceptable. We really should not overestimate.

I think we shouldn't overestimate, but I think that we should also have an implementation that's robust against overestimating by a moderate factor.

Quoted 11 lines
Quoted 11 lines

So I think that the core code should do something along the lines of using progressively larger reseeds. Since I think that /dev/random is silly, this means that we only really care about the extent to which "entropy" measures entropy conditioned on whatever an attacker can actually compute. Since this could vary widely between devices (e.g. if your TPM is malicious), I think that the best we can do is to collect ~256 bits from everything available, shove it all in to the core together, and repeat. For all I know, the core code already does this.

Quoted 8 lines
Quoted 3 lines

The upshot is that the actual rescaling factor should barely matter. 50% is probably fine. So is 100% and 25%. 10% is probably asking for trouble during early boot if all you have is a TPM.

I don't see why small factors should be a problem at all (except that it discourages /dev/random usage.) Keep in mind we still add the entropy -- we just don't credit its existence.

This is only true if the entropy actually makes it to /dev/urandom. If the input pool credited entropy is too small, then account, and hence extract_entropy, will return zero when called on the input pool, xfer_secondary_pool on the urandom pool won't do anything, so urandom won't be reseeded at all.

Damn it, this is crypto code. It should not be this hard to understand what the code is doing.

Quoted 9 lines

TPMs, in particular, should almost certainly be massively derated based on what little we know about TPM.

As a concrete example: RDRAND is a hardware entropy source that is architecturally allowed to be diluted by a DRBG up to 512 times. As far as I know of the hardware, no shipping piece of hardware is anywhere near 512 in this aspect. rngd currently does 512:1 data reduction, but injecting the raw output at 1/512 credit ought to give a much better result in terms of entropy.

Hmm. Maybe the core random code should have a separate way to inject cryptographic entropy-less bits. I agree that the TPM has no business providing any credit at all to /dev/random, but I think that it would be a huge improvement to use the TPM at least on startup to seed urandom. It's there and, however weak it may be, it's a lot better than not seeding urandom at all.

This could be as simple as add_drbg_randomness. It doesn't need to go through hwrng.

rdrand is weird and I have no real problem with sticking it in to /dev/random with a suitable derating. It's a DRBG, but it's also seeded with a read RNG, whereas I suspect that most TPMs have no real entropy source, or at least no entropy source fast enough to be useful if the TPM's crypto is bad.

--Andy

#10 lore raw

Re: [PATCH v2 02/03]: hwrng: create filler thread

In reply to H. Peter Anvin
Quoted 1 line · H. Peter Anvin

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...

If the default were 0, it would be exactly the old behaviour. How about that? Plus, driver authors would have to come up with an estimate on their own.

Quoted 11 lines · Andy Lutomirski
Quoted 10 lines

drivers/char/hw_random/core.c line 63

Quoted 1 line
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +1 -0
+		 "current hwrng entropy estimation per mill");

As an electrical engineer (sort of), I can't read this without thinking you're talking about the amount by which the current is derated. For example, a 14-50 electrical outlet is rated to 50 Amps. If you use it continuously for a long time, though, the current is derated to 40 Amps.

Shouldn't this be called credit_derating or, even better, credit_per_1000bits?

That's an awkward name for a parameter.

Quoted 2 lines
Quoted 2 lines

Also, "per mill" is just obscure enough that someone might think it means "per million".

No. I looked it up, as we have the precise term "Promille" in German. Also in electrical engineering, (imperial :-) PCB design, a mil is one 1000th of an inch. Per million would surely be named PPM.

Quoted 2 lines
Quoted 2 lines

Why the check for derating > 0? Paranoid users may want zero credit, but they probably still want the thing to run.

[...]

Quoted 1 line
Quoted 1 line

ratelimit (heavily), please.

The kthread will stop once the estimated entropy is above the threshold. derating=0 will wind up one CPU core to 100%. So it's an elegant way to disable the whole mechanism.

Quoted 3 lines
Quoted 3 lines

Also, would it make sense to round-robin all hwrngs? Even better:
collect entropy from each one and add them to the pool all at once. If so, would it make sense for the derating to be a per-rng parameter.

Finally, the derating _is_ a per-RNG parameter. I also thought about mixing already, but first I want to see a machine with more than 1 HWRNG :-)

Torsten

#11 lore raw

Re: [PATCH v2 02/03]: hwrng: create filler thread

Quoted 16 lines · Torsten Duwe
Quoted 1 line · H. Peter Anvin

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...

If the default were 0, it would be exactly the old behaviour. How about that? Plus, driver authors would have to come up with an estimate on their own.

Quoted 9 lines · Andy Lutomirski
Quoted 8 lines
Quoted 1 line

+ "current hwrng entropy estimation per mill");

As an electrical engineer (sort of), I can't read this without thinking you're talking about the amount by which the current is derated. For example, a 14-50 electrical outlet is rated to 50 Amps. If you use it continuously for a long time, though, the current is derated to 40 Amps.

Quoted 4 lines
Quoted 2 lines
Quoted 2 lines

Shouldn't this be called credit_derating or, even better, credit_per_1000bits?

That's an awkward name for a parameter.

I don't think it's worse than credit_derating.

Quoted 8 lines
Quoted 3 lines
Quoted 3 lines

Also, "per mill" is just obscure enough that someone might think it means "per million".

No. I looked it up, as we have the precise term "Promille" in German. Also in electrical engineering, (imperial :-) PCB design, a mil is one 1000th of an inch. Per million would surely be named PPM.

I'm not saying that "per mill" is wrong -- I'm just saying it's obscure and may confuse people.

Quoted 11 lines
Quoted 2 lines
Quoted 2 lines

Why the check for derating > 0? Paranoid users may want zero credit, but they probably still want the thing to run.

[...]

Quoted 1 line
Quoted 1 line

ratelimit (heavily), please.

The kthread will stop once the estimated entropy is above the threshold. derating=0 will wind up one CPU core to 100%. So it's an elegant way to disable the whole mechanism.

Sorry, I didn't mean ratelimit the loop. I meant ratelimit the printk.

Quoted 7 lines
Quoted 3 lines
Quoted 3 lines

Also, would it make sense to round-robin all hwrngs? Even better:
collect entropy from each one and add them to the pool all at once. If so, would it make sense for the derating to be a per-rng parameter.

Finally, the derating _is_ a per-RNG parameter. I also thought about mixing already, but first I want to see a machine with more than 1 HWRNG :-)

Any Haswell machine with another hwrng will have two of them. (I'm not sure that the rdrand/rdseed thing registers as an hwrng, but still...)

--Andy

#12 lore raw

v3 cover : hwrng: an in-kernel rngd

In reply to H. Peter Anvin

More or less a resend of v2.

Quoted 1 line · H. Peter Anvin

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

#13 lore raw

PATCH v3 1/3 : 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.

---
 include/linux/hw_random.h |    2 ++
 drivers/char/random.c     |   23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)
include/linux/hw_random.h +2 -0
@@ -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_ */
drivers/char/random.c +23 -0
@@ -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);
+
#14 lore raw

PATCH v3 2/3 : hwrng: create filler thread

In reply to Torsten Duwe

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.

---
 drivers/char/hw_random/core.c |   65 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 3 deletions(-)
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +62 -3
@@ -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);
 }
 
#15 lore raw

PATCH v3 3/3 : hwrng: khwrngd derating per device

In reply to Torsten Duwe

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.

---
 drivers/char/hw_random/core.c |   11 ++++++++++-
 include/linux/hw_random.h     |    3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)
include/linux/hw_random.h renamed from include/linux/hw_random.h.orig +3 -0
@@ -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;
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +10 -1
@@ -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();
 
#16 lore raw

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

Quoted 15 lines · Torsten Duwe

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.

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

include/linux/hw_random.h line 33

Quoted 5 lines
include/linux/hw_random.h renamed from include/linux/hw_random.h.orig +2 -0
@@ -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

#17 lore raw

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

Quoted 13 lines · Andy Lutomirski
Quoted 12 lines · Torsten Duwe

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.

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

include/linux/hw_random.h line 33

Quoted 8 lines
include/linux/hw_random.h renamed from include/linux/hw_random.h.orig +2 -0
@@ -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

#18 lore raw

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

Quoted 26 lines · Torsten Duwe
Quoted 13 lines · Andy Lutomirski
Quoted 12 lines · Torsten Duwe

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.

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

include/linux/hw_random.h line 33

Quoted 8 lines
include/linux/hw_random.h renamed from include/linux/hw_random.h.orig +2 -0
@@ -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.

Quoted 2 lines

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

#19 lore raw

v5 cover : hwrng: an in-kernel rngd

Quoted 5 lines · Andy Lutomirski
Quoted 2 lines · Torsten Duwe

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".

So, since there were no other comments, here's v5 with aforementioned change. Default is still "off". Patch 02/03 de-fuzzed for 3.15.

Torsten

#20 lore raw

PATCH v2 1/3 : 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.

---
 include/linux/hw_random.h |    2 ++
 drivers/char/random.c     |   23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)
include/linux/hw_random.h +2 -0
@@ -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_ */
drivers/char/random.c +23 -0
@@ -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);
+
#21 lore raw

PATCH v5 2/3 : hwrng: create filler thread

In reply to Torsten Duwe

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.

---
 drivers/char/hw_random/core.c |   65 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 3 deletions(-)
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +62 -3
@@ -39,6 +39,7 @@
 #include <linux/fs.h>
 #include <linux/sched.h>
 #include <linux/miscdevice.h>
+#include <linux/kthread.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/random.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 current_quality = 700; /* an arbitrary 70% */
+
+module_param(current_quality, ushort, 0644);
+MODULE_PARM_DESC(current_quality,
+		 "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 (current_quality > 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*current_quality)>>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 err = -EINVAL;
@@ -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);
 }
 
#22 lore raw

PATCH v5 3/3 : hwrng: khwrngd derating per device

In reply to Torsten Duwe

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.

---
 drivers/char/hw_random/core.c |   11 ++++++++++-
 include/linux/hw_random.h     |    3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)
include/linux/hw_random.h renamed from include/linux/hw_random.h.orig +3 -0
@@ -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.
+ * @quality:		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 quality;
 
 	/* internal. */
 	struct list_head list;
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +10 -1
@@ -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 current_quality = 700; /* an arbitrary 70% */
+static unsigned short current_quality;
+static unsigned short default_quality = 0; /* default to "off" */
 
 module_param(current_quality, ushort, 0644);
 MODULE_PARM_DESC(current_quality,
 		 "current hwrng entropy estimation per mill");
+module_param(default_quality, ushort, 0644);
+MODULE_PARM_DESC(default_quality,
+		 "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;
 
+	current_quality = rng->quality ? : default_quality;
+	current_quality &= 1023;
+
+	if (current_quality == 0 && hwrng_fill)
+		kthread_stop(hwrng_fill);
 	if (current_quality > 0 && !hwrng_fill)
 		start_khwrngd();
 
#23 lore raw

PATCH v2 3/3 : hwrng: khwrngd derating per device

[checkpatch tells me not to 0-init...]

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.

---
 drivers/char/hw_random/core.c |   11 ++++++++++-
 include/linux/hw_random.h     |    3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)
include/linux/hw_random.h renamed from include/linux/hw_random.h.orig +3 -0
@@ -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.
+ * @quality:		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 quality;
 
 	/* internal. */
 	struct list_head list;
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +10 -1
@@ -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 current_quality = 700; /* an arbitrary 70% */
+static unsigned short current_quality;
+static unsigned short default_quality; /* = 0; default to "off" */
 
 module_param(current_quality, ushort, 0644);
 MODULE_PARM_DESC(current_quality,
 		 "current hwrng entropy estimation per mill");
+module_param(default_quality, ushort, 0644);
+MODULE_PARM_DESC(default_quality,
+		 "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;
 
+	current_quality = rng->quality ? : default_quality;
+	current_quality &= 1023;
+
+	if (current_quality == 0 && hwrng_fill)
+		kthread_stop(hwrng_fill);
 	if (current_quality > 0 && !hwrng_fill)
 		start_khwrngd();
 
#24 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Quoted 8 lines · Torsten Duwe

[checkpatch tells me not to 0-init...]

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.

Did we lose track of this patchset?

-hpa

#25 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Quoted 11 lines · H. Peter Anvin
Quoted 8 lines · Torsten Duwe

[checkpatch tells me not to 0-init...]

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.

Did we lose track of this patchset?

Yes. I was already considering a resend.

Torsten

maintainer
#26 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Quoted 4 lines · Torsten Duwe
Quoted 2 lines

Did we lose track of this patchset?

Yes. I was already considering a resend.

I've looked it over, and I'm fairly OK with it at this point. Do folks mind if I just run it through the random tree?

I want to add a tracepoint for better debugging, but I can take care of that after it's in the random tree.

- Ted

#27 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Makes sense to me.

Feel free to add my

Quoted 13 lines · Theodore Ts'o
Quoted 4 lines · Torsten Duwe
Quoted 2 lines

Did we lose track of this patchset?

Yes. I was already considering a resend.

I've looked it over, and I'm fairly OK with it at this point. Do folks mind if I just run it through the random tree?

I want to add a tracepoint for better debugging, but I can take care of that after it's in the random tree.

- Ted

Signature · 1 line
Sent from my mobile phone.  Please pardon brevity and lack of formatting.
maintainer
#28 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

OK, I've merged these changes into the random.git tree.

I had to make a few minor changes.

1) Changes so it would compile on 3.15. (random_write_wakeup_thresh got renamed to random_write_wakeup_bits). I'm guessing the patch was massaged so that it would apply, but it was never compile tested.

2) Fixed a bug in patch #2 so that it would work correctly if the rng driver doesn't have an init function (which happens to be the case for the tpm-rng driver, which I used for my testing).

There are also a few minor rough edges that I've noted, but not yet fixed. The main one is that if you've compiled the hw_random's rng_core into the kernel, changes to /sys/modules/rng_core/parameters/* won't actually cause the hwrngd kerenl thread to get started. You have to set the parameters before you load the rng module in order for them to be activated. And if you've compiled the rng module into the kernel, that trick won't work.

Fixing this probably means that we need to set up a formal sysfs tree under /sys/kernel/hw_random.

- Ted

#29 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Quoted 7 lines · Theodore Ts'o

OK, I've merged these changes into the random.git tree.

I had to make a few minor changes.

1) Changes so it would compile on 3.15. (random_write_wakeup_thresh got renamed to random_write_wakeup_bits). I'm guessing the patch was massaged so that it would apply, but it was never compile tested.

I'm keeping and updating 2 versions, one -current (more or less) and one for 3.12. I probably missed that when making the discussion changes back and forth, sorry.

Quoted 3 lines

2) Fixed a bug in patch #2 so that it would work correctly if the rng driver doesn't have an init function (which happens to be the case for the tpm-rng driver, which I used for my testing).

The whole thing stems from entropy-challenged s390. 3.12 on s390 compiles and runs fine. Yields a solid 200 kB/s

TPM RNG is a crook ;-)

Quoted 7 lines

There are also a few minor rough edges that I've noted, but not yet fixed. The main one is that if you've compiled the hw_random's rng_core into the kernel, changes to /sys/modules/rng_core/parameters/* won't actually cause the hwrngd kerenl thread to get started. You have to set the parameters before you load the rng module in order for them to be activated. And if you've compiled the rng module into the kernel, that trick won't work.

With patch 03/03, it is up to the driver author to specify an entropy quality, which can be overridden at boot time, or when loading the module, respectively. This should be a constant hardware property. It would be nice to change it at runtime; but frankly I hope that this won't be neccessary.

Quoted 2 lines

Fixing this probably means that we need to set up a formal sysfs tree under /sys/kernel/hw_random.

Maybe along with more sophisticated steering of how many bits to pick from which source, if multiple are available.

Thanks,

Torsten

maintainer
#30 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Quoted 8 lines · Torsten Duwe
Quoted 3 lines

2) Fixed a bug in patch #2 so that it would work correctly if the rng driver doesn't have an init function (which happens to be the case for the tpm-rng driver, which I used for my testing).

The whole thing stems from entropy-challenged s390. 3.12 on s390 compiles and runs fine. Yields a solid 200 kB/s

TPM RNG is a crook ;-)

I think the word you mean is "crock" (as in "crock of sh*t"?) :-)

Were you referring to the typical hardware implementation in most TPM's, or something else?

If you're testing on s390, I'd appreciate it if you could give the "dev" branch on random.git a quick try. It has some improvements to help deal with "entropy challenged" platforms by using the register values to be mixed in with everything else. This was based on some suggestions and work by Jörn Engel.

Quoted 1 line

From my quick testing on x86 platforms, it doubles the overhead of

add_interrupt_randomness() on platforms that don't have a cycle counter. It still should be fairly low overhead, but it would be great if some folks on other architectures did some testing to confirm that the resulting overhead isn't problematic. (Jörn wanted to mix in all of the registers, but from my calculations, even if we only did this once a clock tick, the overhead and resulting long tail latency in interrupt processing time was not something I was willing to inflict on everyone.)

Also note if you are playing with the random.git tree, there is a fairly serious bug fix on the "for_linus_stable" branch that will be pushed to Linus once it gets a bit more testing.

Quoted 5 lines

With patch 03/03, it is up to the driver author to specify an entropy quality, which can be overridden at boot time, or when loading the module, respectively. This should be a constant hardware property. It would be nice to change it at runtime; but frankly I hope that this won't be neccessary.

The question of what should be the proper derating mechanism is going to be subject to individual administrators. I agree that we should have good defaults, but for example, I'm sure the manufacturer of the TPM that's in my Thinkpad would try to claim that it's the Bug Free(tm), and try to assign it derating factor accordingly. If the manufacturer is supplying the device driver, it may not be a value that other people will agree with. Which is why I think making it runtime configurable is a good thing.

As another example, I assume Peter or someone else from Intel will be shortly submitting a hw_random driver for RDRAND on x86. What should the derating factor be for that? I suspect David Johnson's answer would be quite different from other people's. And that's to be expected, since he has much better information that most of us have access to about the RDRAND implementation, and the likelihood/possibiliy it could have been subverted.

Quoted 5 lines
Quoted 2 lines

Fixing this probably means that we need to set up a formal sysfs tree under /sys/kernel/hw_random.

Maybe along with more sophisticated steering of how many bits to pick from which source, if multiple are available.

Yeah, the question about what to do we have multiple hw random sources is something that I thought about. Do we want to poll from more than one?

Also, suppose some hw random sources require more resources --- battery life in particular, for mobile/laptop devices? How do we deal with policy questions such as these? Should we deal with it all, or just assume that userspace will dynamically enable or disable pulling from certain devices based on policy questions such as power utilization issues?

Cheers,

- Ted

#31 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Quoted 11 lines · Theodore Ts'o
Quoted 8 lines · Torsten Duwe
Quoted 3 lines

2) Fixed a bug in patch #2 so that it would work correctly if the rng driver doesn't have an init function (which happens to be the case for the tpm-rng driver, which I used for my testing).

The whole thing stems from entropy-challenged s390. 3.12 on s390 compiles and runs fine. Yields a solid 200 kB/s

TPM RNG is a crook ;-)

I think the word you mean is "crock" (as in "crock of sh*t"?) :-)

Actually, I was thinking of a crutch. Makes you walk slowly, but better than nothing. Seems I've bent the wrong tube.

Quoted 2 lines

Were you referring to the typical hardware implementation in most TPM's, or something else?

Those are designed for the TPM's own, internal use IIRC. Their exposure to the main computer is only a side effect.

Quoted 11 lines
Quoted 5 lines

With patch 03/03, it is up to the driver author to specify an entropy quality, which can be overridden at boot time, or when loading the module, respectively. This should be a constant hardware property. It would be nice to change it at runtime; but frankly I hope that this won't be neccessary.

The question of what should be the proper derating mechanism is going to be subject to individual administrators. I agree that we should have good defaults, but for example, I'm sure the manufacturer of the TPM that's in my Thinkpad would try to claim that it's the Bug Free(tm), and try to assign it derating factor accordingly. If the

Then the next question would be about the underlying specification. A bug free implementation of dual-EC DRBG?

Quoted 3 lines

manufacturer is supplying the device driver, it may not be a value that other people will agree with. Which is why I think making it runtime configurable is a good thing.

Boot time configurable, I'd say. Again: this is a hardware property, multiplied by the admin's level of confidence in the absence of backdoors. It's easy with s390: from z/VM you can read all the guest's memory anyway. If you use this machine, you already trust IBM.

Quoted 7 lines

As another example, I assume Peter or someone else from Intel will be shortly submitting a hw_random driver for RDRAND on x86. What should the derating factor be for that? I suspect David Johnson's answer would be quite different from other people's. And that's to be expected, since he has much better information that most of us have access to about the RDRAND implementation, and the likelihood/possibiliy it could have been subverted.

So let's keep it close to 0, and allow those to raise it who have confidence.

Quoted 6 lines
Quoted 2 lines

Maybe along with more sophisticated steering of how many bits to pick from which source, if multiple are available.

Yeah, the question about what to do we have multiple hw random sources is something that I thought about. Do we want to poll from more than one?

Of course! Choose your mix!

Quoted 6 lines

Also, suppose some hw random sources require more resources --- battery life in particular, for mobile/laptop devices? How do we deal with policy questions such as these? Should we deal with it all, or just assume that userspace will dynamically enable or disable pulling from certain devices based on policy questions such as power utilization issues?

One thing after the other. What are the consumers of kernel entropy? Mostly ASLR, I guess, and the web server / sshd accepting connections. Those proceses starting probably eats more power than a HWRNG needs for the appropriate random bits. We can address exceptions once they arise.

Torsten

maintainer
#32 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Quoted 6 lines · Torsten Duwe
Quoted 3 lines
Quoted 1 line

TPM RNG is a crook ;-)

I think the word you mean is "crock" (as in "crock of sh*t"?) :-)

Actually, I was thinking of a crutch. Makes you walk slowly, but better than nothing. Seems I've bent the wrong tube.

Heh. One of the things that I have considered, especially for TPM, is that in addition to having a very small quality rating, we should also have some kind of delay so that we sleep some small amount time before we pull from the TPM again.

Otherwise the result of using a very small quality rating is that we end up pounding on the TPM a huge amount until the entropy pool is above the write_wakeup threshold. If there's some "real" use of the TPM, such as authenticating to a wireless network, or some such, I'd rather not be constantly pounding on the TPM if so happens that there is a heavy drain on /dev/random at the same time that Network Manager needs to reauthenticate to the 802.1x network.

Quoted 8 lines
Quoted 3 lines

manufacturer is supplying the device driver, it may not be a value that other people will agree with. Which is why I think making it runtime configurable is a good thing.

Boot time configurable, I'd say. Again: this is a hardware property, multiplied by the admin's level of confidence in the absence of backdoors. It's easy with s390: from z/VM you can read all the guest's memory anyway. If you use this machine, you already trust IBM.

Sure, but I guess I'm a bit allergic to gazillions of boot command-line parameters. I guess if you are building a modular kernel, this matters a lot less, since you can put the configs in /etc/modprobe.d.

- Ted

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#35 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Hi @all,

First, sorry for the late feedback, I was busy with other topics.

In the meanwhile I had a look at this hwrng entropy feature related to s390 and the zcrypt hardware random generator.
I have a concern about the initial call to the hwrng device while trying to register the same.

drivers/char/hw_random/core.c
...
int hwrng_register(struct hwrng *rng) {
...

   bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1);
        if (bytes_read > 0)
                add_device_randomness(bytes, bytes_read);

...
}

At this point in time the zcrypt hw device is not registered completely in the internal zcrypt device list (ap_device_list). The crypto card itself is initialized and ready to receive and service requests, but the tasklet that retrieve the card response is not able to find the device because it's not yet in the list. Finally the response would not be received and the thread is still waiting for completion, hence the probing/registering procedure for all the other crypto devices is blocked. No further devices could be scanned, initialized and registered.
Therefore I would not recommend to trigger a hw device request before the device register process has completed.

Anyway, when the kernel thread (hwrng_fillfn) starts, it take care about the initial call to the zcrypt rng device to be triggered. Therefore I don't see any reason to manually call rng_get_data() during the registration or did I missed something?

Mit freundlichen Grüßen / Kind regards

Ingo Tuchscherer

Software Development - Linux on System z
IBM Systems &Technology Group

Phone: +49-7031-16-1986 IBM Deutschland (Embedded

                                                                                           image moved
                                                                                              to file:
                                                                                         pic60502.gif)

E-Mail: [email protected] Schoenaicher Str. 220

                                                    71032 Boeblingen   
                                                                       
                                                    Germany            

IBM Deutschland
Research &
Development
GmbH /
Vorsitzender des
Aufsichtsrats:
Martina Koederitz

Geschäftsführung:
Dirk Wittkopp
Sitz der
Gesellschaft:
Böblingen /
Registergericht:
Amtsgericht
Stuttgart, HRB
243294

From: "Theodore Ts'o" <[email protected]>
To: Torsten Duwe <[email protected]>,
Cc: Ingo Tuchscherer/Germany/IBM@IBMDE, Hans-Georg
Markgraf/Germany/IBM@IBMDE, [email protected], [email protected], [email protected], Jörn Engel <[email protected]>
Date: 16.06.2014 16:28
Subject: Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Quoted 2 lines · Torsten Duwe

Also, I added the patch below to our kernel. Who wants to take that one-liner upstream?

This will require sucking in the "master" branch of random.git. So either the s390 tree can merge in the master branch, or I can carry it in the random.git tree. I'm happy to do that unless their are any objections.

(Note: if you want to base something on random.git, please use the master branch. The dev branch is sometimes rewound... I've since moved master up to the dev branch, which is my guarantee not to rewind anything up to the master branch.)

Quoted 1 line

--- linux-3.12-SLE12/drivers/s390/crypto/zcrypt_api.c.orig

2014-06-16 14:23:27.726162185 +0200
Quoted 1 line

+++ linux-3.12-SLE12/drivers/s390/crypto/zcrypt_api.c

2014-06-16 14:46:31.869103924 +0200

Review static int zcrypt_rng_data_read(struct h · line 1380

Quoted 8 lines
(quoted diff) +1 -0
@@ -1374,6 +1374,7 @@ static int zcrypt_rng_data_read(struct h
 static struct hwrng zcrypt_rng_dev = {
 		 .name		 		 = "zcrypt",
 		 .data_read		 = zcrypt_rng_data_read,
+		 .quality		 = 990,
 };
 
 static int zcrypt_rng_device_add(void)

I assume the z-Series RNG is presumed to be high quality, yes?

-
Ted

maintainer Reviewed-by Jason Cooper
#36 lore raw

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

Quoted 10 lines · Ingo Tuchscherer

At this point in time the zcrypt hw device is not registered completely in the internal zcrypt device list (ap_device_list). The crypto card itself is initialized and ready to receive and service requests, but the tasklet that retrieve the card response is not able to find the device because it's not yet in the list. Finally the response would not be received and the thread is still waiting for completion, hence the probing/registering procedure for all the other crypto devices is blocked. No further devices could be scanned, initialized and registered.
Therefore I would not recommend to trigger a hw device request before the device register process has completed.

Is there any chance that you could change the zcrypt driver so that it doesn't call hwrng_register() until and the tasklet that can receive the request has been fully registered? It sounds like that should just be a matter of reordering a few lines of code, no?

Quoted 4 lines

Anyway, when the kernel thread (hwrng_fillfn) starts, it take care about the initial call to the zcrypt rng device to be triggered. Therefore I don't see any reason to manually call rng_get_data() during the registration or did I missed something?

The basic idea was described in the commit that added this:

commit d9e79726193346569af7953369a638ee2275ade5

Author: Kees Cook <[email protected]>
Date:   Mon Mar 3 15:51:48 2014 -0800

hwrng: add randomness to system from rng sources

When bringing a new RNG source online, it seems like it would make sense to use some of its bytes to make the system entropy pool more random, as done with all sorts of other devices that contain per-device or per-boot differences.

Cheers,

- Ted

#37 lore raw

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

In reply to Torsten Duwe

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.

Quoted 25 lines · Torsten Duwe

More or less a resend of v2.

Quoted 1 line · H. Peter Anvin

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

Signature · 1 line
Sent from my mobile phone.  Please pardon brevity and lack of formatting.
#38 lore raw

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

Quoted 1 line · H. Peter Anvin

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

#39 lore raw

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

Quoted 7 lines · Torsten Duwe
Quoted 1 line · H. Peter Anvin

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

#40 lore raw

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

In reply to Torsten Duwe
Quoted 7 lines · Torsten Duwe
Quoted 1 line · H. Peter Anvin

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

#41 lore raw

PATCH v4 3/3 : hwrng: khwrngd derating per device

In reply to H. Peter Anvin

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.

---
 drivers/char/hw_random/core.c |   11 ++++++++++-
 include/linux/hw_random.h     |    3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)
include/linux/hw_random.h renamed from include/linux/hw_random.h.orig +3 -0
@@ -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;
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +10 -1
@@ -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();
 
#42 lore raw

PATCH v2 3/3 : hwrng: khwrngd derating per device

In reply to Torsten Duwe

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.

---
 drivers/char/hw_random/core.c |   11 ++++++++++-
 include/linux/hw_random.h     |    3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)
include/linux/hw_random.h renamed from include/linux/hw_random.h.orig +3 -0
@@ -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;
drivers/char/hw_random/core.c renamed from drivers/char/hw_random/core.c.orig +10 -1
@@ -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 = 700; /* an arbitrary 70% */
 
 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();