Commit 4eae2a65 authored by Ladi Prosek's avatar Ladi Prosek Committed by Michael S. Tsirkin
Browse files

balloon: fix segfault and harden the stats queue 



The segfault here is triggered by the driver notifying the stats queue
twice after adding a buffer to it. This effectively resets stats_vq_elem
back to NULL and QEMU crashes on the next stats timer tick in
balloon_stats_poll_cb.

This is a regression introduced in 51b19ebe, although admittedly
the device assumed too much about the stats queue protocol even before
that commit. This commit adds a few more checks and ensures that the one
stats buffer gets deallocated on device reset.

Cc: qemu-stable@nongnu.org
Signed-off-by: default avatarLadi Prosek <lprosek@redhat.com>
Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
parent f2035491

hw/virtio/virtio-balloon.c

+22 −2
Original line number Diff line number Diff line
@@ -101,7 +101,7 @@ static void balloon_stats_poll_cb(void *opaque) 
    VirtIOBalloon *s = opaque;

    VirtIODevice *vdev = VIRTIO_DEVICE(s);



    if (!balloon_stats_supported(s)) {

    if (s->stats_vq_elem == NULL || !balloon_stats_supported(s)) {

        /* re-schedule */

        balloon_stats_change_timer(s, s->stats_poll_interval);

        return;
@@ -258,11 +258,20 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq) 
    size_t offset = 0;

    qemu_timeval tv;



    s->stats_vq_elem = elem = virtqueue_pop(vq, sizeof(VirtQueueElement));

    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));

    if (!elem) {

        goto out;

    }



    if (s->stats_vq_elem != NULL) {

        /* This should never happen if the driver follows the spec. */

        virtqueue_push(vq, s->stats_vq_elem, 0);

        virtio_notify(vdev, vq);

        g_free(s->stats_vq_elem);

    }



    s->stats_vq_elem = elem;



    /* Initialize the stats to get rid of any stale values.  This is only

     * needed to handle the case where a guest supports fewer stats than it

     * used to (ie. it has booted into an old kernel).
@@ -458,6 +467,16 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp) 
    virtio_cleanup(vdev);

}



static void virtio_balloon_device_reset(VirtIODevice *vdev)

{

    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);



    if (s->stats_vq_elem != NULL) {

        g_free(s->stats_vq_elem);

        s->stats_vq_elem = NULL;

    }

}



static void virtio_balloon_instance_init(Object *obj)

{

    VirtIOBalloon *s = VIRTIO_BALLOON(obj);
@@ -486,6 +505,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data) 
    set_bit(DEVICE_CATEGORY_MISC, dc->categories);

    vdc->realize = virtio_balloon_device_realize;

    vdc->unrealize = virtio_balloon_device_unrealize;

    vdc->reset = virtio_balloon_device_reset;

    vdc->get_config = virtio_balloon_get_config;

    vdc->set_config = virtio_balloon_set_config;

    vdc->get_features = virtio_balloon_get_features;