LinuxLists.cc - [PATCH 0/2] [RFC] virtio-rng entropy leak reporting feature

2023-01-19 18:55:39

Subject: [PATCH 0/2] [RFC] virtio-rng entropy leak reporting feature

Recently, a proposal has been published [1] for a new feature in the
VirtIO RNG device which will allows the device to report "entropy leaks"
to the guest VM. Such an event occurs when, for example, we take a VM
snapshot, or when we restore a VM from a snapshot.

The feature allows the guest to request for certain operations to be
performed upon an entropy leak event. When such an event occurs, the
device will handle the requests and add the request buffers to the used
queue. Adding these buffers to the used queue operates as a notification
towards the guest about the entropy leak event.

The proposed changes describe two types of requests that can be
performed: (1) fill a buffer in guest memory with random bytes and (2)
perform a memory copy between two buffers in guest memory.

The mechanism provides similar functionality to Microsoft's Virtual
Machine Generation ID and it can be used to re-seed the kernel's PRNG
upon taking a VM snapshot or resuming from one. Additionally, it allows
to (1) avoid the race-condition that exists with our VMGENID
implementation, between the time a VM is resumed after a "leak event"
and the handling of the ACPI notification before adding the new entropy.
Finally, it allows building on top of it to provide a mechanism for
notifying user-space about such events.

The first patch of this series, extends the current virtio-rng driver to
implement the new feature and ensures that there is always a request to
get some random bytes from the device in the event of an entropy leak
and uses these bytes as entropy through the `add_device_randomness`.

The second patch adds a copy-on-leak command as well in the queue,
implementating the idea of a generation counter that has previously been
part of the VMGENID saga. It then exposes the value of the generation
counter over a sysfs file. User-space can read, mmap and poll on the
file in order to be notified about entropy leak events.

I have performed basic tests of the user-space interfaces using a
Firecracker where I implemented virtio-rng with the proposed features.
Instructions on how to replicate this can be found here:
https://github.com/bchalios/virtio-snapsafe-example

The patchset does not solve all problems. We do not define an API for
other parts of the kernel to be able to use directly the new
functionality (add commands to the queue), mainly because I 'm not sure
what would the correct API be. I was toying with the idea of extending
`struct hwrng` with two new hooks that would be implemented only by
virtio-rng but I'm not sure I like it, so I am open to suggestions.

As a result of the above, the way we use the functionality to add new
entropy, i.e. calling `add_device_randomness`, is as racy as the VMGENID
case, since it relies on used buffers been handled by the virtio driver.

As for user-space, the `mmap` interface *is* race-free. Changes in the
generation counter will be observable by user applications the moment VM
vcpus resume. However, the `poll` interface isn't, `sysfs_notify` is
being called as well when the virtio driver handles used buffers. I am
not sure I have a solution for this last one.

Posting this, I hope we can resume the discussion about solving the
above issues (or any other issue that I haven't thought of), especially
with regards to providing a mechanism suitable for user-space
notifications.

Cheers,
Babis

Babis Chalios (2):
virtio-rng: implement entropy leak feature
virtio-rng: add sysfs entries for leak detection

drivers/char/hw_random/virtio-rng.c | 360 +++++++++++++++++++++++++++-
include/uapi/linux/virtio_rng.h | 3 +
2 files changed, 357 insertions(+), 6 deletions(-)

--
2.38.1

Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja M-401234 . CIF B84570936

2023-01-19 18:55:40

by Babis Chalios

[permalink] [raw]

Subject: [PATCH 1/2] virtio-rng: implement entropy leak feature

Implement the virtio-rng feature that allows a guest driver to request
from the device to perform certain operations in the event of an
"entropy leak", such as when taking a VM snapshot or restoring a VM from
a snapshot. The guest can request one of two operations: (i) fill a
buffer with random bytes, or (ii) perform a memory copy between two
bytes.

The feature is similar to Microsoft's Virtual Machine Generation ID and
it can be used to (1) avoid the race-condition that exists in our
current VMGENID implementation, between the time vcpus are resumed and
the ACPI notification is being handled and (2) provide mechanisms for
notifying user-space about snapshot-related events.

This commit implements the protocol between guest and device.
Additionally, it makes sure there is always a request for random bytes
in the event of entropy leak in-flight. Once such an event is observed,
the driver feeds these bytes to as entropy using
`add_device_randomness`.

Keep in mind that this commit does not solve the race-condition issue,
it adds fresh entropy whenever the driver handles the used buffer from
the fill-on-leak request. In order to close the race window, we need to
expose some API so that other kernel subsystems can request directly
notifications from the device.

Signed-off-by: Babis Chalios <[email protected]>
---
drivers/char/hw_random/virtio-rng.c | 200 +++++++++++++++++++++++++++-
include/uapi/linux/virtio_rng.h | 3 +
2 files changed, 197 insertions(+), 6 deletions(-)

diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index a6f3a8a2aca6..389a091a8801 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -4,12 +4,14 @@
* Copyright (C) 2007, 2008 Rusty Russell IBM Corporation
*/

+#include "asm-generic/errno.h"
#include <linux/err.h>
#include <linux/hw_random.h>
#include <linux/scatterlist.h>
#include <linux/spinlock.h>
#include <linux/virtio.h>
#include <linux/virtio_rng.h>
+#include <linux/random.h>
#include <linux/module.h>
#include <linux/slab.h>

@@ -18,6 +20,12 @@ static DEFINE_IDA(rng_index_ida);
struct virtrng_info {
struct hwrng hwrng;
struct virtqueue *vq;
+ /* Leak queues */
+ bool has_leakqs;
+ struct virtqueue *leakq[2];
+ spinlock_t lock;
+ int active_leakq;
+
char name[25];
int index;
bool hwrng_register_done;
@@ -29,27 +37,159 @@ struct virtrng_info {
/* minimal size returned by rng_buffer_size() */
#if SMP_CACHE_BYTES < 32
u8 data[32];
+ u8 leak_data[32];
#else
u8 data[SMP_CACHE_BYTES];
+ u8 leak_data[SMP_CACHE_BYTES];
#endif
};

+/* Swaps the queues and returns the new active leak queue. */
+static struct virtqueue *swap_leakqs(struct virtrng_info *vi)
+{
+ vi->active_leakq = 1 - vi->active_leakq;
+ return vi->leakq[vi->active_leakq];
+}
+
+static struct virtqueue *get_active_leakq(struct virtrng_info *vi)
+{
+ return vi->leakq[vi->active_leakq];
+}
+
+int add_fill_on_leak_request(struct virtrng_info *vi, struct virtqueue *vq, void *data, size_t len)
+{
+ struct scatterlist sg;
+ int ret;
+
+ sg_init_one(&sg, data, len);
+ ret = virtqueue_add_inbuf(vq, &sg, 1, data, GFP_KERNEL);
+ if (ret)
+ goto err;
+
+err:
+ return ret;
+}
+
+int virtrng_fill_on_leak(struct virtrng_info *vi, void *data, size_t len)
+{
+ struct virtqueue *vq;
+ unsigned long flags;
+ int ret;
+
+ if (!vi->has_leakqs)
+ return -EOPNOTSUPP;
+
+ spin_lock_irqsave(&vi->lock, flags);
+
+ vq = get_active_leakq(vi);
+ ret = add_fill_on_leak_request(vi, vq, data, len);
+ if (ret)
+ virtqueue_kick(vq);
+
+ spin_unlock_irqrestore(&vi->lock, flags);
+
+ return ret;
+}
+
+int add_copy_on_leak_request(struct virtrng_info *vi, struct virtqueue *vq,
+ void *to, void *from, size_t len)
+{
+ int ret;
+ struct scatterlist out, in, *sgs[2];
+
+ sg_init_one(&out, from, len);
+ sgs[0] = &out;
+ sg_init_one(&in, to, len);
+ sgs[1] = &in;
+
+ ret = virtqueue_add_sgs(vq, sgs, 1, 1, to, GFP_KERNEL);
+ if (ret)
+ goto err;
+
+err:
+ return ret;
+}
+
+int virtrng_copy_on_leak(struct virtrng_info *vi, void *to, void *from, size_t len)
+{
+ struct virtqueue *vq;
+ unsigned long flags;
+ int ret;
+
+ if (!vi->has_leakqs)
+ return -EOPNOTSUPP;
+
+ spin_lock_irqsave(&vi->lock, flags);
+
+ vq = get_active_leakq(vi);
+ ret = add_copy_on_leak_request(vi, vq, to, from, len);
+ if (ret)
+ virtqueue_kick(vq);
+
+ spin_unlock_irqrestore(&vi->lock, flags);
+
+ return ret;
+}
+
+static void entropy_leak_detected(struct virtqueue *vq)
+{
+ struct virtrng_info *vi = vq->vdev->priv;
+ struct virtqueue *activeq;
+ unsigned int len;
+ unsigned long flags;
+ void *buffer;
+ bool kick_activeq = false;
+
+ spin_lock_irqsave(&vi->lock, flags);
+
+ activeq = get_active_leakq(vi);
+ /* Drain all the used buffers from the queue */
+ while ((buffer = virtqueue_get_buf(vq, &len)) != NULL) {
+ if (vq == activeq) {
+ pr_debug("%s: entropy leak detected!", vi->name);
+ activeq = swap_leakqs(vi);
+ }
+
+ if (buffer == vi->leak_data) {
+ add_device_randomness(vi->leak_data, sizeof(vi->leak_data));
+
+ /* Ensure we always have a pending request for random bytes on entropy
+ * leak. Do it here, after we have swapped leak queues, so it gets handled
+ * with the next entropy leak event.
+ */
+ add_fill_on_leak_request(vi, activeq, vi->leak_data, sizeof(vi->leak_data));
+ kick_activeq = true;
+ }
+ }
+
+ if (kick_activeq)
+ virtqueue_kick(activeq);
+
+ spin_unlock_irqrestore(&vi->lock, flags);
+}
+
static void random_recv_done(struct virtqueue *vq)
{
struct virtrng_info *vi = vq->vdev->priv;
+ unsigned long flags;

+ spin_lock_irqsave(&vi->lock, flags);
/* We can get spurious callbacks, e.g. shared IRQs + virtio_pci. */
if (!virtqueue_get_buf(vi->vq, &vi->data_avail))
- return;
+ goto unlock;

vi->data_idx = 0;

complete(&vi->have_data);
+
+unlock:
+ spin_unlock_irqrestore(&vi->lock, flags);
}

static void request_entropy(struct virtrng_info *vi)
{
struct scatterlist sg;
+ unsigned long flags;

reinit_completion(&vi->have_data);
vi->data_avail = 0;
@@ -57,10 +197,12 @@ static void request_entropy(struct virtrng_info *vi)

sg_init_one(&sg, vi->data, sizeof(vi->data));

+ spin_lock_irqsave(&vi->lock, flags);
/* There should always be room for one buffer. */
virtqueue_add_inbuf(vi->vq, &sg, 1, vi->data, GFP_KERNEL);

virtqueue_kick(vi->vq);
+ spin_unlock_irqrestore(&vi->lock, flags);
}

static unsigned int copy_data(struct virtrng_info *vi, void *buf,
@@ -126,6 +268,40 @@ static void virtio_cleanup(struct hwrng *rng)
complete(&vi->have_data);
}

+static int init_virtqueues(struct virtrng_info *vi, struct virtio_device *vdev)
+{
+ int ret = -ENOMEM, total_vqs = 1;
+ struct virtqueue *vqs[3];
+ const char *names[3];
+ vq_callback_t *callbacks[3];
+
+ if (vi->has_leakqs)
+ total_vqs = 3;
+
+ callbacks[0] = random_recv_done;
+ names[0] = "input";
+ if (vi->has_leakqs) {
+ callbacks[1] = entropy_leak_detected;
+ names[1] = "leakq.1";
+ callbacks[2] = entropy_leak_detected;
+ names[2] = "leakq.2";
+ }
+
+ ret = virtio_find_vqs(vdev, total_vqs, vqs, callbacks, names, NULL);
+ if (ret)
+ goto err;
+
+ vi->vq = vqs[0];
+
+ if (vi->has_leakqs) {
+ vi->leakq[0] = vqs[1];
+ vi->leakq[1] = vqs[2];
+ }
+
+err:
+ return ret;
+}
+
static int probe_common(struct virtio_device *vdev)
{
int err, index;
@@ -152,18 +328,24 @@ static int probe_common(struct virtio_device *vdev)
};
vdev->priv = vi;

- /* We expect a single virtqueue. */
- vi->vq = virtio_find_single_vq(vdev, random_recv_done, "input");
- if (IS_ERR(vi->vq)) {
- err = PTR_ERR(vi->vq);
- goto err_find;
+ vi->has_leakqs = virtio_has_feature(vdev, VIRTIO_RNG_F_LEAK);
+ if (vi->has_leakqs) {
+ spin_lock_init(&vi->lock);
+ vi->active_leakq = 0;
}

+ err = init_virtqueues(vi, vdev);
+ if (err)
+ goto err_find;
+
virtio_device_ready(vdev);

/* we always have a pending entropy request */
request_entropy(vi);

+ /* we always have a fill_on_leak request pending */
+ virtrng_fill_on_leak(vi, vi->leak_data, sizeof(vi->leak_data));
+
return 0;

err_find:
@@ -246,7 +428,13 @@ static const struct virtio_device_id id_table[] = {
{ 0 },
};

+static unsigned int features[] = {
+ VIRTIO_RNG_F_LEAK,
+};
+
static struct virtio_driver virtio_rng_driver = {
+ .feature_table = features,
+ .feature_table_size = ARRAY_SIZE(features),
.driver.name = KBUILD_MODNAME,
.driver.owner = THIS_MODULE,
.id_table = id_table,
diff --git a/include/uapi/linux/virtio_rng.h b/include/uapi/linux/virtio_rng.h
index c4d5de896f0c..d9774951547e 100644
--- a/include/uapi/linux/virtio_rng.h
+++ b/include/uapi/linux/virtio_rng.h
@@ -5,4 +5,7 @@
#include <linux/virtio_ids.h>
#include <linux/virtio_config.h>

+/* The feature bitmap for virtio entropy device */
+#define VIRTIO_RNG_F_LEAK 0
+
#endif /* _LINUX_VIRTIO_RNG_H */
--
2.38.1

Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja M-401234 . CIF B84570936

2023-01-19 18:55:54

by Babis Chalios

[permalink] [raw]

Subject: [PATCH 2/2] virtio-rng: add sysfs entries for leak detection

Make use of the copy-on-leak functionality of the virtio rng driver to
expose a mechanism to user space for detecting entropy leak events, such
as taking a VM snapshot or restoring from one.

The driver setups a single page of memory where it stores in the first
word a counter and queues a copy-on-leak command for increasing the
counter every time an entropy leak occurs. It exposes the value of the
counter in a binary sysfs file per device. The file can be mmap'ed and
read and every time a change on the counter is observed, `sysfs_notify`
is used to notify processes that are polling it.

The mechanism is implemented based on the idea of a VM generation
counter that had been before proposed as an extension to the VM
Generation ID device, where mmap and poll interfaces can be used on the
file containing the counter and changes in its value signal snapshot
events.

It is worth noting that using mmap is entirely race-free, since changes
in the counter are observable by user-space as soon as vcpus are
resumed. Instead, using poll is not race-free. There is a race-window
between the moment the vcpus are resumed and the used-buffers are
handled by the virtio-rng driver.

Signed-off-by: Babis Chalios <[email protected]>
---
drivers/char/hw_random/virtio-rng.c | 166 +++++++++++++++++++++++++++-
1 file changed, 163 insertions(+), 3 deletions(-)

diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index 389a091a8801..003320f6c574 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -5,6 +5,9 @@
*/

#include "asm-generic/errno.h"
+#include "linux/gfp.h"
+#include "linux/minmax.h"
+#include "linux/sysfs.h"
#include <linux/err.h>
#include <linux/hw_random.h>
#include <linux/scatterlist.h>
@@ -17,6 +20,10 @@

static DEFINE_IDA(rng_index_ida);

+#ifdef CONFIG_SYSFS
+static struct kobject *virtio_rng_kobj;
+#endif
+
struct virtrng_info {
struct hwrng hwrng;
struct virtqueue *vq;
@@ -25,6 +32,12 @@ struct virtrng_info {
struct virtqueue *leakq[2];
spinlock_t lock;
int active_leakq;
+#ifdef CONFIG_SYSFS
+ struct kobject *kobj;
+ struct bin_attribute vm_gen_counter_attr;
+ unsigned long map_buffer;
+ unsigned long next_vm_gen_counter;
+#endif

char name[25];
int index;
@@ -44,6 +57,40 @@ struct virtrng_info {
#endif
};

+#ifdef CONFIG_SYSFS
+ssize_t virtrng_sysfs_read(struct file *filep, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t pos, size_t len)
+{
+ struct virtrng_info *vi = attr->private;
+ unsigned long gen_counter = *(unsigned long *)vi->map_buffer;
+
+ if (!len)
+ return 0;
+
+ len = min(len, sizeof(gen_counter));
+ memcpy(buf, &gen_counter, len);
+
+ return len;
+}
+
+int virtrng_sysfs_mmap(struct file *filep, struct kobject *kobj,
+ struct bin_attribute *attr, struct vm_area_struct *vma)
+{
+ struct virtrng_info *vi = attr->private;
+
+ if (vma->vm_pgoff || vma_pages(vma) > 1)
+ return -EINVAL;
+
+ if (vma->vm_flags & VM_WRITE)
+ return -EPERM;
+
+ vma->vm_flags |= VM_DONTEXPAND;
+ vma->vm_flags &= ~VM_MAYWRITE;
+
+ return vm_insert_page(vma, vma->vm_start, virt_to_page(vi->map_buffer));
+}
+#endif
+
/* Swaps the queues and returns the new active leak queue. */
static struct virtqueue *swap_leakqs(struct virtrng_info *vi)
{
@@ -83,7 +130,7 @@ int virtrng_fill_on_leak(struct virtrng_info *vi, void *data, size_t len)

vq = get_active_leakq(vi);
ret = add_fill_on_leak_request(vi, vq, data, len);
- if (ret)
+ if (!ret)
virtqueue_kick(vq);

spin_unlock_irqrestore(&vi->lock, flags);
@@ -123,7 +170,7 @@ int virtrng_copy_on_leak(struct virtrng_info *vi, void *to, void *from, size_t l

vq = get_active_leakq(vi);
ret = add_copy_on_leak_request(vi, vq, to, from, len);
- if (ret)
+ if (!ret)
virtqueue_kick(vq);

spin_unlock_irqrestore(&vi->lock, flags);
@@ -139,6 +186,9 @@ static void entropy_leak_detected(struct virtqueue *vq)
unsigned long flags;
void *buffer;
bool kick_activeq = false;
+#ifdef CONFIG_SYSFS
+ bool notify_sysfs = false;
+#endif

spin_lock_irqsave(&vi->lock, flags);

@@ -160,12 +210,34 @@ static void entropy_leak_detected(struct virtqueue *vq)
add_fill_on_leak_request(vi, activeq, vi->leak_data, sizeof(vi->leak_data));
kick_activeq = true;
}
+
+#ifdef CONFIG_SYSFS
+ if (buffer == (void *)vi->map_buffer) {
+ notify_sysfs = true;
+
+ /* Add a request to bump the generation counter on the next leak event.
+ * We have already swapped leak queues, so this will get properly handled
+ * with the next entropy leak event.
+ */
+ vi->next_vm_gen_counter++;
+ add_copy_on_leak_request(vi, activeq, (void *)vi->map_buffer,
+ &vi->next_vm_gen_counter, sizeof(unsigned long));
+
+ kick_activeq = true;
+ }
+#endif
}

if (kick_activeq)
virtqueue_kick(activeq);

spin_unlock_irqrestore(&vi->lock, flags);
+
+#ifdef CONFIG_SYSFS
+ /* Notify anyone polling on the sysfs file */
+ if (notify_sysfs)
+ sysfs_notify(vi->kobj, NULL, "vm_gen_counter");
+#endif
}

static void random_recv_done(struct virtqueue *vq)
@@ -302,6 +374,59 @@ static int init_virtqueues(struct virtrng_info *vi, struct virtio_device *vdev)
return ret;
}

+#ifdef CONFIG_SYSFS
+static int setup_sysfs(struct virtrng_info *vi)
+{
+ int err;
+
+ vi->next_vm_gen_counter = 1;
+
+ /* We have one binary file per device under /sys/virtio-rng/<device>/vm_gen_counter */
+ vi->vm_gen_counter_attr.attr.name = "vm_gen_counter";
+ vi->vm_gen_counter_attr.attr.mode = 0444;
+ vi->vm_gen_counter_attr.read = virtrng_sysfs_read;
+ vi->vm_gen_counter_attr.mmap = virtrng_sysfs_mmap;
+ vi->vm_gen_counter_attr.private = vi;
+
+ vi->map_buffer = get_zeroed_page(GFP_KERNEL);
+ if (!vi->map_buffer)
+ return -ENOMEM;
+
+ err = -ENOMEM;
+ vi->kobj = kobject_create_and_add(vi->name, virtio_rng_kobj);
+ if (!vi->kobj)
+ goto err_page;
+
+ err = sysfs_create_bin_file(vi->kobj, &vi->vm_gen_counter_attr);
+ if (err)
+ goto err_kobj;
+
+ return 0;
+
+err_kobj:
+ kobject_put(vi->kobj);
+err_page:
+ free_pages(vi->map_buffer, 0);
+ return err;
+}
+
+static void cleanup_sysfs(struct virtrng_info *vi)
+{
+ sysfs_remove_bin_file(vi->kobj, &vi->vm_gen_counter_attr);
+ kobject_put(vi->kobj);
+ free_pages(vi->map_buffer, 0);
+}
+#else
+static int setup_sysfs(struct virtrng_info *vi)
+{
+ return 0;
+}
+
+static void cleanup_sysfs(struct virtrng_info *vi)
+{
+}
+#endif
+
static int probe_common(struct virtio_device *vdev)
{
int err, index;
@@ -332,11 +457,15 @@ static int probe_common(struct virtio_device *vdev)
if (vi->has_leakqs) {
spin_lock_init(&vi->lock);
vi->active_leakq = 0;
+
+ err = setup_sysfs(vi);
+ if (err)
+ goto err_find;
}

err = init_virtqueues(vi, vdev);
if (err)
- goto err_find;
+ goto err_sysfs;

virtio_device_ready(vdev);

@@ -346,8 +475,18 @@ static int probe_common(struct virtio_device *vdev)
/* we always have a fill_on_leak request pending */
virtrng_fill_on_leak(vi, vi->leak_data, sizeof(vi->leak_data));

+#ifdef CONFIG_SYSFS
+ /* also a copy_on_leak request for the generation counter when we have sysfs
+ * support.
+ */
+ virtrng_copy_on_leak(vi, (void *)vi->map_buffer, &vi->next_vm_gen_counter,
+ sizeof(unsigned long));
+#endif
+
return 0;

+err_sysfs:
+ cleanup_sysfs(vi);
err_find:
ida_simple_remove(&rng_index_ida, index);
err_ida:
@@ -365,6 +504,8 @@ static void remove_common(struct virtio_device *vdev)
complete(&vi->have_data);
if (vi->hwrng_register_done)
hwrng_unregister(&vi->hwrng);
+ if (vi->has_leakqs)
+ cleanup_sysfs(vi);
virtio_reset_device(vdev);
vdev->config->del_vqs(vdev);
ida_simple_remove(&rng_index_ida, vi->index);
@@ -447,6 +588,25 @@ static struct virtio_driver virtio_rng_driver = {
#endif
};

+#ifdef CONFIG_SYSFS
+static int __init virtio_rng_init(void)
+{
+ virtio_rng_kobj = kobject_create_and_add("virtio-rng", NULL);
+ if (!virtio_rng_kobj)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void __exit virtio_rng_fini(void)
+{
+ kobject_put(virtio_rng_kobj);
+}
+
+module_init(virtio_rng_init);
+module_exit(virtio_rng_fini);
+#endif
+
module_virtio_driver(virtio_rng_driver);
MODULE_DEVICE_TABLE(virtio, id_table);
MODULE_DESCRIPTION("Virtio random number driver");
--
2.38.1

Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja M-401234 . CIF B84570936

2023-01-19 20:22:54

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 1/2] virtio-rng: implement entropy leak feature

Hi Babis,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.2-rc4 next-20230119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Babis-Chalios/virtio-rng-implement-entropy-leak-feature/20230120-024631
patch link: https://lore.kernel.org/r/20230119184349.74072-2-bchalios%40amazon.es
patch subject: [PATCH 1/2] virtio-rng: implement entropy leak feature
config: parisc-allyesconfig (https://download.01.org/0day-ci/archive/20230120/[email protected]/config)
compiler: hppa-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/686114cbba5005584d458ad44164b4a4b88135f5
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Babis-Chalios/virtio-rng-implement-entropy-leak-feature/20230120-024631
git checkout 686114cbba5005584d458ad44164b4a4b88135f5
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash drivers/char/hw_random/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

In file included from include/linux/err.h:8,
from drivers/char/hw_random/virtio-rng.c:8:
>> arch/parisc/include/uapi/asm/errno.h:7: warning: "ENOMSG" redefined
7 | #define ENOMSG 35 /* No message of desired type */
|
In file included from drivers/char/hw_random/virtio-rng.c:7:
include/uapi/asm-generic/errno.h:23: note: this is the location of the previous definition
23 | #define ENOMSG 42 /* No message of desired type */
|
>> arch/parisc/include/uapi/asm/errno.h:8: warning: "EIDRM" redefined
8 | #define EIDRM 36 /* Identifier removed */
|
include/uapi/asm-generic/errno.h:24: note: this is the location of the previous definition
24 | #define EIDRM 43 /* Identifier removed */
|
>> arch/parisc/include/uapi/asm/errno.h:9: warning: "ECHRNG" redefined
9 | #define ECHRNG 37 /* Channel number out of range */
|
include/uapi/asm-generic/errno.h:25: note: this is the location of the previous definition
25 | #define ECHRNG 44 /* Channel number out of range */
|
>> arch/parisc/include/uapi/asm/errno.h:10: warning: "EL2NSYNC" redefined
10 | #define EL2NSYNC 38 /* Level 2 not synchronized */
|
include/uapi/asm-generic/errno.h:26: note: this is the location of the previous definition
26 | #define EL2NSYNC 45 /* Level 2 not synchronized */
|
>> arch/parisc/include/uapi/asm/errno.h:11: warning: "EL3HLT" redefined
11 | #define EL3HLT 39 /* Level 3 halted */
|
include/uapi/asm-generic/errno.h:27: note: this is the location of the previous definition
27 | #define EL3HLT 46 /* Level 3 halted */
|
>> arch/parisc/include/uapi/asm/errno.h:12: warning: "EL3RST" redefined
12 | #define EL3RST 40 /* Level 3 reset */
|
include/uapi/asm-generic/errno.h:28: note: this is the location of the previous definition
28 | #define EL3RST 47 /* Level 3 reset */
|
>> arch/parisc/include/uapi/asm/errno.h:13: warning: "ELNRNG" redefined
13 | #define ELNRNG 41 /* Link number out of range */
|
include/uapi/asm-generic/errno.h:29: note: this is the location of the previous definition
29 | #define ELNRNG 48 /* Link number out of range */
|
>> arch/parisc/include/uapi/asm/errno.h:14: warning: "EUNATCH" redefined
14 | #define EUNATCH 42 /* Protocol driver not attached */
|
include/uapi/asm-generic/errno.h:30: note: this is the location of the previous definition
30 | #define EUNATCH 49 /* Protocol driver not attached */
|
>> arch/parisc/include/uapi/asm/errno.h:15: warning: "ENOCSI" redefined
15 | #define ENOCSI 43 /* No CSI structure available */
|
include/uapi/asm-generic/errno.h:31: note: this is the location of the previous definition
31 | #define ENOCSI 50 /* No CSI structure available */
|
>> arch/parisc/include/uapi/asm/errno.h:16: warning: "EL2HLT" redefined
16 | #define EL2HLT 44 /* Level 2 halted */
|
include/uapi/asm-generic/errno.h:32: note: this is the location of the previous definition
32 | #define EL2HLT 51 /* Level 2 halted */
|
>> arch/parisc/include/uapi/asm/errno.h:17: warning: "EDEADLK" redefined
17 | #define EDEADLK 45 /* Resource deadlock would occur */
|
include/uapi/asm-generic/errno.h:7: note: this is the location of the previous definition
7 | #define EDEADLK 35 /* Resource deadlock would occur */
|
>> arch/parisc/include/uapi/asm/errno.h:19: warning: "ENOLCK" redefined
19 | #define ENOLCK 46 /* No record locks available */
|
include/uapi/asm-generic/errno.h:9: note: this is the location of the previous definition
9 | #define ENOLCK 37 /* No record locks available */
|
>> arch/parisc/include/uapi/asm/errno.h:20: warning: "EILSEQ" redefined
20 | #define EILSEQ 47 /* Illegal byte sequence */
|
include/uapi/asm-generic/errno.h:67: note: this is the location of the previous definition
67 | #define EILSEQ 84 /* Illegal byte sequence */
|
>> arch/parisc/include/uapi/asm/errno.h:22: warning: "ENONET" redefined
22 | #define ENONET 50 /* Machine is not on the network */
|
include/uapi/asm-generic/errno.h:47: note: this is the location of the previous definition
47 | #define ENONET 64 /* Machine is not on the network */
|
>> arch/parisc/include/uapi/asm/errno.h:23: warning: "ENODATA" redefined
23 | #define ENODATA 51 /* No data available */
|
include/uapi/asm-generic/errno.h:44: note: this is the location of the previous definition
44 | #define ENODATA 61 /* No data available */
|
>> arch/parisc/include/uapi/asm/errno.h:24: warning: "ETIME" redefined
24 | #define ETIME 52 /* Timer expired */
|
include/uapi/asm-generic/errno.h:45: note: this is the location of the previous definition
45 | #define ETIME 62 /* Timer expired */
|
>> arch/parisc/include/uapi/asm/errno.h:25: warning: "ENOSR" redefined
25 | #define ENOSR 53 /* Out of streams resources */
|
include/uapi/asm-generic/errno.h:46: note: this is the location of the previous definition
46 | #define ENOSR 63 /* Out of streams resources */
|
>> arch/parisc/include/uapi/asm/errno.h:26: warning: "ENOSTR" redefined
26 | #define ENOSTR 54 /* Device not a stream */
|
include/uapi/asm-generic/errno.h:43: note: this is the location of the previous definition
43 | #define ENOSTR 60 /* Device not a stream */
|
>> arch/parisc/include/uapi/asm/errno.h:27: warning: "ENOPKG" redefined
27 | #define ENOPKG 55 /* Package not installed */
|
include/uapi/asm-generic/errno.h:48: note: this is the location of the previous definition
48 | #define ENOPKG 65 /* Package not installed */
|
>> arch/parisc/include/uapi/asm/errno.h:29: warning: "ENOLINK" redefined
29 | #define ENOLINK 57 /* Link has been severed */
|
include/uapi/asm-generic/errno.h:50: note: this is the location of the previous definition
50 | #define ENOLINK 67 /* Link has been severed */
|
arch/parisc/include/uapi/asm/errno.h:30: warning: "EADV" redefined
30 | #define EADV 58 /* Advertise error */
|
include/uapi/asm-generic/errno.h:51: note: this is the location of the previous definition
51 | #define EADV 68 /* Advertise error */
|
arch/parisc/include/uapi/asm/errno.h:31: warning: "ESRMNT" redefined
31 | #define ESRMNT 59 /* Srmount error */
|
include/uapi/asm-generic/errno.h:52: note: this is the location of the previous definition
52 | #define ESRMNT 69 /* Srmount error */
|
arch/parisc/include/uapi/asm/errno.h:32: warning: "ECOMM" redefined
32 | #define ECOMM 60 /* Communication error on send */
|
include/uapi/asm-generic/errno.h:53: note: this is the location of the previous definition
53 | #define ECOMM 70 /* Communication error on send */
|
arch/parisc/include/uapi/asm/errno.h:33: warning: "EPROTO" redefined
33 | #define EPROTO 61 /* Protocol error */
|
include/uapi/asm-generic/errno.h:54: note: this is the location of the previous definition
54 | #define EPROTO 71 /* Protocol error */
|
arch/parisc/include/uapi/asm/errno.h:35: warning: "EMULTIHOP" redefined
35 | #define EMULTIHOP 64 /* Multihop attempted */
|
include/uapi/asm-generic/errno.h:55: note: this is the location of the previous definition
55 | #define EMULTIHOP 72 /* Multihop attempted */
|
arch/parisc/include/uapi/asm/errno.h:37: warning: "EDOTDOT" redefined
37 | #define EDOTDOT 66 /* RFS specific error */
|
include/uapi/asm-generic/errno.h:56: note: this is the location of the previous definition
56 | #define EDOTDOT 73 /* RFS specific error */
|
arch/parisc/include/uapi/asm/errno.h:38: warning: "EBADMSG" redefined
38 | #define EBADMSG 67 /* Not a data message */
|
include/uapi/asm-generic/errno.h:57: note: this is the location of the previous definition
57 | #define EBADMSG 74 /* Not a data message */
|
arch/parisc/include/uapi/asm/errno.h:39: warning: "EUSERS" redefined
39 | #define EUSERS 68 /* Too many users */
|
include/uapi/asm-generic/errno.h:70: note: this is the location of the previous definition
70 | #define EUSERS 87 /* Too many users */
|
arch/parisc/include/uapi/asm/errno.h:40: warning: "EDQUOT" redefined
40 | #define EDQUOT 69 /* Quota exceeded */
|
include/uapi/asm-generic/errno.h:105: note: this is the location of the previous definition
105 | #define EDQUOT 122 /* Quota exceeded */
|
arch/parisc/include/uapi/asm/errno.h:41: warning: "ESTALE" redefined
41 | #define ESTALE 70 /* Stale file handle */
|
include/uapi/asm-generic/errno.h:99: note: this is the location of the previous definition
99 | #define ESTALE 116 /* Stale file handle */
|
arch/parisc/include/uapi/asm/errno.h:42: warning: "EREMOTE" redefined
42 | #define EREMOTE 71 /* Object is remote */
|
include/uapi/asm-generic/errno.h:49: note: this is the location of the previous definition
49 | #define EREMOTE 66 /* Object is remote */
|
arch/parisc/include/uapi/asm/errno.h:43: warning: "EOVERFLOW" redefined
43 | #define EOVERFLOW 72 /* Value too large for defined data type */
|
include/uapi/asm-generic/errno.h:58: note: this is the location of the previous definition
58 | #define EOVERFLOW 75 /* Value too large for defined data type */
|
arch/parisc/include/uapi/asm/errno.h:47: warning: "EBADE" redefined
47 | #define EBADE 160 /* Invalid exchange */
|
include/uapi/asm-generic/errno.h:33: note: this is the location of the previous definition
33 | #define EBADE 52 /* Invalid exchange */
|
arch/parisc/include/uapi/asm/errno.h:48: warning: "EBADR" redefined
48 | #define EBADR 161 /* Invalid request descriptor */
|
include/uapi/asm-generic/errno.h:34: note: this is the location of the previous definition
34 | #define EBADR 53 /* Invalid request descriptor */
|
arch/parisc/include/uapi/asm/errno.h:49: warning: "EXFULL" redefined
49 | #define EXFULL 162 /* Exchange full */
|
include/uapi/asm-generic/errno.h:35: note: this is the location of the previous definition
35 | #define EXFULL 54 /* Exchange full */
|
arch/parisc/include/uapi/asm/errno.h:50: warning: "ENOANO" redefined
50 | #define ENOANO 163 /* No anode */
|
include/uapi/asm-generic/errno.h:36: note: this is the location of the previous definition
36 | #define ENOANO 55 /* No anode */

vim +/ENOMSG +7 arch/parisc/include/uapi/asm/errno.h

^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 6
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @7 #define ENOMSG 35 /* No message of desired type */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @8 #define EIDRM 36 /* Identifier removed */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @9 #define ECHRNG 37 /* Channel number out of range */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @10 #define EL2NSYNC 38 /* Level 2 not synchronized */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @11 #define EL3HLT 39 /* Level 3 halted */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @12 #define EL3RST 40 /* Level 3 reset */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @13 #define ELNRNG 41 /* Link number out of range */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @14 #define EUNATCH 42 /* Protocol driver not attached */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @15 #define ENOCSI 43 /* No CSI structure available */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @16 #define EL2HLT 44 /* Level 2 halted */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @17 #define EDEADLK 45 /* Resource deadlock would occur */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 18 #define EDEADLOCK EDEADLK
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @19 #define ENOLCK 46 /* No record locks available */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @20 #define EILSEQ 47 /* Illegal byte sequence */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 21
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @22 #define ENONET 50 /* Machine is not on the network */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @23 #define ENODATA 51 /* No data available */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @24 #define ETIME 52 /* Timer expired */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @25 #define ENOSR 53 /* Out of streams resources */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @26 #define ENOSTR 54 /* Device not a stream */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @27 #define ENOPKG 55 /* Package not installed */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 28
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @29 #define ENOLINK 57 /* Link has been severed */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @30 #define EADV 58 /* Advertise error */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @31 #define ESRMNT 59 /* Srmount error */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @32 #define ECOMM 60 /* Communication error on send */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @33 #define EPROTO 61 /* Protocol error */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 34
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @35 #define EMULTIHOP 64 /* Multihop attempted */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 36
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @37 #define EDOTDOT 66 /* RFS specific error */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @38 #define EBADMSG 67 /* Not a data message */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @39 #define EUSERS 68 /* Too many users */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @40 #define EDQUOT 69 /* Quota exceeded */
0ca43435188b9f arch/parisc/include/uapi/asm/errno.h Eric Sandeen 2013-11-12 @41 #define ESTALE 70 /* Stale file handle */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @42 #define EREMOTE 71 /* Object is remote */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @43 #define EOVERFLOW 72 /* Value too large for defined data type */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 44
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 45 /* these errnos are defined by Linux but not HPUX. */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 46
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @47 #define EBADE 160 /* Invalid exchange */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @48 #define EBADR 161 /* Invalid request descriptor */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @49 #define EXFULL 162 /* Exchange full */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @50 #define ENOANO 163 /* No anode */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @51 #define EBADRQC 164 /* Invalid request code */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @52 #define EBADSLT 165 /* Invalid slot */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @53 #define EBFONT 166 /* Bad font file format */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @54 #define ENOTUNIQ 167 /* Name not unique on network */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @55 #define EBADFD 168 /* File descriptor in bad state */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @56 #define EREMCHG 169 /* Remote address changed */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @57 #define ELIBACC 170 /* Can not access a needed shared library */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @58 #define ELIBBAD 171 /* Accessing a corrupted shared library */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @59 #define ELIBSCN 172 /* .lib section in a.out corrupted */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @60 #define ELIBMAX 173 /* Attempting to link in too many shared libraries */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @61 #define ELIBEXEC 174 /* Cannot exec a shared library directly */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @62 #define ERESTART 175 /* Interrupted system call should be restarted */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @63 #define ESTRPIPE 176 /* Streams pipe error */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @64 #define EUCLEAN 177 /* Structure needs cleaning */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @65 #define ENOTNAM 178 /* Not a XENIX named type file */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @66 #define ENAVAIL 179 /* No XENIX semaphores available */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @67 #define EISNAM 180 /* Is a named type file */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @68 #define EREMOTEIO 181 /* Remote I/O error */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @69 #define ENOMEDIUM 182 /* No medium found */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @70 #define EMEDIUMTYPE 183 /* Wrong medium type */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @71 #define ENOKEY 184 /* Required key not available */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @72 #define EKEYEXPIRED 185 /* Key has expired */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @73 #define EKEYREVOKED 186 /* Key has been revoked */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @74 #define EKEYREJECTED 187 /* Key was rejected by service */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 75
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 76 /* We now return you to your regularly scheduled HPUX. */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 77
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 78 #define ENOSYM 215 /* symbol does not exist in executable */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @79 #define ENOTSOCK 216 /* Socket operation on non-socket */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @80 #define EDESTADDRREQ 217 /* Destination address required */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @81 #define EMSGSIZE 218 /* Message too long */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @82 #define EPROTOTYPE 219 /* Protocol wrong type for socket */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @83 #define ENOPROTOOPT 220 /* Protocol not available */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @84 #define EPROTONOSUPPORT 221 /* Protocol not supported */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @85 #define ESOCKTNOSUPPORT 222 /* Socket type not supported */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @86 #define EOPNOTSUPP 223 /* Operation not supported on transport endpoint */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @87 #define EPFNOSUPPORT 224 /* Protocol family not supported */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @88 #define EAFNOSUPPORT 225 /* Address family not supported by protocol */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @89 #define EADDRINUSE 226 /* Address already in use */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @90 #define EADDRNOTAVAIL 227 /* Cannot assign requested address */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @91 #define ENETDOWN 228 /* Network is down */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @92 #define ENETUNREACH 229 /* Network is unreachable */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @93 #define ENETRESET 230 /* Network dropped connection because of reset */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @94 #define ECONNABORTED 231 /* Software caused connection abort */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @95 #define ECONNRESET 232 /* Connection reset by peer */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @96 #define ENOBUFS 233 /* No buffer space available */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @97 #define EISCONN 234 /* Transport endpoint is already connected */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @98 #define ENOTCONN 235 /* Transport endpoint is not connected */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @99 #define ESHUTDOWN 236 /* Cannot send after transport endpoint shutdown */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @100 #define ETOOMANYREFS 237 /* Too many references: cannot splice */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @101 #define ETIMEDOUT 238 /* Connection timed out */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @102 #define ECONNREFUSED 239 /* Connection refused */
3eb53b20d7bd13 arch/parisc/include/uapi/asm/errno.h Helge Deller 2016-08-20 103 #define EREFUSED ECONNREFUSED /* for HP's NFS apparently */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 104 #define EREMOTERELEASE 240 /* Remote peer released connection */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @105 #define EHOSTDOWN 241 /* Host is down */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @106 #define EHOSTUNREACH 242 /* No route to host */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 107
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @108 #define EALREADY 244 /* Operation already in progress */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @109 #define EINPROGRESS 245 /* Operation now in progress */
f5a408d53edef3 arch/parisc/include/uapi/asm/errno.h Guy Martin 2014-01-16 110 #define EWOULDBLOCK EAGAIN /* Operation would block (Not HPUX compliant) */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @111 #define ENOTEMPTY 247 /* Directory not empty */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @112 #define ENAMETOOLONG 248 /* File name too long */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @113 #define ELOOP 249 /* Too many symbolic links encountered */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 @114 #define ENOSYS 251 /* Function not implemented */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 115
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 116 #define ECANCELLED 253 /* aio request was canceled before complete (POSIX.4 / HPUX) */
05aa10de701409 include/asm-parisc/errno.h Grant Grundler 2005-10-21 @117 #define ECANCELED ECANCELLED /* SuSv3 and Solaris wants one 'L' */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 118
4750e2c0c59e0c include/asm-parisc/errno.h Joe Korty 2005-05-01 119 /* for robust mutexes */
4750e2c0c59e0c include/asm-parisc/errno.h Joe Korty 2005-05-01 @120 #define EOWNERDEAD 254 /* Owner died */
4750e2c0c59e0c include/asm-parisc/errno.h Joe Korty 2005-05-01 @121 #define ENOTRECOVERABLE 255 /* State not recoverable */
4750e2c0c59e0c include/asm-parisc/errno.h Joe Korty 2005-05-01 122
7d217d7ffc3433 arch/parisc/include/asm/errno.h Alexander Beregalov 2009-06-07 @123 #define ERFKILL 256 /* Operation not possible due to RF-kill */
^1da177e4c3f41 include/asm-parisc/errno.h Linus Torvalds 2005-04-16 124
69ebb83e13e514 arch/parisc/include/asm/errno.h Huang Ying 2011-01-30 @125 #define EHWPOISON 257 /* Memory page has hardware error */
69ebb83e13e514 arch/parisc/include/asm/errno.h Huang Ying 2011-01-30 126

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-01-19 22:59:20

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 2/2] virtio-rng: add sysfs entries for leak detection

2023-01-19 23:03:48

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 2/2] virtio-rng: add sysfs entries for leak detection

Hi Babis,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.2-rc4 next-20230119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Babis-Chalios/virtio-rng-implement-entropy-leak-feature/20230120-024631
patch link: https://lore.kernel.org/r/20230119184349.74072-3-bchalios%40amazon.es
patch subject: [PATCH 2/2] virtio-rng: add sysfs entries for leak detection
config: ia64-randconfig-r023-20230119 (https://download.01.org/0day-ci/archive/20230120/[email protected]/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2a801d93b8225555e4cb293a173e2053870cb2d1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Babis-Chalios/virtio-rng-implement-entropy-leak-feature/20230120-024631
git checkout 2a801d93b8225555e4cb293a173e2053870cb2d1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/char/hw_random/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/char/hw_random/virtio-rng.c:61:9: warning: no previous prototype for 'virtrng_sysfs_read' [-Wmissing-prototypes]
61 | ssize_t virtrng_sysfs_read(struct file *filep, struct kobject *kobj,
| ^~~~~~~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:76:5: warning: no previous prototype for 'virtrng_sysfs_mmap' [-Wmissing-prototypes]
76 | int virtrng_sysfs_mmap(struct file *filep, struct kobject *kobj,
| ^~~~~~~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:106:5: warning: no previous prototype for 'add_fill_on_leak_request' [-Wmissing-prototypes]
106 | int add_fill_on_leak_request(struct virtrng_info *vi, struct virtqueue *vq, void *data, size_t len)
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:120:5: warning: no previous prototype for 'virtrng_fill_on_leak' [-Wmissing-prototypes]
120 | int virtrng_fill_on_leak(struct virtrng_info *vi, void *data, size_t len)
| ^~~~~~~~~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:141:5: warning: no previous prototype for 'add_copy_on_leak_request' [-Wmissing-prototypes]
141 | int add_copy_on_leak_request(struct virtrng_info *vi, struct virtqueue *vq,
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:160:5: warning: no previous prototype for 'virtrng_copy_on_leak' [-Wmissing-prototypes]
160 | int virtrng_copy_on_leak(struct virtrng_info *vi, void *to, void *from, size_t len)
| ^~~~~~~~~~~~~~~~~~~~
In file included from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from include/linux/virtio.h:9,
from drivers/char/hw_random/virtio-rng.c:15:
include/linux/module.h:130:49: error: redefinition of '__inittest'
130 | static inline initcall_t __maybe_unused __inittest(void) \
| ^~~~~~~~~~
include/linux/device/driver.h:267:1: note: in expansion of macro 'module_init'
267 | module_init(__driver##_init); \
| ^~~~~~~~~~~
include/linux/virtio.h:207:9: note: in expansion of macro 'module_driver'
207 | module_driver(__virtio_driver, register_virtio_driver, \
| ^~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:609:1: note: in expansion of macro 'module_virtio_driver'
609 | module_virtio_driver(virtio_rng_driver);
| ^~~~~~~~~~~~~~~~~~~~
include/linux/module.h:130:49: note: previous definition of '__inittest' with type 'int (*(void))(void)'
130 | static inline initcall_t __maybe_unused __inittest(void) \
| ^~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:605:1: note: in expansion of macro 'module_init'
605 | module_init(virtio_rng_init);
| ^~~~~~~~~~~
include/linux/module.h:132:13: error: redefinition of 'init_module'
132 | int init_module(void) __copy(initfn) \
| ^~~~~~~~~~~
include/linux/device/driver.h:267:1: note: in expansion of macro 'module_init'
267 | module_init(__driver##_init); \
| ^~~~~~~~~~~
include/linux/virtio.h:207:9: note: in expansion of macro 'module_driver'
207 | module_driver(__virtio_driver, register_virtio_driver, \
| ^~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:609:1: note: in expansion of macro 'module_virtio_driver'
609 | module_virtio_driver(virtio_rng_driver);
| ^~~~~~~~~~~~~~~~~~~~
include/linux/module.h:132:13: note: previous definition of 'init_module' with type 'int(void)'
132 | int init_module(void) __copy(initfn) \
| ^~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:605:1: note: in expansion of macro 'module_init'
605 | module_init(virtio_rng_init);
| ^~~~~~~~~~~
>> include/linux/module.h:138:49: error: redefinition of '__exittest'
138 | static inline exitcall_t __maybe_unused __exittest(void) \
| ^~~~~~~~~~
include/linux/device/driver.h:272:1: note: in expansion of macro 'module_exit'
272 | module_exit(__driver##_exit);
| ^~~~~~~~~~~
include/linux/virtio.h:207:9: note: in expansion of macro 'module_driver'
207 | module_driver(__virtio_driver, register_virtio_driver, \
| ^~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:609:1: note: in expansion of macro 'module_virtio_driver'
609 | module_virtio_driver(virtio_rng_driver);
| ^~~~~~~~~~~~~~~~~~~~
include/linux/module.h:138:49: note: previous definition of '__exittest' with type 'void (*(void))(void)'
138 | static inline exitcall_t __maybe_unused __exittest(void) \
| ^~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:606:1: note: in expansion of macro 'module_exit'
606 | module_exit(virtio_rng_fini);
| ^~~~~~~~~~~
>> include/linux/module.h:140:14: error: redefinition of 'cleanup_module'
140 | void cleanup_module(void) __copy(exitfn) \
| ^~~~~~~~~~~~~~
include/linux/device/driver.h:272:1: note: in expansion of macro 'module_exit'
272 | module_exit(__driver##_exit);
| ^~~~~~~~~~~
include/linux/virtio.h:207:9: note: in expansion of macro 'module_driver'
207 | module_driver(__virtio_driver, register_virtio_driver, \
| ^~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:609:1: note: in expansion of macro 'module_virtio_driver'
609 | module_virtio_driver(virtio_rng_driver);
| ^~~~~~~~~~~~~~~~~~~~
include/linux/module.h:140:14: note: previous definition of 'cleanup_module' with type 'void(void)'
140 | void cleanup_module(void) __copy(exitfn) \
| ^~~~~~~~~~~~~~
drivers/char/hw_random/virtio-rng.c:606:1: note: in expansion of macro 'module_exit'
606 | module_exit(virtio_rng_fini);
| ^~~~~~~~~~~

vim +/__exittest +138 include/linux/module.h

0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 127
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 128 /* Each module must use one module_init(). */
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 129 #define module_init(initfn) \
1f318a8bafcfba9 Arnd Bergmann 2017-02-01 130 static inline initcall_t __maybe_unused __inittest(void) \
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 131 { return initfn; } \
cf68fffb66d60d9 Sami Tolvanen 2021-04-08 132 int init_module(void) __copy(initfn) \
cf68fffb66d60d9 Sami Tolvanen 2021-04-08 133 __attribute__((alias(#initfn))); \
92efda8eb15295a Sami Tolvanen 2022-09-08 134 ___ADDRESSABLE(init_module, __initdata);
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 135
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 136 /* This is only required if you want to be unloadable. */
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 137 #define module_exit(exitfn) \
1f318a8bafcfba9 Arnd Bergmann 2017-02-01 @138 static inline exitcall_t __maybe_unused __exittest(void) \
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 139 { return exitfn; } \
cf68fffb66d60d9 Sami Tolvanen 2021-04-08 @140 void cleanup_module(void) __copy(exitfn) \
cf68fffb66d60d9 Sami Tolvanen 2021-04-08 141 __attribute__((alias(#exitfn))); \
92efda8eb15295a Sami Tolvanen 2022-09-08 142 ___ADDRESSABLE(cleanup_module, __exitdata);
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 143

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-01-20 01:32:07

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 1/2] virtio-rng: implement entropy leak feature

2023-01-20 04:46:44

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 2/2] virtio-rng: add sysfs entries for leak detection

2023-01-20 12:35:49

by Michael S. Tsirkin

[permalink] [raw]

Subject: Re: [PATCH 1/2] virtio-rng: implement entropy leak feature

On Thu, Jan 19, 2023 at 07:43:47PM +0100, Babis Chalios wrote:
> Implement the virtio-rng feature that allows a guest driver to request
> from the device to perform certain operations in the event of an
> "entropy leak", such as when taking a VM snapshot or restoring a VM from
> a snapshot. The guest can request one of two operations: (i) fill a
> buffer with random bytes, or (ii) perform a memory copy between two
> bytes.
>
> The feature is similar to Microsoft's Virtual Machine Generation ID and
> it can be used to (1) avoid the race-condition that exists in our
> current VMGENID implementation, between the time vcpus are resumed and
> the ACPI notification is being handled and (2) provide mechanisms for
> notifying user-space about snapshot-related events.
>
> This commit implements the protocol between guest and device.
> Additionally, it makes sure there is always a request for random bytes
> in the event of entropy leak in-flight. Once such an event is observed,
> the driver feeds these bytes to as entropy using
> `add_device_randomness`.
>
> Keep in mind that this commit does not solve the race-condition issue,
> it adds fresh entropy whenever the driver handles the used buffer from
> the fill-on-leak request. In order to close the race window, we need to
> expose some API so that other kernel subsystems can request directly
> notifications from the device.
>
> Signed-off-by: Babis Chalios <[email protected]>
> ---
> drivers/char/hw_random/virtio-rng.c | 200 +++++++++++++++++++++++++++-
> include/uapi/linux/virtio_rng.h | 3 +
> 2 files changed, 197 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
> index a6f3a8a2aca6..389a091a8801 100644
> --- a/drivers/char/hw_random/virtio-rng.c
> +++ b/drivers/char/hw_random/virtio-rng.c
> @@ -4,12 +4,14 @@
> * Copyright (C) 2007, 2008 Rusty Russell IBM Corporation
> */
>
> +#include "asm-generic/errno.h"

Why are you pulling this in?
If you really need errno the thing to include
is linux/errno.h

checkpatch has a rule to catch such cases:

# warn if <asm/foo.h> is #included and <linux/foo.h> is available and includes
# itself <asm/foo.h> (uses RAW line)

did you run checkpatch?

> #include <linux/err.h>
> #include <linux/hw_random.h>
> #include <linux/scatterlist.h>
> #include <linux/spinlock.h>
> #include <linux/virtio.h>
> #include <linux/virtio_rng.h>
> +#include <linux/random.h>
> #include <linux/module.h>
> #include <linux/slab.h>
>
> @@ -18,6 +20,12 @@ static DEFINE_IDA(rng_index_ida);
> struct virtrng_info {
> struct hwrng hwrng;
> struct virtqueue *vq;
> + /* Leak queues */
> + bool has_leakqs;
> + struct virtqueue *leakq[2];
> + spinlock_t lock;
> + int active_leakq;
> +
> char name[25];
> int index;
> bool hwrng_register_done;
> @@ -29,27 +37,159 @@ struct virtrng_info {
> /* minimal size returned by rng_buffer_size() */
> #if SMP_CACHE_BYTES < 32
> u8 data[32];
> + u8 leak_data[32];
> #else
> u8 data[SMP_CACHE_BYTES];
> + u8 leak_data[SMP_CACHE_BYTES];
> #endif
> };
>
> +/* Swaps the queues and returns the new active leak queue. */
> +static struct virtqueue *swap_leakqs(struct virtrng_info *vi)
> +{
> + vi->active_leakq = 1 - vi->active_leakq;
> + return vi->leakq[vi->active_leakq];
> +}
> +
> +static struct virtqueue *get_active_leakq(struct virtrng_info *vi)
> +{
> + return vi->leakq[vi->active_leakq];
> +}
> +
> +int add_fill_on_leak_request(struct virtrng_info *vi, struct virtqueue *vq, void *data, size_t len)
> +{
> + struct scatterlist sg;
> + int ret;
> +
> + sg_init_one(&sg, data, len);
> + ret = virtqueue_add_inbuf(vq, &sg, 1, data, GFP_KERNEL);
> + if (ret)
> + goto err;
> +
> +err:
> + return ret;
> +}
> +
> +int virtrng_fill_on_leak(struct virtrng_info *vi, void *data, size_t len)
> +{
> + struct virtqueue *vq;
> + unsigned long flags;
> + int ret;
> +
> + if (!vi->has_leakqs)
> + return -EOPNOTSUPP;
> +
> + spin_lock_irqsave(&vi->lock, flags);
> +
> + vq = get_active_leakq(vi);
> + ret = add_fill_on_leak_request(vi, vq, data, len);
> + if (ret)
> + virtqueue_kick(vq);
> +
> + spin_unlock_irqrestore(&vi->lock, flags);
> +
> + return ret;
> +}
> +
> +int add_copy_on_leak_request(struct virtrng_info *vi, struct virtqueue *vq,
> + void *to, void *from, size_t len)
> +{
> + int ret;
> + struct scatterlist out, in, *sgs[2];
> +
> + sg_init_one(&out, from, len);
> + sgs[0] = &out;
> + sg_init_one(&in, to, len);
> + sgs[1] = &in;
> +
> + ret = virtqueue_add_sgs(vq, sgs, 1, 1, to, GFP_KERNEL);
> + if (ret)
> + goto err;
> +
> +err:
> + return ret;
> +}
> +
> +int virtrng_copy_on_leak(struct virtrng_info *vi, void *to, void *from, size_t len)
> +{
> + struct virtqueue *vq;
> + unsigned long flags;
> + int ret;
> +
> + if (!vi->has_leakqs)
> + return -EOPNOTSUPP;
> +
> + spin_lock_irqsave(&vi->lock, flags);
> +
> + vq = get_active_leakq(vi);
> + ret = add_copy_on_leak_request(vi, vq, to, from, len);
> + if (ret)
> + virtqueue_kick(vq);
> +
> + spin_unlock_irqrestore(&vi->lock, flags);
> +
> + return ret;
> +}
> +
> +static void entropy_leak_detected(struct virtqueue *vq)
> +{
> + struct virtrng_info *vi = vq->vdev->priv;
> + struct virtqueue *activeq;
> + unsigned int len;
> + unsigned long flags;
> + void *buffer;
> + bool kick_activeq = false;
> +
> + spin_lock_irqsave(&vi->lock, flags);
> +
> + activeq = get_active_leakq(vi);
> + /* Drain all the used buffers from the queue */
> + while ((buffer = virtqueue_get_buf(vq, &len)) != NULL) {
> + if (vq == activeq) {
> + pr_debug("%s: entropy leak detected!", vi->name);
> + activeq = swap_leakqs(vi);
> + }
> +
> + if (buffer == vi->leak_data) {
> + add_device_randomness(vi->leak_data, sizeof(vi->leak_data));
> +
> + /* Ensure we always have a pending request for random bytes on entropy
> + * leak. Do it here, after we have swapped leak queues, so it gets handled
> + * with the next entropy leak event.
> + */
> + add_fill_on_leak_request(vi, activeq, vi->leak_data, sizeof(vi->leak_data));
> + kick_activeq = true;
> + }
> + }
> +
> + if (kick_activeq)
> + virtqueue_kick(activeq);
> +
> + spin_unlock_irqrestore(&vi->lock, flags);
> +}
> +
> static void random_recv_done(struct virtqueue *vq)
> {
> struct virtrng_info *vi = vq->vdev->priv;
> + unsigned long flags;
>
> + spin_lock_irqsave(&vi->lock, flags);
> /* We can get spurious callbacks, e.g. shared IRQs + virtio_pci. */
> if (!virtqueue_get_buf(vi->vq, &vi->data_avail))
> - return;
> + goto unlock;
>
> vi->data_idx = 0;
>
> complete(&vi->have_data);
> +
> +unlock:
> + spin_unlock_irqrestore(&vi->lock, flags);
> }
>
> static void request_entropy(struct virtrng_info *vi)
> {
> struct scatterlist sg;
> + unsigned long flags;
>
> reinit_completion(&vi->have_data);
> vi->data_avail = 0;
> @@ -57,10 +197,12 @@ static void request_entropy(struct virtrng_info *vi)
>
> sg_init_one(&sg, vi->data, sizeof(vi->data));
>
> + spin_lock_irqsave(&vi->lock, flags);
> /* There should always be room for one buffer. */
> virtqueue_add_inbuf(vi->vq, &sg, 1, vi->data, GFP_KERNEL);
>
> virtqueue_kick(vi->vq);
> + spin_unlock_irqrestore(&vi->lock, flags);
> }
>
> static unsigned int copy_data(struct virtrng_info *vi, void *buf,
> @@ -126,6 +268,40 @@ static void virtio_cleanup(struct hwrng *rng)
> complete(&vi->have_data);
> }
>
> +static int init_virtqueues(struct virtrng_info *vi, struct virtio_device *vdev)
> +{
> + int ret = -ENOMEM, total_vqs = 1;
> + struct virtqueue *vqs[3];
> + const char *names[3];
> + vq_callback_t *callbacks[3];
> +
> + if (vi->has_leakqs)
> + total_vqs = 3;
> +
> + callbacks[0] = random_recv_done;
> + names[0] = "input";
> + if (vi->has_leakqs) {
> + callbacks[1] = entropy_leak_detected;
> + names[1] = "leakq.1";
> + callbacks[2] = entropy_leak_detected;
> + names[2] = "leakq.2";
> + }
> +
> + ret = virtio_find_vqs(vdev, total_vqs, vqs, callbacks, names, NULL);
> + if (ret)
> + goto err;
> +
> + vi->vq = vqs[0];
> +
> + if (vi->has_leakqs) {
> + vi->leakq[0] = vqs[1];
> + vi->leakq[1] = vqs[2];
> + }
> +
> +err:
> + return ret;
> +}
> +
> static int probe_common(struct virtio_device *vdev)
> {
> int err, index;
> @@ -152,18 +328,24 @@ static int probe_common(struct virtio_device *vdev)
> };
> vdev->priv = vi;
>
> - /* We expect a single virtqueue. */
> - vi->vq = virtio_find_single_vq(vdev, random_recv_done, "input");
> - if (IS_ERR(vi->vq)) {
> - err = PTR_ERR(vi->vq);
> - goto err_find;
> + vi->has_leakqs = virtio_has_feature(vdev, VIRTIO_RNG_F_LEAK);
> + if (vi->has_leakqs) {
> + spin_lock_init(&vi->lock);
> + vi->active_leakq = 0;
> }
>
> + err = init_virtqueues(vi, vdev);
> + if (err)
> + goto err_find;
> +
> virtio_device_ready(vdev);
>
> /* we always have a pending entropy request */
> request_entropy(vi);
>
> + /* we always have a fill_on_leak request pending */
> + virtrng_fill_on_leak(vi, vi->leak_data, sizeof(vi->leak_data));
> +
> return 0;
>
> err_find:
> @@ -246,7 +428,13 @@ static const struct virtio_device_id id_table[] = {
> { 0 },
> };
>
> +static unsigned int features[] = {
> + VIRTIO_RNG_F_LEAK,
> +};
> +
> static struct virtio_driver virtio_rng_driver = {
> + .feature_table = features,
> + .feature_table_size = ARRAY_SIZE(features),
> .driver.name = KBUILD_MODNAME,
> .driver.owner = THIS_MODULE,
> .id_table = id_table,
> diff --git a/include/uapi/linux/virtio_rng.h b/include/uapi/linux/virtio_rng.h
> index c4d5de896f0c..d9774951547e 100644
> --- a/include/uapi/linux/virtio_rng.h
> +++ b/include/uapi/linux/virtio_rng.h
> @@ -5,4 +5,7 @@
> #include <linux/virtio_ids.h>
> #include <linux/virtio_config.h>
>
> +/* The feature bitmap for virtio entropy device */
> +#define VIRTIO_RNG_F_LEAK 0
> +
> #endif /* _LINUX_VIRTIO_RNG_H */
> --
> 2.38.1
>
> Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja M-401234 . CIF B84570936

2023-01-20 13:07:21

by Babis Chalios

[permalink] [raw]

Subject: Re: [PATCH 1/2] virtio-rng: implement entropy leak feature

On 20/1/23 13:32, Michael S. Tsirkin wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On Thu, Jan 19, 2023 at 07:43:47PM +0100, Babis Chalios wrote:
>> Implement the virtio-rng feature that allows a guest driver to request
>> from the device to perform certain operations in the event of an
>> "entropy leak", such as when taking a VM snapshot or restoring a VM from
>> a snapshot. The guest can request one of two operations: (i) fill a
>> buffer with random bytes, or (ii) perform a memory copy between two
>> bytes.
>>
>> The feature is similar to Microsoft's Virtual Machine Generation ID and
>> it can be used to (1) avoid the race-condition that exists in our
>> current VMGENID implementation, between the time vcpus are resumed and
>> the ACPI notification is being handled and (2) provide mechanisms for
>> notifying user-space about snapshot-related events.
>>
>> This commit implements the protocol between guest and device.
>> Additionally, it makes sure there is always a request for random bytes
>> in the event of entropy leak in-flight. Once such an event is observed,
>> the driver feeds these bytes to as entropy using
>> `add_device_randomness`.
>>
>> Keep in mind that this commit does not solve the race-condition issue,
>> it adds fresh entropy whenever the driver handles the used buffer from
>> the fill-on-leak request. In order to close the race window, we need to
>> expose some API so that other kernel subsystems can request directly
>> notifications from the device.
>>
>> Signed-off-by: Babis Chalios <[email protected]>
>> ---
>> drivers/char/hw_random/virtio-rng.c | 200 +++++++++++++++++++++++++++-
>> include/uapi/linux/virtio_rng.h | 3 +
>> 2 files changed, 197 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
>> index a6f3a8a2aca6..389a091a8801 100644
>> --- a/drivers/char/hw_random/virtio-rng.c
>> +++ b/drivers/char/hw_random/virtio-rng.c
>> @@ -4,12 +4,14 @@
>> * Copyright (C) 2007, 2008 Rusty Russell IBM Corporation
>> */
>>
>> +#include "asm-generic/errno.h"
> Why are you pulling this in?
> If you really need errno the thing to include
> is linux/errno.h
>
> checkpatch has a rule to catch such cases:
>
> # warn if <asm/foo.h> is #included and <linux/foo.h> is available and includes
> # itself <asm/foo.h> (uses RAW line)
>
> did you run checkpatch?
>
You are right, this is wrong. I think my LSP client pulled this in and I
missed it because, strangely enough, I did ran
checkpatch but it didn't complain about it.

>> #include <linux/err.h>
>> #include <linux/hw_random.h>
>> #include <linux/scatterlist.h>
>> #include <linux/spinlock.h>
>> #include <linux/virtio.h>
>> #include <linux/virtio_rng.h>
>> +#include <linux/random.h>
>> #include <linux/module.h>
>> #include <linux/slab.h>
>>
>> @@ -18,6 +20,12 @@ static DEFINE_IDA(rng_index_ida);
>> struct virtrng_info {
>> struct hwrng hwrng;
>> struct virtqueue *vq;
>> + /* Leak queues */
>> + bool has_leakqs;
>> + struct virtqueue *leakq[2];
>> + spinlock_t lock;
>> + int active_leakq;
>> +
>> char name[25];
>> int index;
>> bool hwrng_register_done;
>> @@ -29,27 +37,159 @@ struct virtrng_info {
>> /* minimal size returned by rng_buffer_size() */
>> #if SMP_CACHE_BYTES < 32
>> u8 data[32];
>> + u8 leak_data[32];
>> #else
>> u8 data[SMP_CACHE_BYTES];
>> + u8 leak_data[SMP_CACHE_BYTES];
>> #endif
>> };
>>
>> +/* Swaps the queues and returns the new active leak queue. */
>> +static struct virtqueue *swap_leakqs(struct virtrng_info *vi)
>> +{
>> + vi->active_leakq = 1 - vi->active_leakq;
>> + return vi->leakq[vi->active_leakq];
>> +}
>> +
>> +static struct virtqueue *get_active_leakq(struct virtrng_info *vi)
>> +{
>> + return vi->leakq[vi->active_leakq];
>> +}
>> +
>> +int add_fill_on_leak_request(struct virtrng_info *vi, struct virtqueue *vq, void *data, size_t len)
>> +{
>> + struct scatterlist sg;
>> + int ret;
>> +
>> + sg_init_one(&sg, data, len);
>> + ret = virtqueue_add_inbuf(vq, &sg, 1, data, GFP_KERNEL);
>> + if (ret)
>> + goto err;
>> +
>> +err:
>> + return ret;
>> +}
>> +
>> +int virtrng_fill_on_leak(struct virtrng_info *vi, void *data, size_t len)
>> +{
>> + struct virtqueue *vq;
>> + unsigned long flags;
>> + int ret;
>> +
>> + if (!vi->has_leakqs)
>> + return -EOPNOTSUPP;
>> +
>> + spin_lock_irqsave(&vi->lock, flags);
>> +
>> + vq = get_active_leakq(vi);
>> + ret = add_fill_on_leak_request(vi, vq, data, len);
>> + if (ret)
>> + virtqueue_kick(vq);
>> +
>> + spin_unlock_irqrestore(&vi->lock, flags);
>> +
>> + return ret;
>> +}
>> +
>> +int add_copy_on_leak_request(struct virtrng_info *vi, struct virtqueue *vq,
>> + void *to, void *from, size_t len)
>> +{
>> + int ret;
>> + struct scatterlist out, in, *sgs[2];
>> +
>> + sg_init_one(&out, from, len);
>> + sgs[0] = &out;
>> + sg_init_one(&in, to, len);
>> + sgs[1] = &in;
>> +
>> + ret = virtqueue_add_sgs(vq, sgs, 1, 1, to, GFP_KERNEL);
>> + if (ret)
>> + goto err;
>> +
>> +err:
>> + return ret;
>> +}
>> +
>> +int virtrng_copy_on_leak(struct virtrng_info *vi, void *to, void *from, size_t len)
>> +{
>> + struct virtqueue *vq;
>> + unsigned long flags;
>> + int ret;
>> +
>> + if (!vi->has_leakqs)
>> + return -EOPNOTSUPP;
>> +
>> + spin_lock_irqsave(&vi->lock, flags);
>> +
>> + vq = get_active_leakq(vi);
>> + ret = add_copy_on_leak_request(vi, vq, to, from, len);
>> + if (ret)
>> + virtqueue_kick(vq);
>> +
>> + spin_unlock_irqrestore(&vi->lock, flags);
>> +
>> + return ret;
>> +}
>> +
>> +static void entropy_leak_detected(struct virtqueue *vq)
>> +{
>> + struct virtrng_info *vi = vq->vdev->priv;
>> + struct virtqueue *activeq;
>> + unsigned int len;
>> + unsigned long flags;
>> + void *buffer;
>> + bool kick_activeq = false;
>> +
>> + spin_lock_irqsave(&vi->lock, flags);
>> +
>> + activeq = get_active_leakq(vi);
>> + /* Drain all the used buffers from the queue */
>> + while ((buffer = virtqueue_get_buf(vq, &len)) != NULL) {
>> + if (vq == activeq) {
>> + pr_debug("%s: entropy leak detected!", vi->name);
>> + activeq = swap_leakqs(vi);
>> + }
>> +
>> + if (buffer == vi->leak_data) {
>> + add_device_randomness(vi->leak_data, sizeof(vi->leak_data));
>> +
>> + /* Ensure we always have a pending request for random bytes on entropy
>> + * leak. Do it here, after we have swapped leak queues, so it gets handled
>> + * with the next entropy leak event.
>> + */
>> + add_fill_on_leak_request(vi, activeq, vi->leak_data, sizeof(vi->leak_data));
>> + kick_activeq = true;
>> + }
>> + }
>> +
>> + if (kick_activeq)
>> + virtqueue_kick(activeq);
>> +
>> + spin_unlock_irqrestore(&vi->lock, flags);
>> +}
>> +
>> static void random_recv_done(struct virtqueue *vq)
>> {
>> struct virtrng_info *vi = vq->vdev->priv;
>> + unsigned long flags;
>>
>> + spin_lock_irqsave(&vi->lock, flags);
>> /* We can get spurious callbacks, e.g. shared IRQs + virtio_pci. */
>> if (!virtqueue_get_buf(vi->vq, &vi->data_avail))
>> - return;
>> + goto unlock;
>>
>> vi->data_idx = 0;
>>
>> complete(&vi->have_data);
>> +
>> +unlock:
>> + spin_unlock_irqrestore(&vi->lock, flags);
>> }
>>
>> static void request_entropy(struct virtrng_info *vi)
>> {
>> struct scatterlist sg;
>> + unsigned long flags;
>>
>> reinit_completion(&vi->have_data);
>> vi->data_avail = 0;
>> @@ -57,10 +197,12 @@ static void request_entropy(struct virtrng_info *vi)
>>
>> sg_init_one(&sg, vi->data, sizeof(vi->data));
>>
>> + spin_lock_irqsave(&vi->lock, flags);
>> /* There should always be room for one buffer. */
>> virtqueue_add_inbuf(vi->vq, &sg, 1, vi->data, GFP_KERNEL);
>>
>> virtqueue_kick(vi->vq);
>> + spin_unlock_irqrestore(&vi->lock, flags);
>> }
>>
>> static unsigned int copy_data(struct virtrng_info *vi, void *buf,
>> @@ -126,6 +268,40 @@ static void virtio_cleanup(struct hwrng *rng)
>> complete(&vi->have_data);
>> }
>>
>> +static int init_virtqueues(struct virtrng_info *vi, struct virtio_device *vdev)
>> +{
>> + int ret = -ENOMEM, total_vqs = 1;
>> + struct virtqueue *vqs[3];
>> + const char *names[3];
>> + vq_callback_t *callbacks[3];
>> +
>> + if (vi->has_leakqs)
>> + total_vqs = 3;
>> +
>> + callbacks[0] = random_recv_done;
>> + names[0] = "input";
>> + if (vi->has_leakqs) {
>> + callbacks[1] = entropy_leak_detected;
>> + names[1] = "leakq.1";
>> + callbacks[2] = entropy_leak_detected;
>> + names[2] = "leakq.2";
>> + }
>> +
>> + ret = virtio_find_vqs(vdev, total_vqs, vqs, callbacks, names, NULL);
>> + if (ret)
>> + goto err;
>> +
>> + vi->vq = vqs[0];
>> +
>> + if (vi->has_leakqs) {
>> + vi->leakq[0] = vqs[1];
>> + vi->leakq[1] = vqs[2];
>> + }
>> +
>> +err:
>> + return ret;
>> +}
>> +
>> static int probe_common(struct virtio_device *vdev)
>> {
>> int err, index;
>> @@ -152,18 +328,24 @@ static int probe_common(struct virtio_device *vdev)
>> };
>> vdev->priv = vi;
>>
>> - /* We expect a single virtqueue. */
>> - vi->vq = virtio_find_single_vq(vdev, random_recv_done, "input");
>> - if (IS_ERR(vi->vq)) {
>> - err = PTR_ERR(vi->vq);
>> - goto err_find;
>> + vi->has_leakqs = virtio_has_feature(vdev, VIRTIO_RNG_F_LEAK);
>> + if (vi->has_leakqs) {
>> + spin_lock_init(&vi->lock);
>> + vi->active_leakq = 0;
>> }
>>
>> + err = init_virtqueues(vi, vdev);
>> + if (err)
>> + goto err_find;
>> +
>> virtio_device_ready(vdev);
>>
>> /* we always have a pending entropy request */
>> request_entropy(vi);
>>
>> + /* we always have a fill_on_leak request pending */
>> + virtrng_fill_on_leak(vi, vi->leak_data, sizeof(vi->leak_data));
>> +
>> return 0;
>>
>> err_find:
>> @@ -246,7 +428,13 @@ static const struct virtio_device_id id_table[] = {
>> { 0 },
>> };
>>
>> +static unsigned int features[] = {
>> + VIRTIO_RNG_F_LEAK,
>> +};
>> +
>> static struct virtio_driver virtio_rng_driver = {
>> + .feature_table = features,
>> + .feature_table_size = ARRAY_SIZE(features),
>> .driver.name = KBUILD_MODNAME,
>> .driver.owner = THIS_MODULE,
>> .id_table = id_table,
>> diff --git a/include/uapi/linux/virtio_rng.h b/include/uapi/linux/virtio_rng.h
>> index c4d5de896f0c..d9774951547e 100644
>> --- a/include/uapi/linux/virtio_rng.h
>> +++ b/include/uapi/linux/virtio_rng.h
>> @@ -5,4 +5,7 @@
>> #include <linux/virtio_ids.h>
>> #include <linux/virtio_config.h>
>>
>> +/* The feature bitmap for virtio entropy device */
>> +#define VIRTIO_RNG_F_LEAK 0
>> +
>> #endif /* _LINUX_VIRTIO_RNG_H */
>> --
>> 2.38.1
>>
>> Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja M-401234 . CIF B84570936

Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja M-401234 . CIF B84570936

2023-01-20 14:29:34

by Jason A. Donenfeld

[permalink] [raw]

Subject: Re: [PATCH 0/2] [RFC] virtio-rng entropy leak reporting feature

Hi Babis,

As I mentioned to you privately this week, I'm about to be out of town,
so I won't be able to look at this until I'm back in a few weeks. I
appreciate your patience.

But as a cursory look, I'm happy that you've written the hardware-side
code for this. That's a great starting point. The plumbing is not so
nice, though. This needs to be integrated more closely with random.c
itself, similar to how vmgenid works.

When I'm back in a few weeks, I'll see if I can either write a
description of what I have in mind, or simply integrate the useful
hardware work here into an expanded patch series.

[Please don't merge anything for now.]

Jason

2023-01-20 14:30:05

by Babis Chalios

[permalink] [raw]

Subject: Re: [PATCH 0/2] [RFC] virtio-rng entropy leak reporting feature

On 1/20/23 3:20 PM, "Jason A. Donenfeld" <[email protected]> wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> Hi Babis,
>
> As I mentioned to you privately this week, I'm about to be out of town,
> so I won't be able to look at this until I'm back in a few weeks. I
> appreciate your patience.
>
> But as a cursory look, I'm happy that you've written the hardware-side
> code for this. That's a great starting point. The plumbing is not so
> nice, though. This needs to be integrated more closely with random.c
> itself, similar to how vmgenid works.
>
> When I'm back in a few weeks, I'll see if I can either write a
> description of what I have in mind, or simply integrate the useful
> hardware work here into an expanded patch series.
>
> [Please don't merge anything for now.]
>
> Jason
>

Hey Jason,

I agree that the plumbing with random.c needs work. That's why this is an RFC! That's the kind of input I'm looking for.
As I mention in the cover letter, IMHO, we need to give to random.c (and other parts of the kernel that they might need it)
an API to directly add buffers in the queue, so that we avoid the race-condition here.

Any ideas on that front are more than welcome and looking forward to them.

Cheers,
Babis
Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja M-401234 . CIF B84570936

2023-01-20 15:44:31

by Elliott, Robert (Servers)

[permalink] [raw]

Subject: RE: [PATCH 1/2] virtio-rng: implement entropy leak feature

> > +#include "asm-generic/errno.h"
>
> Why are you pulling this in?
> If you really need errno the thing to include
> is linux/errno.h
>
> checkpatch has a rule to catch such cases:
>
> # warn if <asm/foo.h> is #included and <linux/foo.h> is available and includes
> # itself <asm/foo.h> (uses RAW line)

It's only looking for asm, not variations like asm-generic.