qemu-stable
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-stable] [PATCH 4/6] balloon: fix segfault and harden the stats que


From: Ladi Prosek
Subject: [Qemu-stable] [PATCH 4/6] balloon: fix segfault and harden the stats queue
Date: Mon, 2 Jan 2017 12:58:43 +0100

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 51b19ebe4320f3dc, 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: address@hidden
Signed-off-by: Ladi Prosek <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>
(cherry picked from commit 4eae2a657d1ff5ada56eb9b4966eae0eff333b0b)
Signed-off-by: Ladi Prosek <address@hidden>

Conflicts:
  * 0.12.1.2 does not return pointers from virtqueue_pop so only the
    "harden the stats queue" part of the upstream patch description
    applies
  * a new field stats_vq_elem_pending is introduced to keep track
    of the state of stats_vq_elem in lieu of its nullness upstream
  * virtio_balloon_device_reset only resets stats_vq_elem_pending
    because there is nothing to free
---
 hw/virtio-balloon.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 495a483..24e50af 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -44,6 +44,7 @@ typedef struct VirtIOBalloon
     uint32_t actual;
     uint64_t stats[VIRTIO_BALLOON_S_NR];
     VirtQueueElement stats_vq_elem;
+    bool stats_vq_elem_pending;
     size_t stats_vq_offset;
     MonitorCompletion *stats_callback;
     void *stats_opaque_callback_data;
@@ -150,13 +151,21 @@ static void complete_stats_request(VirtIOBalloon *vb)
 static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOBalloon *s = DO_UPCAST(VirtIOBalloon, vdev, vdev);
-    VirtQueueElement *elem = &s->stats_vq_elem;
+    VirtQueueElement elem;
     VirtIOBalloonStat stat;
     size_t offset = 0;
 
-    if (!virtqueue_pop(vq, elem)) {
+    if (!virtqueue_pop(vq, &elem)) {
         return;
     }
+    if (s->stats_vq_elem_pending) {
+        /* This should never happen if the driver follows the spec. */
+        virtqueue_push(vq, &s->stats_vq_elem, 0);
+        virtio_notify(vdev, vq);
+    }
+
+    s->stats_vq_elem = elem;
+    s->stats_vq_elem_pending = true;
 
     /* 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
@@ -164,7 +173,7 @@ static void virtio_balloon_receive_stats(VirtIODevice 
*vdev, VirtQueue *vq)
      */
     reset_stats(s);
 
-    while (iov_to_buf(elem->out_sg, elem->out_num, &stat, offset, sizeof(stat))
+    while (iov_to_buf(elem.out_sg, elem.out_num, &stat, offset, sizeof(stat))
            == sizeof(stat)) {
         uint16_t tag = tswap16(stat.tag);
         uint64_t val = tswap64(stat.val);
@@ -178,6 +187,15 @@ static void virtio_balloon_receive_stats(VirtIODevice 
*vdev, VirtQueue *vq)
     complete_stats_request(s);
 }
 
+static void virtio_balloon_device_reset(VirtIODevice *vdev)
+{
+    VirtIOBalloon *s = to_virtio_balloon(vdev);
+
+    if (s->stats_vq_elem_pending) {
+        s->stats_vq_elem_pending = false;
+    }
+}
+
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
     VirtIOBalloon *dev = to_virtio_balloon(vdev);
@@ -224,8 +242,10 @@ static void virtio_balloon_stat(void *opaque, 
MonitorCompletion cb,
     dev->stats_opaque_callback_data = cb_data;
 
     if (ENABLE_GUEST_STATS
+        && dev->stats_vq_elem_pending
         && (dev->vdev.guest_features & (1 << VIRTIO_BALLOON_F_STATS_VQ))) {
         virtqueue_push(dev->svq, &dev->stats_vq_elem, dev->stats_vq_offset);
+        dev->stats_vq_elem_pending = false;
         virtio_notify(&dev->vdev, dev->svq);
         return;
     }
@@ -287,6 +307,7 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
                                             VIRTIO_ID_BALLOON,
                                             8, sizeof(VirtIOBalloon));
 
+    s->vdev.reset = virtio_balloon_device_reset;
     s->vdev.get_config = virtio_balloon_get_config;
     s->vdev.set_config = virtio_balloon_set_config;
     s->vdev.get_features = virtio_balloon_get_features;
-- 
2.7.4




reply via email to

[Prev in Thread] Current Thread [Next in Thread]