Re: [PATCH] virtio-net: fix use after unmap/free for sg

[Jason Wang]

在 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,