QEMU is a FAST! processor emulator
CentOS Sources
2017-03-02 4f5da8cf40ea809191d5b389aa6a63327196fdcf
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
From 75255574498fad12727529c4ecbd4ccdabe86839 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <lprosek@redhat.com>
Date: Wed, 5 Oct 2016 17:22:26 +0200
Subject: [PATCH 4/8] balloon: fix segfault and harden the stats queue
 
RH-Author: Ladi Prosek <lprosek@redhat.com>
Message-id: <1475666548-9186-5-git-send-email-lprosek@redhat.com>
Patchwork-id: 72483
O-Subject: [RHEL-7.4 qemu-kvm v2 PATCH 4/6] balloon: fix segfault and harden the stats queue
Bugzilla: 1393484
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
RH-Acked-by: Michael S. Tsirkin <mst@redhat.com>
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
 
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: qemu-stable@nongnu.org
Signed-off-by: Ladi Prosek <lprosek@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
(cherry picked from commit 4eae2a657d1ff5ada56eb9b4966eae0eff333b0b)
Signed-off-by: Ladi Prosek <lprosek@redhat.com>
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
 
Conflicts:
  * 1.5.3 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/virtio-balloon.c         | 27 +++++++++++++++++++++++----
 include/hw/virtio/virtio-balloon.h |  1 +
 2 files changed, 24 insertions(+), 4 deletions(-)
 
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 016dc60..17b3029 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -95,13 +95,14 @@ 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_pending || !balloon_stats_supported(s)) {
         /* re-schedule */
         balloon_stats_change_timer(s, s->stats_poll_interval);
         return;
     }
 
     virtqueue_push(s->svq, &s->stats_vq_elem, s->stats_vq_offset);
+    s->stats_vq_elem_pending = false;
     virtio_notify(vdev, s->svq);
 }
 
@@ -220,14 +221,22 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
-    VirtQueueElement *elem = &s->stats_vq_elem;
+    VirtQueueElement elem;
     VirtIOBalloonStat stat;
     size_t offset = 0;
     qemu_timeval tv;
 
-    if (!virtqueue_pop(vq, elem)) {
+    if (!virtqueue_pop(vq, &elem)) {
         goto out;
     }
+    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
@@ -235,7 +244,7 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
      */
     reset_stats(s);
 
-    while (iov_to_buf(elem->out_sg, elem->out_num, offset, &stat, sizeof(stat))
+    while (iov_to_buf(elem.out_sg, elem.out_num, offset, &stat, sizeof(stat))
            == sizeof(stat)) {
         uint16_t tag = tswap16(stat.tag);
         uint64_t val = tswap64(stat.val);
@@ -384,6 +393,15 @@ static void virtio_balloon_device_exit(VirtIODevice *vdev)
     virtio_cleanup(vdev);
 }
 
+static void virtio_balloon_device_reset(VirtIODevice *vdev)
+{
+    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+
+    if (s->stats_vq_elem_pending) {
+        s->stats_vq_elem_pending = false;
+    }
+}
+
 static Property virtio_balloon_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -396,6 +414,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     vdc->init = virtio_balloon_device_init;
     vdc->exit = virtio_balloon_device_exit;
+    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;
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index f863bfe..a84736b 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -63,6 +63,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;
     QEMUTimer *stats_timer;
     int64_t stats_last_update;
-- 
1.8.3.1