Kees, Ted, On Mon, Mar 03, 2014 at 03:51:48PM -0800, Kees Cook wrote: > 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. Why is this necessary? init_std_data() already calls arch_get_random_long() while stirring each of the pools. I'm a little concerned here because this gives potentially untrusted hwrngs more influence over the entropy pools initial state than most users of random.c expect. Many of the drivers in hw_random/ are platform drivers and are initialized before random.c. I'm comfortable with the design decisions Ted has made wrt random.c and hwrngs. However, I think that this changes that trust relationship in a fundamental way. I'm ok with building support into my kernels for hwrngs as long as random.c's internal use of them is limited to the mixing in extract_buf() and init_std_data(). By adding this patch, even without crediting entropy to the pool, a rogue hwrng now has significantly more influence over the initial state of the entropy pools. Or, am I missing something? thx, Jason. > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > --- > drivers/char/hw_random/core.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c > index a0f7724852eb..6e5bb68a708c 100644 > --- a/drivers/char/hw_random/core.c > +++ b/drivers/char/hw_random/core.c > @@ -41,6 +41,7 @@ > #include <linux/miscdevice.h> > #include <linux/delay.h> > #include <linux/slab.h> > +#include <linux/random.h> > #include <asm/uaccess.h> > > > @@ -305,6 +306,8 @@ int hwrng_register(struct hwrng *rng) > int must_register_misc; > int err = -EINVAL; > struct hwrng *old_rng, *tmp; > + unsigned char bytes[16]; > + int bytes_read; > > if (rng->name == NULL || > (rng->data_read == NULL && rng->read == NULL)) > @@ -348,6 +351,10 @@ int hwrng_register(struct hwrng *rng) > } > INIT_LIST_HEAD(&rng->list); > list_add_tail(&rng->list, &rng_list); > + > + bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1); > + if (bytes_read > 0) > + add_device_randomness(bytes, bytes_read); > out_unlock: > mutex_unlock(&rng_mutex); > out: > -- > 1.7.9.5 > > > -- > Kees Cook > Chrome OS Security > -- > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
