[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio: guard vring access when setting notific
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [PATCH] virtio: guard vring access when setting notification |
Date: |
Tue, 28 Feb 2017 16:29:13 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 02/28/2017 04:24 PM, Cornelia Huck wrote:
> Switching to vring caches exposed an existing bug in
> virtio_queue_set_notification(): We can't access vring structures
> if they have not been set up yet. This may happen, for example,
> for virtio-blk devices with multiple queues: The code will try to
> switch notifiers for every queue, but the guest may have only set up
> a subset of them.
>
> Fix this by (1) guarding access to the vring memory by checking
> for vring.desc and (2) triggering an update to the vring flags
> for consistency with the configured notification state once the
> queue is actually configured.
>
> Signed-off-by: Cornelia Huck <address@hidden>
Together with Paolos patch this fixes my issue.
Tested-by: Christian Borntraeger <address@hidden>
> ---
> hw/virtio/virtio.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index e487e36..d2ecd64 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -284,10 +284,11 @@ static inline void vring_set_avail_event(VirtQueue *vq,
> uint16_t val)
> virtio_stw_phys_cached(vq->vdev, &caches->used, pa, val);
> }
>
> -void virtio_queue_set_notification(VirtQueue *vq, int enable)
> +static void vring_set_notification(VirtQueue *vq, int enable)
> {
> - vq->notification = enable;
> -
> + if (!vq->vring.desc) {
> + return;
> + }
> rcu_read_lock();
> if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
> vring_set_avail_event(vq, vring_avail_idx(vq));
> @@ -303,6 +304,13 @@ void virtio_queue_set_notification(VirtQueue *vq, int
> enable)
> rcu_read_unlock();
> }
>
> +void virtio_queue_set_notification(VirtQueue *vq, int enable)
> +{
> + vq->notification = enable;
> +
> + vring_set_notification(vq, enable);
> +}
> +
> int virtio_queue_ready(VirtQueue *vq)
> {
> return vq->vring.avail != 0;
> @@ -1348,6 +1356,7 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n,
> hwaddr addr)
> {
> vdev->vq[n].vring.desc = addr;
> virtio_queue_update_rings(vdev, n);
> + vring_set_notification(&vdev->vq[n], vdev->vq[n].notification);
> }
>
> hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
> @@ -1362,6 +1371,7 @@ void virtio_queue_set_rings(VirtIODevice *vdev, int n,
> hwaddr desc,
> vdev->vq[n].vring.avail = avail;
> vdev->vq[n].vring.used = used;
> virtio_init_region_cache(vdev, n);
> + vring_set_notification(&vdev->vq[n], vdev->vq[n].notification);
> }
>
> void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
>