Commit bedd7e93d0196 ("virtio-net: fix use after unmap/free for sg")

tries to fix the use after free of the sg by caching the virtqueue

elements in an array and unmap them at once after receiving the

packets, But it forgot to unmap the cached elements on error which

will lead to leaking of mapping and other unexpected results.

Fixing this by detaching the cached elements on error. This addresses

CVE-2022-26353.

Reported-by: Victor Tom <vv474172261@gmail.com>

Cc: qemu-stable@nongnu.org

Fixes: CVE-2022-26353

Fixes: bedd7e93d0196 ("virtio-net: fix use after unmap/free for sg")

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Signed-off-by: Jason Wang <jasowang@redhat.com>

---

hw/net/virtio-net.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/net/virtio-net.c

+++ b/hw/net/virtio-net.c

@@ -XXX,XX +XXX,XX @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,

err:

for (j = 0; j < i; j++) {

+ virtqueue_detach_element(q->rx_vq, elems[j], lens[j]);

g_free(elems[j]);

}

--

2.7.4

From: Eugenio Pérez <eperezma@redhat.com>

Vhost shadow virtqueue (SVQ) is an intermediate jump for virtqueue

notifications and buffers, allowing qemu to track them. While qemu is

forwarding the buffers and virtqueue changes, it is able to commit the

memory it's being dirtied, the same way regular qemu's VirtIO devices

do.

This commit only exposes basic SVQ allocation and free. Next patches of

the series add functionality like notifications and buffers forwarding.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Signed-off-by: Jason Wang <jasowang@redhat.com>

---

hw/virtio/meson.build | 2 +-

hw/virtio/vhost-shadow-virtqueue.c | 62 ++++++++++++++++++++++++++++++++++++++

hw/virtio/vhost-shadow-virtqueue.h | 28 +++++++++++++++++

3 files changed, 91 insertions(+), 1 deletion(-)

create mode 100644 hw/virtio/vhost-shadow-virtqueue.c

create mode 100644 hw/virtio/vhost-shadow-virtqueue.h

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/meson.build

+++ b/hw/virtio/meson.build

@@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('vhost-stub.c'))

virtio_ss = ss.source_set()

virtio_ss.add(files('virtio.c'))

-virtio_ss.add(when: 'CONFIG_VHOST', if_true: files('vhost.c', 'vhost-backend.c'))

+virtio_ss.add(when: 'CONFIG_VHOST', if_true: files('vhost.c', 'vhost-backend.c', 'vhost-shadow-virtqueue.c'))

virtio_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user.c'))

virtio_ss.add(when: 'CONFIG_VHOST_VDPA', if_true: files('vhost-vdpa.c'))

virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon.c'))

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/hw/virtio/vhost-shadow-virtqueue.c

@@ -XXX,XX +XXX,XX @@

+/*

+ * vhost shadow virtqueue

+ *

+ * SPDX-FileCopyrightText: Red Hat, Inc. 2021

+ * SPDX-FileContributor: Author: Eugenio Pérez <eperezma@redhat.com>

+ *

+ * SPDX-License-Identifier: GPL-2.0-or-later

+ */

+

+#include "qemu/osdep.h"

+#include "hw/virtio/vhost-shadow-virtqueue.h"

+

+#include "qemu/error-report.h"

+

+/**

+ * Creates vhost shadow virtqueue, and instructs the vhost device to use the

+ * shadow methods and file descriptors.

+ *

+ * Returns the new virtqueue or NULL.

+ *

+ * In case of error, reason is reported through error_report.

+ */

+VhostShadowVirtqueue *vhost_svq_new(void)

+{

+ g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);

+ int r;

+

+ r = event_notifier_init(&svq->hdev_kick, 0);

+ if (r != 0) {

+ error_report("Couldn't create kick event notifier: %s (%d)",

+ g_strerror(errno), errno);

+ goto err_init_hdev_kick;

+ }

+

+ r = event_notifier_init(&svq->hdev_call, 0);

+ if (r != 0) {

+ error_report("Couldn't create call event notifier: %s (%d)",

+ g_strerror(errno), errno);

+ goto err_init_hdev_call;

+ }

+

+ return g_steal_pointer(&svq);

+

+err_init_hdev_call:

+ event_notifier_cleanup(&svq->hdev_kick);

+

+err_init_hdev_kick:

+ return NULL;

+}

+

+/**

+ * Free the resources of the shadow virtqueue.

+ *

+ * @pvq: gpointer to SVQ so it can be used by autofree functions.

+ */

+void vhost_svq_free(gpointer pvq)

+{

+ VhostShadowVirtqueue *vq = pvq;

+ event_notifier_cleanup(&vq->hdev_kick);

+ event_notifier_cleanup(&vq->hdev_call);

+ g_free(vq);

+}

diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/hw/virtio/vhost-shadow-virtqueue.h

@@ -XXX,XX +XXX,XX @@

+/*

+ * vhost shadow virtqueue

+ *

+ * SPDX-FileCopyrightText: Red Hat, Inc. 2021

+ * SPDX-FileContributor: Author: Eugenio Pérez <eperezma@redhat.com>

+ *

+ * SPDX-License-Identifier: GPL-2.0-or-later

+ */

+

+#ifndef VHOST_SHADOW_VIRTQUEUE_H

+#define VHOST_SHADOW_VIRTQUEUE_H

+

+#include "qemu/event_notifier.h"

+

+/* Shadow virtqueue to relay notifications */

+typedef struct VhostShadowVirtqueue {

+ /* Shadow kick notifier, sent to vhost */

+ EventNotifier hdev_kick;

+ /* Shadow call notifier, sent to vhost */

+ EventNotifier hdev_call;

+} VhostShadowVirtqueue;

+

+VhostShadowVirtqueue *vhost_svq_new(void);

+

+void vhost_svq_free(gpointer vq);

+G_DEFINE_AUTOPTR_CLEANUP_FUNC(VhostShadowVirtqueue, vhost_svq_free);

+

+#endif

--

2.7.4

This will make qemu aware of the device used buffers, allowing it to

write the guest memory with its contents if needed.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

From: Eugenio Pérez <eperezma@redhat.com>

This allows SVQ to negotiate features with the guest and the device. For

the device, SVQ is a driver. While this function bypasses all

non-transport features, it needs to disable the features that SVQ does

not support when forwarding buffers. This includes packed vq layout,

indirect descriptors or event idx.

Future changes can add support to offer more features to the guest,

since the use of VirtQueue gives this for free. This is left out at the

moment for simplicity.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Signed-off-by: Jason Wang <jasowang@redhat.com>

---

hw/virtio/vhost-shadow-virtqueue.c | 44 ++++++++++++++++++++++++++++++++++++++

hw/virtio/vhost-shadow-virtqueue.h | 2 ++

hw/virtio/vhost-vdpa.c | 15 +++++++++++++

3 files changed, 61 insertions(+)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/vhost-shadow-virtqueue.c

+++ b/hw/virtio/vhost-shadow-virtqueue.c

@@ -XXX,XX +XXX,XX @@

#include "hw/virtio/vhost-shadow-virtqueue.h"

#include "qemu/error-report.h"

+#include "qapi/error.h"

#include "qemu/main-loop.h"

#include "linux-headers/linux/vhost.h"

/**

+ * Validate the transport device features that both guests can use with the SVQ

+ * and SVQs can use with the device.

+ *

+ * @dev_features: The features

+ * @errp: Error pointer

+ */

+bool vhost_svq_valid_features(uint64_t features, Error **errp)

+{

+ bool ok = true;

+ uint64_t svq_features = features;

+

+ for (uint64_t b = VIRTIO_TRANSPORT_F_START; b <= VIRTIO_TRANSPORT_F_END;

+ ++b) {

+ switch (b) {

+ case VIRTIO_F_ANY_LAYOUT:

+ continue;

+

+ case VIRTIO_F_ACCESS_PLATFORM:

+ /* SVQ trust in the host's IOMMU to translate addresses */

+ case VIRTIO_F_VERSION_1:

+ /* SVQ trust that the guest vring is little endian */

+ if (!(svq_features & BIT_ULL(b))) {

+ svq_features |= BIT_ULL(b);

+ ok = false;

+ }

+ continue;

+

+ default:

+ if (svq_features & BIT_ULL(b)) {

+ svq_features &= ~BIT_ULL(b);

+ ok = false;

+ }

+ }

+ }

+

+ if (!ok) {

+ error_setg(errp, "SVQ Invalid device feature flags, offer: 0x%"PRIx64

+ ", ok: 0x%"PRIx64, features, svq_features);

+ }

+ return ok;

+}

+

+/**

* Forward guest notifications.

*

* @n: guest kick event notifier, the one that guest set to notify svq.

diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/vhost-shadow-virtqueue.h

+++ b/hw/virtio/vhost-shadow-virtqueue.h

@@ -XXX,XX +XXX,XX @@ typedef struct VhostShadowVirtqueue {

EventNotifier svq_call;

} VhostShadowVirtqueue;

+bool vhost_svq_valid_features(uint64_t features, Error **errp);

+

void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);

void vhost_svq_set_svq_call_fd(VhostShadowVirtqueue *svq, int call_fd);

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/vhost-vdpa.c

+++ b/hw/virtio/vhost-vdpa.c

@@ -XXX,XX +XXX,XX @@ static int vhost_vdpa_init_svq(struct vhost_dev *hdev, struct vhost_vdpa *v,

Error **errp)

{

g_autoptr(GPtrArray) shadow_vqs = NULL;

+ uint64_t dev_features, svq_features;

+ int r;

+ bool ok;

if (!v->shadow_vqs_enabled) {

return 0;

}

+ r = hdev->vhost_ops->vhost_get_features(hdev, &dev_features);

+ if (r != 0) {

+ error_setg_errno(errp, -r, "Can't get vdpa device features");

+ return r;

+ }

+

+ svq_features = dev_features;

+ ok = vhost_svq_valid_features(svq_features, errp);

+ if (unlikely(!ok)) {

+ return -1;

+ }

+

shadow_vqs = g_ptr_array_new_full(hdev->nvqs, vhost_svq_free);

for (unsigned n = 0; n < hdev->nvqs; ++n) {

g_autoptr(VhostShadowVirtqueue) svq = vhost_svq_new();

--

2.7.4

From: Eugenio Pérez <eperezma@redhat.com>

It reports the shadow virtqueue address from qemu virtual address space.

Since this will be different from the guest's vaddr, but the device can

access it, SVQ takes special care about its alignment & lack of garbage

data. It assumes that IOMMU will work in host_page_size ranges for that.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Signed-off-by: Jason Wang <jasowang@redhat.com>

---

hw/virtio/vhost-shadow-virtqueue.c | 29 +++++++++++++++++++++++++++++

hw/virtio/vhost-shadow-virtqueue.h | 9 +++++++++

2 files changed, 38 insertions(+)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/vhost-shadow-virtqueue.c

+++ b/hw/virtio/vhost-shadow-virtqueue.c

@@ -XXX,XX +XXX,XX @@ void vhost_svq_set_svq_call_fd(VhostShadowVirtqueue *svq, int call_fd)

}

/**

+ * Get the shadow vq vring address.

+ * @svq: Shadow virtqueue

+ * @addr: Destination to store address

+ */

+void vhost_svq_get_vring_addr(const VhostShadowVirtqueue *svq,

+ struct vhost_vring_addr *addr)

+{

+ addr->desc_user_addr = (uint64_t)(intptr_t)svq->vring.desc;

+ addr->avail_user_addr = (uint64_t)(intptr_t)svq->vring.avail;

+ addr->used_user_addr = (uint64_t)(intptr_t)svq->vring.used;

+}

+

+size_t vhost_svq_driver_area_size(const VhostShadowVirtqueue *svq)

+{

+ size_t desc_size = sizeof(vring_desc_t) * svq->vring.num;

+ size_t avail_size = offsetof(vring_avail_t, ring) +

+ sizeof(uint16_t) * svq->vring.num;

+

+ return ROUND_UP(desc_size + avail_size, qemu_real_host_page_size);

+}

+

+size_t vhost_svq_device_area_size(const VhostShadowVirtqueue *svq)

+{

+ size_t used_size = offsetof(vring_used_t, ring) +

+ sizeof(vring_used_elem_t) * svq->vring.num;

+ return ROUND_UP(used_size, qemu_real_host_page_size);

+}

+

+/**

* Set a new file descriptor for the guest to kick the SVQ and notify for avail

*

* @svq: The svq

diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/vhost-shadow-virtqueue.h

+++ b/hw/virtio/vhost-shadow-virtqueue.h

@@ -XXX,XX +XXX,XX @@

#define VHOST_SHADOW_VIRTQUEUE_H

#include "qemu/event_notifier.h"

+#include "hw/virtio/virtio.h"

+#include "standard-headers/linux/vhost_types.h"

/* Shadow virtqueue to relay notifications */

typedef struct VhostShadowVirtqueue {

+ /* Shadow vring */

+ struct vring vring;

+

/* Shadow kick notifier, sent to vhost */

EventNotifier hdev_kick;

/* Shadow call notifier, sent to vhost */

@@ -XXX,XX +XXX,XX @@ bool vhost_svq_valid_features(uint64_t features, Error **errp);

void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);

void vhost_svq_set_svq_call_fd(VhostShadowVirtqueue *svq, int call_fd);

+void vhost_svq_get_vring_addr(const VhostShadowVirtqueue *svq,

+ struct vhost_vring_addr *addr);

+size_t vhost_svq_driver_area_size(const VhostShadowVirtqueue *svq);

+size_t vhost_svq_device_area_size(const VhostShadowVirtqueue *svq);

void vhost_svq_stop(VhostShadowVirtqueue *svq);

--

2.7.4

First half of the buffers forwarding part, preparing vhost-vdpa

callbacks to SVQ to offer it. QEMU cannot enable it at this moment, so

this is effectively dead code at the moment, but it helps to reduce

patch size.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>