virtio_ring: Fix two use after free bugs
authorDan Carpenter <dan.carpenter@oracle.com>
Fri, 4 Dec 2020 14:23:36 +0000 (17:23 +0300)
committerMichael S. Tsirkin <mst@redhat.com>
Fri, 18 Dec 2020 21:14:31 +0000 (16:14 -0500)
The "vq" struct is added to the "vdev->vqs" list prematurely.  If we
encounter an error later in the function then the "vq" is freed, but
since it is still on the list that could lead to a use after free bug.

Fixes: cbeedb72b97a ("virtio_ring: allocate desc state for split ring separately")
Reported-by: Robert Buhren <robert.buhren@sect.tu-berlin.de>
Reported-by: Felicitas Hetzelt <file@sect.tu-berlin.de>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/r/X8pGaG/zkI3jk8mk@mwanda
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
drivers/virtio/virtio_ring.c

index 924b6b8..71e16b5 100644 (file)
@@ -1608,7 +1608,6 @@ static struct virtqueue *vring_create_virtqueue_packed(
        vq->num_added = 0;
        vq->packed_ring = true;
        vq->use_dma_api = vring_use_dma_api(vdev);
-       list_add_tail(&vq->vq.list, &vdev->vqs);
 #ifdef DEBUG
        vq->in_use = false;
        vq->last_add_time_valid = false;
@@ -1669,6 +1668,7 @@ static struct virtqueue *vring_create_virtqueue_packed(
                        cpu_to_le16(vq->packed.event_flags_shadow);
        }
 
+       list_add_tail(&vq->vq.list, &vdev->vqs);
        return &vq->vq;
 
 err_desc_extra:
@@ -2085,7 +2085,6 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
        vq->last_used_idx = 0;
        vq->num_added = 0;
        vq->use_dma_api = vring_use_dma_api(vdev);
-       list_add_tail(&vq->vq.list, &vdev->vqs);
 #ifdef DEBUG
        vq->in_use = false;
        vq->last_add_time_valid = false;
@@ -2127,6 +2126,7 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
        memset(vq->split.desc_state, 0, vring.num *
                        sizeof(struct vring_desc_state_split));
 
+       list_add_tail(&vq->vq.list, &vdev->vqs);
        return &vq->vq;
 }
 EXPORT_SYMBOL_GPL(__vring_new_virtqueue);