Virtualization - Jul 2014 - [RFC PATCH 1/3] hw_random: allow RNG devices to give early randomness after a delay

Hello,

This series introduces a way to allow devices to contribute to initial
system randomness after a certain delay.  Specifically, the virtio-rng
device can contribute initial randomness only after a successful
probe().

A delayed workqueue item is queued in the system queue to fetch this
randomness if the device indicates it's capable of contributing only
after a delay, via the new HWRNG_DELAY_READ_AT_INIT flag.

This series is intended for the next merge window.  I've marked it as
RFC because the last revert doesn't yet have a linux.git sha1.  I'll
re-post when the previous series gets committed.

Please review,

Amit Shah (3):
  hw_random: allow RNG devices to give early randomness after a delay
  virtio: rng: only accept delayed early randomness requests
  Revert "virtio: rng: ensure reads happen after successful probe"

 drivers/char/hw_random/core.c       | 26 +++++++++++++++++++-------
 drivers/char/hw_random/virtio-rng.c | 11 +----------
 include/linux/hw_random.h           |  8 ++++++++
 3 files changed, 28 insertions(+), 17 deletions(-)

-- 
1.9.3

Some RNG devices may not be ready to give early randomness at probe()
time, and hence lose out on the opportunity to contribute to system
randomness at boot- or device hotplug- time.

This commit schedules a delayed work item for such devices, and fetches
early randomness after a delay.  Currently the delay is 500ms, which is
enough for the lone device that needs such treatment: virtio-rng.

CC: Kees Cook <keescook at chromium.org>
CC: Jason Cooper <jason at lakedaemon.net>
CC: Herbert Xu <herbert at gondor.apana.org.au>
Signed-off-by: Amit Shah <amit.shah at redhat.com>
---
 drivers/char/hw_random/core.c | 20 +++++++++++++++++++-
 include/linux/hw_random.h     |  8 ++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index c4419ea..2a765fd 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -63,7 +63,7 @@ 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 get_early_randomness(struct hwrng *rng)
 {
 	unsigned char bytes[16];
 	int bytes_read;
@@ -79,6 +79,21 @@ static void add_early_randomness(struct hwrng *rng)
 		add_device_randomness(bytes, bytes_read);
 }
 
+static void sched_init_random(struct work_struct *work)
+{
+	struct hwrng *rng = container_of(work, struct hwrng, dwork.work);
+
+	get_early_randomness(rng);
+}
+
+static void add_early_randomness(struct hwrng *rng)
+{
+	if (!(rng->flags & HWRNG_DELAY_READ_AT_INIT))
+		return get_early_randomness(rng);
+
+	schedule_delayed_work(&rng->dwork, msecs_to_jiffies(500));
+}
+
 static inline int hwrng_init(struct hwrng *rng)
 {
 	if (rng->init) {
@@ -351,6 +366,7 @@ int hwrng_register(struct hwrng *rng)
 			goto out_unlock;
 	}
 
+	INIT_DELAYED_WORK(&rng->dwork, sched_init_random);
 	old_rng = current_rng;
 	if (!old_rng) {
 		err = hwrng_init(rng);
@@ -362,6 +378,7 @@ int hwrng_register(struct hwrng *rng)
 	if (!old_rng) {
 		err = register_miscdev();
 		if (err) {
+			cancel_delayed_work_sync(&rng->dwork);
 			hwrng_cleanup(rng);
 			current_rng = NULL;
 			goto out_unlock;
@@ -395,6 +412,7 @@ void hwrng_unregister(struct hwrng *rng)
 	mutex_lock(&rng_mutex);
 
 	list_del(&rng->list);
+	cancel_delayed_work_sync(&rng->dwork);
 	if (current_rng == rng) {
 		hwrng_cleanup(rng);
 		if (list_empty(&rng_list)) {
diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h
index b4b0eef..8f7370d 100644
--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -14,6 +14,11 @@
 
 #include <linux/types.h>
 #include <linux/list.h>
+#include <linux/workqueue.h>
+
+#define HWRNG_DELAY_READ_AT_INIT BIT(0)	/* Schedule delayed work to fetch
+					 * initial randomness instead of doing
+					 * it at ->init()-time */
 
 /**
  * struct hwrng - Hardware Random Number Generator driver
@@ -28,6 +33,7 @@
  *			Must not be NULL.    *OBSOLETE*
  * @read:		New API. drivers can fill up to max bytes of data
  *			into the buffer. The buffer is aligned for any type.
+ * @flags:		Per-device flags.
  * @priv:		Private data, for use by the RNG driver.
  */
 struct hwrng {
@@ -37,9 +43,11 @@ struct hwrng {
 	int (*data_present)(struct hwrng *rng, int wait);
 	int (*data_read)(struct hwrng *rng, u32 *data);
 	int (*read)(struct hwrng *rng, void *data, size_t max, bool wait);
+	unsigned int flags;
 	unsigned long priv;
 
 	/* internal. */
+	struct delayed_work dwork;
 	struct list_head list;
 };
 
-- 
1.9.3

hw_random core can ask for initial randomness after a slight delay after
probe() finishes, and we can contribute to system randomness at that
point.  Tell the hw_random core by setting the HWRNG_DELAY_READ_AT_INIT
flag.

CC: Kees Cook <keescook at chromium.org>
CC: Jason Cooper <jason at lakedaemon.net>
CC: Herbert Xu <herbert at gondor.apana.org.au>
Signed-off-by: Amit Shah <amit.shah at redhat.com>
---
 drivers/char/hw_random/virtio-rng.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/char/hw_random/virtio-rng.c
b/drivers/char/hw_random/virtio-rng.c
index d8ffebd..f53f1e8 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -123,6 +123,7 @@ static int probe_common(struct virtio_device *vdev)
 		.cleanup = virtio_cleanup,
 		.priv = (unsigned long)vi,
 		.name = vi->name,
+		.flags = HWRNG_DELAY_READ_AT_INIT,
 	};
 	vdev->priv = vi;
 
-- 
1.9.3

This reverts commit ceb5d72a2e27e95bc9570ce259c45b35f0e23462.

This commit was added for -stable so systems with virtio-rng don't
freeze at boot-time.

With the addition of the previous commits that delay the request for
initial randomness after probe() is successful, this is no longer
needed.

CC: Kees Cook <keescook at chromium.org>
CC: Jason Cooper <jason at lakedaemon.net>
CC: Herbert Xu <herbert at gondor.apana.org.au>
Signed-off-by: Amit Shah <amit.shah at redhat.com>
---
 drivers/char/hw_random/core.c       |  6 ------
 drivers/char/hw_random/virtio-rng.c | 10 ----------
 2 files changed, 16 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 2a765fd..148d9be 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -68,12 +68,6 @@ static void get_early_randomness(struct hwrng *rng)
 	unsigned char bytes[16];
 	int bytes_read;
 
-	/*
-	 * Currently only virtio-rng cannot return data during device
-	 * probe, and that's handled in virtio-rng.c itself.  If there
-	 * are more such devices, this call to rng_get_data can be
-	 * made conditional here instead of doing it per-device.
-	 */
 	bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1);
 	if (bytes_read > 0)
 		add_device_randomness(bytes, bytes_read);
diff --git a/drivers/char/hw_random/virtio-rng.c
b/drivers/char/hw_random/virtio-rng.c
index f53f1e8..f7d1573 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -37,8 +37,6 @@ struct virtrng_info {
 	int index;
 };
 
-static bool probe_done;
-
 static void random_recv_done(struct virtqueue *vq)
 {
 	struct virtrng_info *vi = vq->vdev->priv;
@@ -68,13 +66,6 @@ static int virtio_read(struct hwrng *rng, void *buf, size_t
size, bool wait)
 	int ret;
 	struct virtrng_info *vi = (struct virtrng_info *)rng->priv;
 
-	/*
-	 * Don't ask host for data till we're setup.  This call can
-	 * happen during hwrng_register(), after commit d9e7972619.
-	 */
-	if (unlikely(!probe_done))
-		return 0;
-
 	if (!vi->busy) {
 		vi->busy = true;
 		init_completion(&vi->have_data);
@@ -146,7 +137,6 @@ static int probe_common(struct virtio_device *vdev)
 		return err;
 	}
 
-	probe_done = true;
 	return 0;
 }
 
-- 
1.9.3

On Mon, Jul 14, 2014 at 10:05:19AM +0530, Amit Shah
wrote:
> Some RNG devices may not be ready to give early randomness at probe()
> time, and hence lose out on the opportunity to contribute to system
> randomness at boot- or device hotplug- time.
> 
> This commit schedules a delayed work item for such devices, and fetches
> early randomness after a delay.  Currently the delay is 500ms, which is
> enough for the lone device that needs such treatment: virtio-rng.
> 
> CC: Kees Cook <keescook at chromium.org>
> CC: Jason Cooper <jason at lakedaemon.net>
> CC: Herbert Xu <herbert at gondor.apana.org.au>
> Signed-off-by: Amit Shah <amit.shah at redhat.com>
> ---
>  drivers/char/hw_random/core.c | 20 +++++++++++++++++++-
>  include/linux/hw_random.h     |  8 ++++++++
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index c4419ea..2a765fd 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -63,7 +63,7 @@ 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 get_early_randomness(struct hwrng *rng)
>  {
>  	unsigned char bytes[16];
>  	int bytes_read;
> @@ -79,6 +79,21 @@ static void add_early_randomness(struct hwrng *rng)
>  		add_device_randomness(bytes, bytes_read);
>  }
>  
> +static void sched_init_random(struct work_struct *work)
> +{
> +	struct hwrng *rng = container_of(work, struct hwrng, dwork.work);
> +
> +	get_early_randomness(rng);
> +}
> +
> +static void add_early_randomness(struct hwrng *rng)
The add/get naming seems awkward in the above hunks.
> +{
> +	if (!(rng->flags & HWRNG_DELAY_READ_AT_INIT))
> +		return get_early_randomness(rng);
> +
> +	schedule_delayed_work(&rng->dwork, msecs_to_jiffies(500));
> +}
> +
Perhaps instead of rng->flags and a hardcoded delay, we could have
rng->seed_delay = msecs_to_jiffies(500) in virtio-rng?  Then you can
just call unconditionally:

	schedule_delayed_work(&rng->dwork, rng->seed_delay);

I think that would be a more extensible solution should other drivers
show up with the same issue.

thx,

Jason.