* [PATCH] virtio-net: fix use after unmap/free for sg
@ 2021-09-02 5:44 Jason Wang
2021-09-09 3:10 ` Jason Wang
0 siblings, 1 reply; 2+ messages in thread
From: Jason Wang @ 2021-09-02 5:44 UTC (permalink / raw)
To: mst, jasowang
Cc: mcascell, Alexander Bulekov, qemu-devel, qemu-stable,
darren.kenny, pj.pandit
When mergeable buffer is enabled, we try to set the num_buffers after
the virtqueue elem has been unmapped. This will lead several issues,
E.g a use after free when the descriptor has an address which belongs
to the non direct access region. In this case we use bounce buffer
that is allocated during address_space_map() and freed during
address_space_unmap().
Fixing this by storing the elems temporarily in an array and delay the
unmap after we set the the num_buffers.
This addresses CVE-2021-3748.
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Fixes: fbe78f4f55c6 ("virtio-net support")
Cc: qemu-stable@nongnu.org
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/virtio-net.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 16d20cdee5..f205331dcf 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1746,10 +1746,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
VirtIONet *n = qemu_get_nic_opaque(nc);
VirtIONetQueue *q = virtio_net_get_subqueue(nc);
VirtIODevice *vdev = VIRTIO_DEVICE(n);
+ VirtQueueElement *elems[VIRTQUEUE_MAX_SIZE];
+ size_t lens[VIRTQUEUE_MAX_SIZE];
struct iovec mhdr_sg[VIRTQUEUE_MAX_SIZE];
struct virtio_net_hdr_mrg_rxbuf mhdr;
unsigned mhdr_cnt = 0;
- size_t offset, i, guest_offset;
+ size_t offset, i, guest_offset, j;
+ ssize_t err;
if (!virtio_net_can_receive(nc)) {
return -1;
@@ -1780,6 +1783,12 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
total = 0;
+ if (i == VIRTQUEUE_MAX_SIZE) {
+ virtio_error(vdev, "virtio-net unexpected long buffer chain");
+ err = size;
+ goto err;
+ }
+
elem = virtqueue_pop(q->rx_vq, sizeof(VirtQueueElement));
if (!elem) {
if (i) {
@@ -1791,7 +1800,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
n->guest_hdr_len, n->host_hdr_len,
vdev->guest_features);
}
- return -1;
+ err = -1;
+ goto err;
}
if (elem->in_num < 1) {
@@ -1799,7 +1809,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
"virtio-net receive queue contains no in buffers");
virtqueue_detach_element(q->rx_vq, elem, 0);
g_free(elem);
- return -1;
+ err = -1;
+ goto err;
}
sg = elem->in_sg;
@@ -1836,12 +1847,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
if (!n->mergeable_rx_bufs && offset < size) {
virtqueue_unpop(q->rx_vq, elem, total);
g_free(elem);
- return size;
+ err = size;
+ goto err;
}
- /* signal other side */
- virtqueue_fill(q->rx_vq, elem, total, i++);
- g_free(elem);
+ elems[i] = elem;
+ lens[i] = total;
+ i++;
}
if (mhdr_cnt) {
@@ -1851,10 +1863,23 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
&mhdr.num_buffers, sizeof mhdr.num_buffers);
}
+ for (j = 0; j < i; j++) {
+ /* signal other side */
+ virtqueue_fill(q->rx_vq, elems[j], lens[j], j);
+ g_free(elems[j]);
+ }
+
virtqueue_flush(q->rx_vq, i);
virtio_notify(vdev, q->rx_vq);
return size;
+
+err:
+ for (j = 0; j < i; j++) {
+ g_free(elems[j]);
+ }
+
+ return err;
}
static ssize_t virtio_net_do_receive(NetClientState *nc, const uint8_t *buf,
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] virtio-net: fix use after unmap/free for sg
2021-09-02 5:44 [PATCH] virtio-net: fix use after unmap/free for sg Jason Wang
@ 2021-09-09 3:10 ` Jason Wang
0 siblings, 0 replies; 2+ messages in thread
From: Jason Wang @ 2021-09-09 3:10 UTC (permalink / raw)
To: mst
Cc: mcascell, Alexander Bulekov, qemu-devel, qemu-stable,
darren.kenny, pj.pandit
在 2021/9/2 下午1:44, Jason Wang 写道:
> When mergeable buffer is enabled, we try to set the num_buffers after
> the virtqueue elem has been unmapped. This will lead several issues,
> E.g a use after free when the descriptor has an address which belongs
> to the non direct access region. In this case we use bounce buffer
> that is allocated during address_space_map() and freed during
> address_space_unmap().
>
> Fixing this by storing the elems temporarily in an array and delay the
> unmap after we set the the num_buffers.
>
> This addresses CVE-2021-3748.
>
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Fixes: fbe78f4f55c6 ("virtio-net support")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied.
Thanks
> ---
> hw/net/virtio-net.c | 39 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 16d20cdee5..f205331dcf 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1746,10 +1746,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> VirtIONet *n = qemu_get_nic_opaque(nc);
> VirtIONetQueue *q = virtio_net_get_subqueue(nc);
> VirtIODevice *vdev = VIRTIO_DEVICE(n);
> + VirtQueueElement *elems[VIRTQUEUE_MAX_SIZE];
> + size_t lens[VIRTQUEUE_MAX_SIZE];
> struct iovec mhdr_sg[VIRTQUEUE_MAX_SIZE];
> struct virtio_net_hdr_mrg_rxbuf mhdr;
> unsigned mhdr_cnt = 0;
> - size_t offset, i, guest_offset;
> + size_t offset, i, guest_offset, j;
> + ssize_t err;
>
> if (!virtio_net_can_receive(nc)) {
> return -1;
> @@ -1780,6 +1783,12 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>
> total = 0;
>
> + if (i == VIRTQUEUE_MAX_SIZE) {
> + virtio_error(vdev, "virtio-net unexpected long buffer chain");
> + err = size;
> + goto err;
> + }
> +
> elem = virtqueue_pop(q->rx_vq, sizeof(VirtQueueElement));
> if (!elem) {
> if (i) {
> @@ -1791,7 +1800,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> n->guest_hdr_len, n->host_hdr_len,
> vdev->guest_features);
> }
> - return -1;
> + err = -1;
> + goto err;
> }
>
> if (elem->in_num < 1) {
> @@ -1799,7 +1809,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> "virtio-net receive queue contains no in buffers");
> virtqueue_detach_element(q->rx_vq, elem, 0);
> g_free(elem);
> - return -1;
> + err = -1;
> + goto err;
> }
>
> sg = elem->in_sg;
> @@ -1836,12 +1847,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> if (!n->mergeable_rx_bufs && offset < size) {
> virtqueue_unpop(q->rx_vq, elem, total);
> g_free(elem);
> - return size;
> + err = size;
> + goto err;
> }
>
> - /* signal other side */
> - virtqueue_fill(q->rx_vq, elem, total, i++);
> - g_free(elem);
> + elems[i] = elem;
> + lens[i] = total;
> + i++;
> }
>
> if (mhdr_cnt) {
> @@ -1851,10 +1863,23 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> &mhdr.num_buffers, sizeof mhdr.num_buffers);
> }
>
> + for (j = 0; j < i; j++) {
> + /* signal other side */
> + virtqueue_fill(q->rx_vq, elems[j], lens[j], j);
> + g_free(elems[j]);
> + }
> +
> virtqueue_flush(q->rx_vq, i);
> virtio_notify(vdev, q->rx_vq);
>
> return size;
> +
> +err:
> + for (j = 0; j < i; j++) {
> + g_free(elems[j]);
> + }
> +
> + return err;
> }
>
> static ssize_t virtio_net_do_receive(NetClientState *nc, const uint8_t *buf,
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-09-09 3:12 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-02 5:44 [PATCH] virtio-net: fix use after unmap/free for sg Jason Wang
2021-09-09 3:10 ` Jason Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).