Amit, Kees, On Wed, Jul 09, 2014 at 06:55:24PM +0530, Amit Shah wrote: > On (Wed) 09 Jul 2014 [09:17:37], Jason Cooper wrote: > > On Wed, Jul 09, 2014 at 06:38:22PM +0530, Amit Shah wrote: > > > On (Wed) 09 Jul 2014 [07:53:17], Jason Cooper wrote: > > > > On Sat, Jul 05, 2014 at 11:04:52AM +0530, Amit Shah wrote: > > > > > Commit d9e7972619334 "hwrng: add randomness to system from rng sources" > > > > > added a call to rng_get_data() from the hwrng_register() function. > > > > > However, some rng devices need initialization before data can be read > > > > > from them. > > > > > > > > > > This commit makes the call to rng_get_data() depend on no init fn > > > > > pointer being registered by the device. If an init function is > > > > > registered, this call is made after device init. > > > > > > > > > > CC: Kees Cook <keescook@xxxxxxxxxxxx> > > > > > CC: Jason Cooper <jason@xxxxxxxxxxxxxx> > > > > > CC: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> > > > > > CC: <stable@xxxxxxxxxxxxxxx> # For v3.15+ > > > > > Signed-off-by: Amit Shah <amit.shah@xxxxxxxxxx> > > > > > --- > > > > > drivers/char/hw_random/core.c | 31 +++++++++++++++++++++++++------ > > > > > 1 file changed, 25 insertions(+), 6 deletions(-) > > > > > > > > Thanks for cleaning this up! > > > > > > Thanks! > > > > > > Any thoughts on the follow-up patch posted later that resolves some of > > > the weirdness in init? > > > > hmm, I'd rather see an init function for virtio-rng that checks the bit > > and returns 0 or -EAGAIN. With your proposed change, you would get > > hangs again. > > Confused; I meant the patch in > > https://lkml.org/lkml/2014/7/7/58 Yes, I meant without 2/2 of this current series. I'm cooling to the idea of the init function for virtio-rng, and it might be best just to admit that there's no way to seed the entropy pool from the virtio-rng at probe time. After all, once userspace is up, the system should take advantage of /dev/hwrng for the generation of long-term keys. Either via rngd feeding /dev/random, or directly. As for the follow-on patch you asked about, I think that's fine. More entropy can't hurt. The below patch might be worth considering so that the user of a system with only virtio-rng can kick the entropy pool as they see fit. It's probably not too kosher as is, but if the idea is liked, I could clean it up and submit. The advantage is that users don't need to have rngd installed and running on the system in order to jump-start the entropy pool. thx, Jason. ------------>8---------------- diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index df95e2ff9d48..b54af066d075 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -63,14 +63,25 @@ static size_t rng_buffer_size(void) return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES; } -static void add_early_randomness(struct hwrng *rng) +static void add_early_randomness(struct hwrng *rng, u8 size) { - unsigned char bytes[16]; + unsigned char *bytes; int bytes_read; + /* size can come from the user, sanitize */ + size = size > 256 ? 256 : size; + + size = size == 0 ? 16 : size; + + bytes = kmalloc(size, GFP_KERNEL); + if (!bytes) + return; + bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1); if (bytes_read > 0) add_device_randomness(bytes, bytes_read); + + kfree(bytes); } static inline int hwrng_init(struct hwrng *rng) @@ -84,7 +95,7 @@ static inline int hwrng_init(struct hwrng *rng) if (ret) return ret; - add_early_randomness(rng); + add_early_randomness(rng, 0); return ret; } @@ -281,18 +292,54 @@ static ssize_t hwrng_attr_available_show(struct device *dev, return ret; } +/* + * seed the kernel's entropy pool from the current hwrng. + * + * 'echo "n" >rng_seed_kernel', where n >= 0. + * n = 0: default size added (16 bytes) + * 0 < n <= 256: n bytes added. + * n > 256: 256 bytes added. + */ +static ssize_t hwrng_attr_seed_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + int err; + u8 sz; + + if (!current_rng) + return -ENODEV; + + err = kstrtou8(buf, 10, &sz); + if (err) + return err; + + err = mutex_lock_interruptible(&rng_mutex); + if (err) + return -ERESTARTSYS; + + add_early_randomness(current_rng, sz); + + mutex_unlock(&rng_mutex); + + return err ? : len; +} + static DEVICE_ATTR(rng_current, S_IRUGO | S_IWUSR, hwrng_attr_current_show, hwrng_attr_current_store); static DEVICE_ATTR(rng_available, S_IRUGO, hwrng_attr_available_show, NULL); - +static DEVICE_ATTR(rng_seed_kernel, S_IWUSR, + NULL, + hwrng_attr_seed_store); static void unregister_miscdev(void) { device_remove_file(rng_miscdev.this_device, &dev_attr_rng_available); device_remove_file(rng_miscdev.this_device, &dev_attr_rng_current); + device_remove_file(rng_miscdev.this_device, &dev_attr_rng_seed_kernel); misc_deregister(&rng_miscdev); } @@ -311,9 +358,15 @@ static int register_miscdev(void) &dev_attr_rng_available); if (err) goto err_remove_current; + err = device_create_file(rng_miscdev.this_device, + &dev_attr_rng_seed_kernel); + if (err) + goto err_remove_available; out: return err; +err_remove_available: + device_remove_file(rng_miscdev.this_device, &dev_attr_rng_available); err_remove_current: device_remove_file(rng_miscdev.this_device, &dev_attr_rng_current); err_misc_dereg: @@ -367,7 +420,7 @@ int hwrng_register(struct hwrng *rng) list_add_tail(&rng->list, &rng_list); if (!rng->init) - add_early_randomness(rng); + add_early_randomness(rng, 0); out_unlock: mutex_unlock(&rng_mutex); -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html