619821
From 1f177df6a47fb1e2961067a50e005efad52595cc Mon Sep 17 00:00:00 2001
4f5da8
From: Ladi Prosek <lprosek@redhat.com>
4f5da8
Date: Wed, 5 Oct 2016 17:22:26 +0200
4f5da8
Subject: [PATCH 4/8] balloon: fix segfault and harden the stats queue
4f5da8
4f5da8
RH-Author: Ladi Prosek <lprosek@redhat.com>
4f5da8
Message-id: <1475666548-9186-5-git-send-email-lprosek@redhat.com>
4f5da8
Patchwork-id: 72483
4f5da8
O-Subject: [RHEL-7.4 qemu-kvm v2 PATCH 4/6] balloon: fix segfault and harden the stats queue
619821
Bugzilla: 1377968
4f5da8
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
4f5da8
RH-Acked-by: Michael S. Tsirkin <mst@redhat.com>
4f5da8
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
4f5da8
4f5da8
The segfault here is triggered by the driver notifying the stats queue
4f5da8
twice after adding a buffer to it. This effectively resets stats_vq_elem
4f5da8
back to NULL and QEMU crashes on the next stats timer tick in
4f5da8
balloon_stats_poll_cb.
4f5da8
4f5da8
This is a regression introduced in 51b19ebe4320f3dc, although admittedly
4f5da8
the device assumed too much about the stats queue protocol even before
4f5da8
that commit. This commit adds a few more checks and ensures that the one
4f5da8
stats buffer gets deallocated on device reset.
4f5da8
4f5da8
Cc: qemu-stable@nongnu.org
4f5da8
Signed-off-by: Ladi Prosek <lprosek@redhat.com>
4f5da8
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
4f5da8
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
4f5da8
(cherry picked from commit 4eae2a657d1ff5ada56eb9b4966eae0eff333b0b)
4f5da8
Signed-off-by: Ladi Prosek <lprosek@redhat.com>
4f5da8
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
4f5da8
4f5da8
Conflicts:
4f5da8
  * 1.5.3 does not return pointers from virtqueue_pop so only the
4f5da8
    "harden the stats queue" part of the upstream patch description
4f5da8
    applies
4f5da8
  * a new field stats_vq_elem_pending is introduced to keep track
4f5da8
    of the state of stats_vq_elem in lieu of its nullness upstream
4f5da8
  * virtio_balloon_device_reset only resets stats_vq_elem_pending
4f5da8
    because there is nothing to free
4f5da8
---
4f5da8
 hw/virtio/virtio-balloon.c         | 27 +++++++++++++++++++++++----
4f5da8
 include/hw/virtio/virtio-balloon.h |  1 +
4f5da8
 2 files changed, 24 insertions(+), 4 deletions(-)
4f5da8
4f5da8
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
4f5da8
index 016dc60..17b3029 100644
4f5da8
--- a/hw/virtio/virtio-balloon.c
4f5da8
+++ b/hw/virtio/virtio-balloon.c
4f5da8
@@ -95,13 +95,14 @@ static void balloon_stats_poll_cb(void *opaque)
4f5da8
     VirtIOBalloon *s = opaque;
4f5da8
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
4f5da8
 
4f5da8
-    if (!balloon_stats_supported(s)) {
4f5da8
+    if (!s->stats_vq_elem_pending || !balloon_stats_supported(s)) {
4f5da8
         /* re-schedule */
4f5da8
         balloon_stats_change_timer(s, s->stats_poll_interval);
4f5da8
         return;
4f5da8
     }
4f5da8
 
4f5da8
     virtqueue_push(s->svq, &s->stats_vq_elem, s->stats_vq_offset);
4f5da8
+    s->stats_vq_elem_pending = false;
4f5da8
     virtio_notify(vdev, s->svq);
4f5da8
 }
4f5da8
 
4f5da8
@@ -220,14 +221,22 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
4f5da8
 static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
4f5da8
 {
4f5da8
     VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
4f5da8
-    VirtQueueElement *elem = &s->stats_vq_elem;
4f5da8
+    VirtQueueElement elem;
4f5da8
     VirtIOBalloonStat stat;
4f5da8
     size_t offset = 0;
4f5da8
     qemu_timeval tv;
4f5da8
 
4f5da8
-    if (!virtqueue_pop(vq, elem)) {
4f5da8
+    if (!virtqueue_pop(vq, &elem)) {
4f5da8
         goto out;
4f5da8
     }
4f5da8
+    if (s->stats_vq_elem_pending) {
4f5da8
+        /* This should never happen if the driver follows the spec. */
4f5da8
+        virtqueue_push(vq, &s->stats_vq_elem, 0);
4f5da8
+        virtio_notify(vdev, vq);
4f5da8
+    }
4f5da8
+
4f5da8
+    s->stats_vq_elem = elem;
4f5da8
+    s->stats_vq_elem_pending = true;
4f5da8
 
4f5da8
     /* Initialize the stats to get rid of any stale values.  This is only
4f5da8
      * needed to handle the case where a guest supports fewer stats than it
4f5da8
@@ -235,7 +244,7 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
4f5da8
      */
4f5da8
     reset_stats(s);
4f5da8
 
4f5da8
-    while (iov_to_buf(elem->out_sg, elem->out_num, offset, &stat, sizeof(stat))
4f5da8
+    while (iov_to_buf(elem.out_sg, elem.out_num, offset, &stat, sizeof(stat))
4f5da8
            == sizeof(stat)) {
4f5da8
         uint16_t tag = tswap16(stat.tag);
4f5da8
         uint64_t val = tswap64(stat.val);
4f5da8
@@ -384,6 +393,15 @@ static void virtio_balloon_device_exit(VirtIODevice *vdev)
4f5da8
     virtio_cleanup(vdev);
4f5da8
 }
4f5da8
 
4f5da8
+static void virtio_balloon_device_reset(VirtIODevice *vdev)
4f5da8
+{
4f5da8
+    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
4f5da8
+
4f5da8
+    if (s->stats_vq_elem_pending) {
4f5da8
+        s->stats_vq_elem_pending = false;
4f5da8
+    }
4f5da8
+}
4f5da8
+
4f5da8
 static Property virtio_balloon_properties[] = {
4f5da8
     DEFINE_PROP_END_OF_LIST(),
4f5da8
 };
4f5da8
@@ -396,6 +414,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data)
4f5da8
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
4f5da8
     vdc->init = virtio_balloon_device_init;
4f5da8
     vdc->exit = virtio_balloon_device_exit;
4f5da8
+    vdc->reset = virtio_balloon_device_reset;
4f5da8
     vdc->get_config = virtio_balloon_get_config;
4f5da8
     vdc->set_config = virtio_balloon_set_config;
4f5da8
     vdc->get_features = virtio_balloon_get_features;
4f5da8
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
4f5da8
index f863bfe..a84736b 100644
4f5da8
--- a/include/hw/virtio/virtio-balloon.h
4f5da8
+++ b/include/hw/virtio/virtio-balloon.h
4f5da8
@@ -63,6 +63,7 @@ typedef struct VirtIOBalloon {
4f5da8
     uint32_t actual;
4f5da8
     uint64_t stats[VIRTIO_BALLOON_S_NR];
4f5da8
     VirtQueueElement stats_vq_elem;
4f5da8
+    bool stats_vq_elem_pending;
4f5da8
     size_t stats_vq_offset;
4f5da8
     QEMUTimer *stats_timer;
4f5da8
     int64_t stats_last_update;
4f5da8
-- 
4f5da8
1.8.3.1
4f5da8