Re: virtio-rng only returns zeros with CONFIG_HW_RANDOM=m

Rusty Russell
2013-02-28T03:21:22Z

Aurelien Jarno <aurelien@aurel32.net> writes:
> On Wed, Feb 27, 2013 at 11:56:55AM +1030, Rusty Russell wrote:
>> Aurelien Jarno <aurelien@aurel32.net> writes:
>> > Hi,
>> >
>> > I have noticed that virtio-rng only returns zero for kernels >= 2.6.33
>> > built with CONFIG_HW_RANDOM=m. This is a bit much too predictable for a
>> > random generator ;-).
>> >
>> > The reason for that is virtio expects guest real addresses, while
>> > rng_core.ko (ie when built as a module) is passing a vmalloced buffer
>> > to the virtio-rng read function, declared as such:
>> >
>> > static u8 rng_buffer[SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES]
>> > __cacheline_aligned;
>> >
>> > This is basically the same issue than the following one:
>> >
>> > https://lists.linux-foundation.org/pipermail/virtualization/2008-May/010946.html
>> >
>> > but introduced in a more subtle way in this commit:
>> >
>> > commit bb347d98079a547e80bd4722dee1de61e4dca0e8
>> > Author: Ian Molton <ian.molton@collabora.co.uk>
>> > Date: Tue Dec 1 15:26:33 2009 +0800
>>
>> OK, I looked at doing a kmalloc and copy in virtio_rng, but it's very
>> inelegant (we don't know what size of buffer to allocate).
>
> On the other hand, the rng API allows to return less bytes than
> requested, so it's possible to have a fixed buffer size of for example
> 64 or 128 bytes. But I agree it's better to do that in rng core.

That's true, too.

I'd really like Ian's feedback, since he was the one who made the
change, but the previous email address bounced. Trying again...

hw_random: make buffer usable in scatterlist.

virtio_rng feeds the randomness buffer handed by the core directly
into the scatterlist, since commit bb347d98079a547e80bd4722dee1de61e4dca0e8.

However, if CONFIG_HW_RANDOM=m, the static buffer isn't a linear address
(at least on most archs). We could fix this in virtio_rng, but it's actually
far easier to just do it in the core as virtio_rng would have to allocate
a buffer every time (it doesn't know how much the core will want to read).

Reported-by: Aurelien Jarno <aurelien@aurel32.net>
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 1bafb40..69ae597 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -40,6 +40,7 @@
#include <linux/init.h>
#include <linux/miscdevice.h>
#include <linux/delay.h>
+#include <linux/slab.h>
#include <asm/uaccess.h>

@@ -52,8 +53,12 @@ static struct hwrng *current_rng;
static LIST_HEAD(rng_list);
static DEFINE_MUTEX(rng_mutex);
static int data_avail;
-static u8 rng_buffer[SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES]
- __cacheline_aligned;
+static u8 *rng_buffer;
+
+static size_t rng_buffer_size(void)
+{
+ return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES;
+}

static inline int hwrng_init(struct hwrng *rng)
{
@@ -116,7 +121,7 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,

if (!data_avail) {
bytes_read = rng_get_data(current_rng, rng_buffer,
- sizeof(rng_buffer),
+ rng_buffer_size(),
!(filp->f_flags & O_NONBLOCK));
if (bytes_read < 0) {
err = bytes_read;
@@ -307,6 +312,14 @@ int hwrng_register(struct hwrng *rng)

mutex_lock(&rng_mutex);

+ /* kmalloc makes this safe for virt_to_page() in virtio_rng.c */
+ err = -ENOMEM;
+ if (!rng_buffer) {
+ rng_buffer = kmalloc(rng_buffer_size(), GFP_KERNEL);
+ if (!rng_buffer)
+ goto out_unlock;
+ }
+
/* Must not register two RNGs with the same name. */
err = -EEXIST;
list_for_each_entry(tmp, &rng_list, list) {