diff options
author | Alistair Francis <alistair.francis@xilinx.com> | 2018-02-16 11:48:41 -0800 |
---|---|---|
committer | Richard Purdie <richard.purdie@linuxfoundation.org> | 2018-02-24 10:31:45 +0000 |
commit | 704d0b114d99cc5bd556730de1370c2c058e822e (patch) | |
tree | 824a63a16a0398007affff902868887aebe3c523 | |
parent | f9c90755eb7ffc5ce8b87a2fd1b9c05522ffe7d9 (diff) | |
download | poky-704d0b114d99cc5bd556730de1370c2c058e822e.tar.gz |
recipes-devtools: Bump QEMU to 2.11.1
Bump the QEMU version to the bug fix release of 2.11.1 and remove the
patches that are no longer required.
(From OE-Core rev: da7fa8a15dfafd07e5956b69996d99880596c333)
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Signed-off-by: Ross Burton <ross.burton@intel.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
-rw-r--r-- | meta/recipes-devtools/qemu/qemu/CVE-2017-15124.patch | 1476 | ||||
-rw-r--r-- | meta/recipes-devtools/qemu/qemu_2.11.1.bb (renamed from meta/recipes-devtools/qemu/qemu_2.11.0.bb) | 5 |
2 files changed, 2 insertions, 1479 deletions
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2017-15124.patch b/meta/recipes-devtools/qemu/qemu/CVE-2017-15124.patch deleted file mode 100644 index a47b6d0510..0000000000 --- a/meta/recipes-devtools/qemu/qemu/CVE-2017-15124.patch +++ /dev/null | |||
@@ -1,1476 +0,0 @@ | |||
1 | VNC server implementation in Quick Emulator (QEMU) 2.11.0 and older was found to | ||
2 | be vulnerable to an unbounded memory allocation issue, as it did not throttle | ||
3 | the framebuffer updates sent to its client. If the client did not consume these | ||
4 | updates, VNC server allocates growing memory to hold onto this data. A malicious | ||
5 | remote VNC client could use this flaw to cause DoS to the server host. | ||
6 | |||
7 | CVE: CVE-2017-15124 | ||
8 | Upstream-Status: Backport | ||
9 | Signed-off-by: Ross Burton <ross.burton@intel.com> | ||
10 | |||
11 | From 090fdc83b0960f68d204624a73c6814780da52d9 Mon Sep 17 00:00:00 2001 | ||
12 | From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@redhat.com> | ||
13 | Date: Wed, 20 Dec 2017 15:06:18 +0100 | ||
14 | Subject: [PATCH 01/14] vnc: fix debug spelling | ||
15 | MIME-Version: 1.0 | ||
16 | Content-Type: text/plain; charset=UTF-8 | ||
17 | Content-Transfer-Encoding: 8bit | ||
18 | |||
19 | Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
20 | Message-id: 20171220140618.12701-1-marcandre.lureau@redhat.com | ||
21 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
22 | --- | ||
23 | ui/vnc.c | 2 +- | ||
24 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
25 | |||
26 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
27 | index 9f8d5a1b1f..7d537b5c6b 100644 | ||
28 | --- a/ui/vnc.c | ||
29 | +++ b/ui/vnc.c | ||
30 | @@ -2255,7 +2255,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len) | ||
31 | } | ||
32 | vs->as.nchannels = read_u8(data, 5); | ||
33 | if (vs->as.nchannels != 1 && vs->as.nchannels != 2) { | ||
34 | - VNC_DEBUG("Invalid audio channel coount %d\n", | ||
35 | + VNC_DEBUG("Invalid audio channel count %d\n", | ||
36 | read_u8(data, 5)); | ||
37 | vnc_client_error(vs); | ||
38 | break; | ||
39 | -- | ||
40 | 2.11.0 | ||
41 | |||
42 | |||
43 | From 6af998db05aec9af95a06f84ad94f1b96785e667 Mon Sep 17 00:00:00 2001 | ||
44 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
45 | Date: Mon, 18 Dec 2017 19:12:16 +0000 | ||
46 | Subject: [PATCH 02/14] ui: remove 'sync' parameter from vnc_update_client | ||
47 | MIME-Version: 1.0 | ||
48 | Content-Type: text/plain; charset=UTF-8 | ||
49 | Content-Transfer-Encoding: 8bit | ||
50 | |||
51 | There is only one caller of vnc_update_client and that always passes false | ||
52 | for the 'sync' parameter. | ||
53 | |||
54 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
55 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
56 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
57 | Message-id: 20171218191228.31018-2-berrange@redhat.com | ||
58 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
59 | --- | ||
60 | ui/vnc.c | 11 +++-------- | ||
61 | 1 file changed, 3 insertions(+), 8 deletions(-) | ||
62 | |||
63 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
64 | index 7d537b5c6b..d72a61bde3 100644 | ||
65 | --- a/ui/vnc.c | ||
66 | +++ b/ui/vnc.c | ||
67 | @@ -596,7 +596,7 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp) | ||
68 | 3) resolutions > 1024 | ||
69 | */ | ||
70 | |||
71 | -static int vnc_update_client(VncState *vs, int has_dirty, bool sync); | ||
72 | +static int vnc_update_client(VncState *vs, int has_dirty); | ||
73 | static void vnc_disconnect_start(VncState *vs); | ||
74 | |||
75 | static void vnc_colordepth(VncState *vs); | ||
76 | @@ -961,7 +961,7 @@ static int find_and_clear_dirty_height(VncState *vs, | ||
77 | return h; | ||
78 | } | ||
79 | |||
80 | -static int vnc_update_client(VncState *vs, int has_dirty, bool sync) | ||
81 | +static int vnc_update_client(VncState *vs, int has_dirty) | ||
82 | { | ||
83 | if (vs->disconnecting) { | ||
84 | vnc_disconnect_finish(vs); | ||
85 | @@ -1025,9 +1025,6 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync) | ||
86 | } | ||
87 | |||
88 | vnc_job_push(job); | ||
89 | - if (sync) { | ||
90 | - vnc_jobs_join(vs); | ||
91 | - } | ||
92 | vs->force_update = 0; | ||
93 | vs->has_dirty = 0; | ||
94 | return n; | ||
95 | @@ -1035,8 +1032,6 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync) | ||
96 | |||
97 | if (vs->disconnecting) { | ||
98 | vnc_disconnect_finish(vs); | ||
99 | - } else if (sync) { | ||
100 | - vnc_jobs_join(vs); | ||
101 | } | ||
102 | |||
103 | return 0; | ||
104 | @@ -2863,7 +2858,7 @@ static void vnc_refresh(DisplayChangeListener *dcl) | ||
105 | vnc_unlock_display(vd); | ||
106 | |||
107 | QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) { | ||
108 | - rects += vnc_update_client(vs, has_dirty, false); | ||
109 | + rects += vnc_update_client(vs, has_dirty); | ||
110 | /* vs might be free()ed here */ | ||
111 | } | ||
112 | |||
113 | -- | ||
114 | 2.11.0 | ||
115 | |||
116 | |||
117 | From c53df961617736f94731d94b62c2954c261d2bae Mon Sep 17 00:00:00 2001 | ||
118 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
119 | Date: Mon, 18 Dec 2017 19:12:17 +0000 | ||
120 | Subject: [PATCH 03/14] ui: remove unreachable code in vnc_update_client | ||
121 | MIME-Version: 1.0 | ||
122 | Content-Type: text/plain; charset=UTF-8 | ||
123 | Content-Transfer-Encoding: 8bit | ||
124 | |||
125 | A previous commit: | ||
126 | |||
127 | commit 5a8be0f73d6f60ff08746377eb09ca459f39deab | ||
128 | Author: Gerd Hoffmann <kraxel@redhat.com> | ||
129 | Date: Wed Jul 13 12:21:20 2016 +0200 | ||
130 | |||
131 | vnc: make sure we finish disconnect | ||
132 | |||
133 | Added a check for vs->disconnecting at the very start of the | ||
134 | vnc_update_client method. This means that the very next "if" | ||
135 | statement check for !vs->disconnecting always evaluates true, | ||
136 | and is thus redundant. This in turn means the vs->disconnecting | ||
137 | check at the very end of the method never evaluates true, and | ||
138 | is thus unreachable code. | ||
139 | |||
140 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
141 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
142 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
143 | Message-id: 20171218191228.31018-3-berrange@redhat.com | ||
144 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
145 | --- | ||
146 | ui/vnc.c | 6 +----- | ||
147 | 1 file changed, 1 insertion(+), 5 deletions(-) | ||
148 | |||
149 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
150 | index d72a61bde3..29a7208475 100644 | ||
151 | --- a/ui/vnc.c | ||
152 | +++ b/ui/vnc.c | ||
153 | @@ -969,7 +969,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) | ||
154 | } | ||
155 | |||
156 | vs->has_dirty += has_dirty; | ||
157 | - if (vs->need_update && !vs->disconnecting) { | ||
158 | + if (vs->need_update) { | ||
159 | VncDisplay *vd = vs->vd; | ||
160 | VncJob *job; | ||
161 | int y; | ||
162 | @@ -1030,10 +1030,6 @@ static int vnc_update_client(VncState *vs, int has_dirty) | ||
163 | return n; | ||
164 | } | ||
165 | |||
166 | - if (vs->disconnecting) { | ||
167 | - vnc_disconnect_finish(vs); | ||
168 | - } | ||
169 | - | ||
170 | return 0; | ||
171 | } | ||
172 | |||
173 | -- | ||
174 | 2.11.0 | ||
175 | |||
176 | |||
177 | From b939eb89b6f320544a9328fa908d881d0024c1ee Mon Sep 17 00:00:00 2001 | ||
178 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
179 | Date: Mon, 18 Dec 2017 19:12:18 +0000 | ||
180 | Subject: [PATCH 04/14] ui: remove redundant indentation in vnc_client_update | ||
181 | MIME-Version: 1.0 | ||
182 | Content-Type: text/plain; charset=UTF-8 | ||
183 | Content-Transfer-Encoding: 8bit | ||
184 | |||
185 | Now that previous dead / unreachable code has been removed, we can simplify | ||
186 | the indentation in the vnc_client_update method. | ||
187 | |||
188 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
189 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
190 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
191 | Message-id: 20171218191228.31018-4-berrange@redhat.com | ||
192 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
193 | --- | ||
194 | ui/vnc.c | 112 ++++++++++++++++++++++++++++++++------------------------------- | ||
195 | 1 file changed, 57 insertions(+), 55 deletions(-) | ||
196 | |||
197 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
198 | index 29a7208475..7582111ca6 100644 | ||
199 | --- a/ui/vnc.c | ||
200 | +++ b/ui/vnc.c | ||
201 | @@ -963,74 +963,76 @@ static int find_and_clear_dirty_height(VncState *vs, | ||
202 | |||
203 | static int vnc_update_client(VncState *vs, int has_dirty) | ||
204 | { | ||
205 | + VncDisplay *vd = vs->vd; | ||
206 | + VncJob *job; | ||
207 | + int y; | ||
208 | + int height, width; | ||
209 | + int n = 0; | ||
210 | + | ||
211 | if (vs->disconnecting) { | ||
212 | vnc_disconnect_finish(vs); | ||
213 | return 0; | ||
214 | } | ||
215 | |||
216 | vs->has_dirty += has_dirty; | ||
217 | - if (vs->need_update) { | ||
218 | - VncDisplay *vd = vs->vd; | ||
219 | - VncJob *job; | ||
220 | - int y; | ||
221 | - int height, width; | ||
222 | - int n = 0; | ||
223 | - | ||
224 | - if (vs->output.offset && !vs->audio_cap && !vs->force_update) | ||
225 | - /* kernel send buffers are full -> drop frames to throttle */ | ||
226 | - return 0; | ||
227 | + if (!vs->need_update) { | ||
228 | + return 0; | ||
229 | + } | ||
230 | |||
231 | - if (!vs->has_dirty && !vs->audio_cap && !vs->force_update) | ||
232 | - return 0; | ||
233 | + if (vs->output.offset && !vs->audio_cap && !vs->force_update) { | ||
234 | + /* kernel send buffers are full -> drop frames to throttle */ | ||
235 | + return 0; | ||
236 | + } | ||
237 | |||
238 | - /* | ||
239 | - * Send screen updates to the vnc client using the server | ||
240 | - * surface and server dirty map. guest surface updates | ||
241 | - * happening in parallel don't disturb us, the next pass will | ||
242 | - * send them to the client. | ||
243 | - */ | ||
244 | - job = vnc_job_new(vs); | ||
245 | - | ||
246 | - height = pixman_image_get_height(vd->server); | ||
247 | - width = pixman_image_get_width(vd->server); | ||
248 | - | ||
249 | - y = 0; | ||
250 | - for (;;) { | ||
251 | - int x, h; | ||
252 | - unsigned long x2; | ||
253 | - unsigned long offset = find_next_bit((unsigned long *) &vs->dirty, | ||
254 | - height * VNC_DIRTY_BPL(vs), | ||
255 | - y * VNC_DIRTY_BPL(vs)); | ||
256 | - if (offset == height * VNC_DIRTY_BPL(vs)) { | ||
257 | - /* no more dirty bits */ | ||
258 | + if (!vs->has_dirty && !vs->audio_cap && !vs->force_update) { | ||
259 | + return 0; | ||
260 | + } | ||
261 | + | ||
262 | + /* | ||
263 | + * Send screen updates to the vnc client using the server | ||
264 | + * surface and server dirty map. guest surface updates | ||
265 | + * happening in parallel don't disturb us, the next pass will | ||
266 | + * send them to the client. | ||
267 | + */ | ||
268 | + job = vnc_job_new(vs); | ||
269 | + | ||
270 | + height = pixman_image_get_height(vd->server); | ||
271 | + width = pixman_image_get_width(vd->server); | ||
272 | + | ||
273 | + y = 0; | ||
274 | + for (;;) { | ||
275 | + int x, h; | ||
276 | + unsigned long x2; | ||
277 | + unsigned long offset = find_next_bit((unsigned long *) &vs->dirty, | ||
278 | + height * VNC_DIRTY_BPL(vs), | ||
279 | + y * VNC_DIRTY_BPL(vs)); | ||
280 | + if (offset == height * VNC_DIRTY_BPL(vs)) { | ||
281 | + /* no more dirty bits */ | ||
282 | + break; | ||
283 | + } | ||
284 | + y = offset / VNC_DIRTY_BPL(vs); | ||
285 | + x = offset % VNC_DIRTY_BPL(vs); | ||
286 | + x2 = find_next_zero_bit((unsigned long *) &vs->dirty[y], | ||
287 | + VNC_DIRTY_BPL(vs), x); | ||
288 | + bitmap_clear(vs->dirty[y], x, x2 - x); | ||
289 | + h = find_and_clear_dirty_height(vs, y, x, x2, height); | ||
290 | + x2 = MIN(x2, width / VNC_DIRTY_PIXELS_PER_BIT); | ||
291 | + if (x2 > x) { | ||
292 | + n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y, | ||
293 | + (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h); | ||
294 | + } | ||
295 | + if (!x && x2 == width / VNC_DIRTY_PIXELS_PER_BIT) { | ||
296 | + y += h; | ||
297 | + if (y == height) { | ||
298 | break; | ||
299 | } | ||
300 | - y = offset / VNC_DIRTY_BPL(vs); | ||
301 | - x = offset % VNC_DIRTY_BPL(vs); | ||
302 | - x2 = find_next_zero_bit((unsigned long *) &vs->dirty[y], | ||
303 | - VNC_DIRTY_BPL(vs), x); | ||
304 | - bitmap_clear(vs->dirty[y], x, x2 - x); | ||
305 | - h = find_and_clear_dirty_height(vs, y, x, x2, height); | ||
306 | - x2 = MIN(x2, width / VNC_DIRTY_PIXELS_PER_BIT); | ||
307 | - if (x2 > x) { | ||
308 | - n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y, | ||
309 | - (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h); | ||
310 | - } | ||
311 | - if (!x && x2 == width / VNC_DIRTY_PIXELS_PER_BIT) { | ||
312 | - y += h; | ||
313 | - if (y == height) { | ||
314 | - break; | ||
315 | - } | ||
316 | - } | ||
317 | } | ||
318 | - | ||
319 | - vnc_job_push(job); | ||
320 | - vs->force_update = 0; | ||
321 | - vs->has_dirty = 0; | ||
322 | - return n; | ||
323 | } | ||
324 | |||
325 | - return 0; | ||
326 | + vnc_job_push(job); | ||
327 | + vs->force_update = 0; | ||
328 | + vs->has_dirty = 0; | ||
329 | + return n; | ||
330 | } | ||
331 | |||
332 | /* audio */ | ||
333 | -- | ||
334 | 2.11.0 | ||
335 | |||
336 | |||
337 | From 3541b08475d51bddf8aded36576a0ff5a547a978 Mon Sep 17 00:00:00 2001 | ||
338 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
339 | Date: Mon, 18 Dec 2017 19:12:19 +0000 | ||
340 | Subject: [PATCH 05/14] ui: avoid pointless VNC updates if framebuffer isn't | ||
341 | dirty | ||
342 | MIME-Version: 1.0 | ||
343 | Content-Type: text/plain; charset=UTF-8 | ||
344 | Content-Transfer-Encoding: 8bit | ||
345 | |||
346 | The vnc_update_client() method checks the 'has_dirty' flag to see if there are | ||
347 | dirty regions that are pending to send to the client. Regardless of this flag, | ||
348 | if a forced update is requested, updates must be sent. For unknown reasons | ||
349 | though, the code also tries to sent updates if audio capture is enabled. This | ||
350 | makes no sense as audio capture state does not impact framebuffer contents, so | ||
351 | this check is removed. | ||
352 | |||
353 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
354 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
355 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
356 | Message-id: 20171218191228.31018-5-berrange@redhat.com | ||
357 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
358 | --- | ||
359 | ui/vnc.c | 2 +- | ||
360 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
361 | |||
362 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
363 | index 7582111ca6..a79848f083 100644 | ||
364 | --- a/ui/vnc.c | ||
365 | +++ b/ui/vnc.c | ||
366 | @@ -984,7 +984,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) | ||
367 | return 0; | ||
368 | } | ||
369 | |||
370 | - if (!vs->has_dirty && !vs->audio_cap && !vs->force_update) { | ||
371 | + if (!vs->has_dirty && !vs->force_update) { | ||
372 | return 0; | ||
373 | } | ||
374 | |||
375 | -- | ||
376 | 2.11.0 | ||
377 | |||
378 | |||
379 | From 8f61f1c5a6bc06438a1172efa80bc7606594fa07 Mon Sep 17 00:00:00 2001 | ||
380 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
381 | Date: Mon, 18 Dec 2017 19:12:20 +0000 | ||
382 | Subject: [PATCH 06/14] ui: track how much decoded data we consumed when doing | ||
383 | SASL encoding | ||
384 | MIME-Version: 1.0 | ||
385 | Content-Type: text/plain; charset=UTF-8 | ||
386 | Content-Transfer-Encoding: 8bit | ||
387 | |||
388 | When we encode data for writing with SASL, we encode the entire pending output | ||
389 | buffer. The subsequent write, however, may not be able to send the full encoded | ||
390 | data in one go though, particularly with a slow network. So we delay setting the | ||
391 | output buffer offset back to zero until all the SASL encoded data is sent. | ||
392 | |||
393 | Between encoding the data and completing sending of the SASL encoded data, | ||
394 | however, more data might have been placed on the pending output buffer. So it | ||
395 | is not valid to set offset back to zero. Instead we must keep track of how much | ||
396 | data we consumed during encoding and subtract only that amount. | ||
397 | |||
398 | With the current bug we would be throwing away some pending data without having | ||
399 | sent it at all. By sheer luck this did not previously cause any serious problem | ||
400 | because appending data to the send buffer is always an atomic action, so we | ||
401 | only ever throw away complete RFB protocol messages. In the case of frame buffer | ||
402 | updates we'd catch up fairly quickly, so no obvious problem was visible. | ||
403 | |||
404 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
405 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
406 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
407 | Message-id: 20171218191228.31018-6-berrange@redhat.com | ||
408 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
409 | --- | ||
410 | ui/vnc-auth-sasl.c | 3 ++- | ||
411 | ui/vnc-auth-sasl.h | 1 + | ||
412 | 2 files changed, 3 insertions(+), 1 deletion(-) | ||
413 | |||
414 | diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c | ||
415 | index 23f28280e7..761493b9b2 100644 | ||
416 | --- a/ui/vnc-auth-sasl.c | ||
417 | +++ b/ui/vnc-auth-sasl.c | ||
418 | @@ -67,6 +67,7 @@ long vnc_client_write_sasl(VncState *vs) | ||
419 | if (err != SASL_OK) | ||
420 | return vnc_client_io_error(vs, -1, NULL); | ||
421 | |||
422 | + vs->sasl.encodedRawLength = vs->output.offset; | ||
423 | vs->sasl.encodedOffset = 0; | ||
424 | } | ||
425 | |||
426 | @@ -78,7 +79,7 @@ long vnc_client_write_sasl(VncState *vs) | ||
427 | |||
428 | vs->sasl.encodedOffset += ret; | ||
429 | if (vs->sasl.encodedOffset == vs->sasl.encodedLength) { | ||
430 | - vs->output.offset = 0; | ||
431 | + vs->output.offset -= vs->sasl.encodedRawLength; | ||
432 | vs->sasl.encoded = NULL; | ||
433 | vs->sasl.encodedOffset = vs->sasl.encodedLength = 0; | ||
434 | } | ||
435 | diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h | ||
436 | index cb42745a6b..b9d8de1c10 100644 | ||
437 | --- a/ui/vnc-auth-sasl.h | ||
438 | +++ b/ui/vnc-auth-sasl.h | ||
439 | @@ -53,6 +53,7 @@ struct VncStateSASL { | ||
440 | */ | ||
441 | const uint8_t *encoded; | ||
442 | unsigned int encodedLength; | ||
443 | + unsigned int encodedRawLength; | ||
444 | unsigned int encodedOffset; | ||
445 | char *username; | ||
446 | char *mechlist; | ||
447 | -- | ||
448 | 2.11.0 | ||
449 | |||
450 | |||
451 | From fef1bbadfb2c3027208eb3d14b43e1bdb51166ca Mon Sep 17 00:00:00 2001 | ||
452 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
453 | Date: Mon, 18 Dec 2017 19:12:21 +0000 | ||
454 | Subject: [PATCH 07/14] ui: introduce enum to track VNC client framebuffer | ||
455 | update request state | ||
456 | MIME-Version: 1.0 | ||
457 | Content-Type: text/plain; charset=UTF-8 | ||
458 | Content-Transfer-Encoding: 8bit | ||
459 | |||
460 | Currently the VNC servers tracks whether a client has requested an incremental | ||
461 | or forced update with two boolean flags. There are only really 3 distinct | ||
462 | states to track, so create an enum to more accurately reflect permitted states. | ||
463 | |||
464 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
465 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
466 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
467 | Message-id: 20171218191228.31018-7-berrange@redhat.com | ||
468 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
469 | --- | ||
470 | ui/vnc.c | 21 +++++++++++---------- | ||
471 | ui/vnc.h | 9 +++++++-- | ||
472 | 2 files changed, 18 insertions(+), 12 deletions(-) | ||
473 | |||
474 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
475 | index a79848f083..30e2feeae3 100644 | ||
476 | --- a/ui/vnc.c | ||
477 | +++ b/ui/vnc.c | ||
478 | @@ -975,16 +975,17 @@ static int vnc_update_client(VncState *vs, int has_dirty) | ||
479 | } | ||
480 | |||
481 | vs->has_dirty += has_dirty; | ||
482 | - if (!vs->need_update) { | ||
483 | + if (vs->update == VNC_STATE_UPDATE_NONE) { | ||
484 | return 0; | ||
485 | } | ||
486 | |||
487 | - if (vs->output.offset && !vs->audio_cap && !vs->force_update) { | ||
488 | + if (vs->output.offset && !vs->audio_cap && | ||
489 | + vs->update != VNC_STATE_UPDATE_FORCE) { | ||
490 | /* kernel send buffers are full -> drop frames to throttle */ | ||
491 | return 0; | ||
492 | } | ||
493 | |||
494 | - if (!vs->has_dirty && !vs->force_update) { | ||
495 | + if (!vs->has_dirty && vs->update != VNC_STATE_UPDATE_FORCE) { | ||
496 | return 0; | ||
497 | } | ||
498 | |||
499 | @@ -1030,7 +1031,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) | ||
500 | } | ||
501 | |||
502 | vnc_job_push(job); | ||
503 | - vs->force_update = 0; | ||
504 | + vs->update = VNC_STATE_UPDATE_INCREMENTAL; | ||
505 | vs->has_dirty = 0; | ||
506 | return n; | ||
507 | } | ||
508 | @@ -1869,14 +1870,14 @@ static void ext_key_event(VncState *vs, int down, | ||
509 | static void framebuffer_update_request(VncState *vs, int incremental, | ||
510 | int x, int y, int w, int h) | ||
511 | { | ||
512 | - vs->need_update = 1; | ||
513 | - | ||
514 | if (incremental) { | ||
515 | - return; | ||
516 | + if (vs->update != VNC_STATE_UPDATE_FORCE) { | ||
517 | + vs->update = VNC_STATE_UPDATE_INCREMENTAL; | ||
518 | + } | ||
519 | + } else { | ||
520 | + vs->update = VNC_STATE_UPDATE_FORCE; | ||
521 | + vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h); | ||
522 | } | ||
523 | - | ||
524 | - vs->force_update = 1; | ||
525 | - vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h); | ||
526 | } | ||
527 | |||
528 | static void send_ext_key_event_ack(VncState *vs) | ||
529 | diff --git a/ui/vnc.h b/ui/vnc.h | ||
530 | index 694cf32ca9..b9d310e640 100644 | ||
531 | --- a/ui/vnc.h | ||
532 | +++ b/ui/vnc.h | ||
533 | @@ -252,6 +252,12 @@ struct VncJob | ||
534 | QTAILQ_ENTRY(VncJob) next; | ||
535 | }; | ||
536 | |||
537 | +typedef enum { | ||
538 | + VNC_STATE_UPDATE_NONE, | ||
539 | + VNC_STATE_UPDATE_INCREMENTAL, | ||
540 | + VNC_STATE_UPDATE_FORCE, | ||
541 | +} VncStateUpdate; | ||
542 | + | ||
543 | struct VncState | ||
544 | { | ||
545 | QIOChannelSocket *sioc; /* The underlying socket */ | ||
546 | @@ -264,8 +270,7 @@ struct VncState | ||
547 | * vnc-jobs-async.c */ | ||
548 | |||
549 | VncDisplay *vd; | ||
550 | - int need_update; | ||
551 | - int force_update; | ||
552 | + VncStateUpdate update; /* Most recent pending request from client */ | ||
553 | int has_dirty; | ||
554 | uint32_t features; | ||
555 | int absolute; | ||
556 | -- | ||
557 | 2.11.0 | ||
558 | |||
559 | |||
560 | From 728a7ac95484a7ba5e624ccbac4c1326571576b0 Mon Sep 17 00:00:00 2001 | ||
561 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
562 | Date: Mon, 18 Dec 2017 19:12:22 +0000 | ||
563 | Subject: [PATCH 08/14] ui: correctly reset framebuffer update state after | ||
564 | processing dirty regions | ||
565 | MIME-Version: 1.0 | ||
566 | Content-Type: text/plain; charset=UTF-8 | ||
567 | Content-Transfer-Encoding: 8bit | ||
568 | |||
569 | According to the RFB protocol, a client sends one or more framebuffer update | ||
570 | requests to the server. The server can reply with a single framebuffer update | ||
571 | response, that covers all previously received requests. Once the client has | ||
572 | read this update from the server, it may send further framebuffer update | ||
573 | requests to monitor future changes. The client is free to delay sending the | ||
574 | framebuffer update request if it needs to throttle the amount of data it is | ||
575 | reading from the server. | ||
576 | |||
577 | The QEMU VNC server, however, has never correctly handled the framebuffer | ||
578 | update requests. Once QEMU has received an update request, it will continue to | ||
579 | send client updates forever, even if the client hasn't asked for further | ||
580 | updates. This prevents the client from throttling back data it gets from the | ||
581 | server. This change fixes the flawed logic such that after a set of updates are | ||
582 | sent out, QEMU waits for a further update request before sending more data. | ||
583 | |||
584 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
585 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
586 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
587 | Message-id: 20171218191228.31018-8-berrange@redhat.com | ||
588 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
589 | --- | ||
590 | ui/vnc.c | 2 +- | ||
591 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
592 | |||
593 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
594 | index 30e2feeae3..243c72be13 100644 | ||
595 | --- a/ui/vnc.c | ||
596 | +++ b/ui/vnc.c | ||
597 | @@ -1031,7 +1031,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) | ||
598 | } | ||
599 | |||
600 | vnc_job_push(job); | ||
601 | - vs->update = VNC_STATE_UPDATE_INCREMENTAL; | ||
602 | + vs->update = VNC_STATE_UPDATE_NONE; | ||
603 | vs->has_dirty = 0; | ||
604 | return n; | ||
605 | } | ||
606 | -- | ||
607 | 2.11.0 | ||
608 | |||
609 | |||
610 | From 0bad834228b9ee63e4239108d02dcb94568254d0 Mon Sep 17 00:00:00 2001 | ||
611 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
612 | Date: Mon, 18 Dec 2017 19:12:23 +0000 | ||
613 | Subject: [PATCH 09/14] ui: refactor code for determining if an update should | ||
614 | be sent to the client | ||
615 | MIME-Version: 1.0 | ||
616 | Content-Type: text/plain; charset=UTF-8 | ||
617 | Content-Transfer-Encoding: 8bit | ||
618 | |||
619 | The logic for determining if it is possible to send an update to the client | ||
620 | will become more complicated shortly, so pull it out into a separate method | ||
621 | for easier extension later. | ||
622 | |||
623 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
624 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
625 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
626 | Message-id: 20171218191228.31018-9-berrange@redhat.com | ||
627 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
628 | --- | ||
629 | ui/vnc.c | 27 ++++++++++++++++++++------- | ||
630 | 1 file changed, 20 insertions(+), 7 deletions(-) | ||
631 | |||
632 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
633 | index 243c72be13..4ba7fc076a 100644 | ||
634 | --- a/ui/vnc.c | ||
635 | +++ b/ui/vnc.c | ||
636 | @@ -961,6 +961,25 @@ static int find_and_clear_dirty_height(VncState *vs, | ||
637 | return h; | ||
638 | } | ||
639 | |||
640 | +static bool vnc_should_update(VncState *vs) | ||
641 | +{ | ||
642 | + switch (vs->update) { | ||
643 | + case VNC_STATE_UPDATE_NONE: | ||
644 | + break; | ||
645 | + case VNC_STATE_UPDATE_INCREMENTAL: | ||
646 | + /* Only allow incremental updates if the output buffer | ||
647 | + * is empty, or if audio capture is enabled. | ||
648 | + */ | ||
649 | + if (!vs->output.offset || vs->audio_cap) { | ||
650 | + return true; | ||
651 | + } | ||
652 | + break; | ||
653 | + case VNC_STATE_UPDATE_FORCE: | ||
654 | + return true; | ||
655 | + } | ||
656 | + return false; | ||
657 | +} | ||
658 | + | ||
659 | static int vnc_update_client(VncState *vs, int has_dirty) | ||
660 | { | ||
661 | VncDisplay *vd = vs->vd; | ||
662 | @@ -975,13 +994,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) | ||
663 | } | ||
664 | |||
665 | vs->has_dirty += has_dirty; | ||
666 | - if (vs->update == VNC_STATE_UPDATE_NONE) { | ||
667 | - return 0; | ||
668 | - } | ||
669 | - | ||
670 | - if (vs->output.offset && !vs->audio_cap && | ||
671 | - vs->update != VNC_STATE_UPDATE_FORCE) { | ||
672 | - /* kernel send buffers are full -> drop frames to throttle */ | ||
673 | + if (!vnc_should_update(vs)) { | ||
674 | return 0; | ||
675 | } | ||
676 | |||
677 | -- | ||
678 | 2.11.0 | ||
679 | |||
680 | |||
681 | From e2b72cb6e0443d90d7ab037858cb6834b6cca852 Mon Sep 17 00:00:00 2001 | ||
682 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
683 | Date: Mon, 18 Dec 2017 19:12:24 +0000 | ||
684 | Subject: [PATCH 10/14] ui: fix VNC client throttling when audio capture is | ||
685 | active | ||
686 | MIME-Version: 1.0 | ||
687 | Content-Type: text/plain; charset=UTF-8 | ||
688 | Content-Transfer-Encoding: 8bit | ||
689 | |||
690 | The VNC server must throttle data sent to the client to prevent the 'output' | ||
691 | buffer size growing without bound, if the client stops reading data off the | ||
692 | socket (either maliciously or due to stalled/slow network connection). | ||
693 | |||
694 | The current throttling is very crude because it simply checks whether the | ||
695 | output buffer offset is zero. This check must be disabled if audio capture is | ||
696 | enabled, because when streaming audio the output buffer offset will rarely be | ||
697 | zero due to queued audio data, and so this would starve framebuffer updates. | ||
698 | |||
699 | As a result, the VNC client can cause QEMU to allocate arbitrary amounts of RAM. | ||
700 | They can first start something in the guest that triggers lots of framebuffer | ||
701 | updates eg play a youtube video. Then enable audio capture, and simply never | ||
702 | read data back from the server. This can easily make QEMU's VNC server send | ||
703 | buffer consume 100MB of RAM per second, until the OOM killer starts reaping | ||
704 | processes (hopefully the rogue QEMU process, but it might pick others...). | ||
705 | |||
706 | To address this we make the throttling more intelligent, so we can throttle | ||
707 | when audio capture is active too. To determine how to throttle incremental | ||
708 | updates or audio data, we calculate a size threshold. Normally the threshold is | ||
709 | the approximate number of bytes associated with a single complete framebuffer | ||
710 | update. ie width * height * bytes per pixel. We'll send incremental updates | ||
711 | until we hit this threshold, at which point we'll stop sending updates until | ||
712 | data has been written to the wire, causing the output buffer offset to fall | ||
713 | back below the threshold. | ||
714 | |||
715 | If audio capture is enabled, we increase the size of the threshold to also | ||
716 | allow for upto 1 seconds worth of audio data samples. ie nchannels * bytes | ||
717 | per sample * frequency. This allows the output buffer to have a mixture of | ||
718 | incremental framebuffer updates and audio data queued, but once the threshold | ||
719 | is exceeded, audio data will be dropped and incremental updates will be | ||
720 | throttled. | ||
721 | |||
722 | This unbounded memory growth affects all VNC server configurations supported by | ||
723 | QEMU, with no workaround possible. The mitigating factor is that it can only be | ||
724 | triggered by a client that has authenticated with the VNC server, and who is | ||
725 | able to trigger a large quantity of framebuffer updates or audio samples from | ||
726 | the guest OS. Mostly they'll just succeed in getting the OOM killer to kill | ||
727 | their own QEMU process, but its possible other processes can get taken out as | ||
728 | collateral damage. | ||
729 | |||
730 | This is a more general variant of the similar unbounded memory usage flaw in | ||
731 | the websockets server, that was previously assigned CVE-2017-15268, and fixed | ||
732 | in 2.11 by: | ||
733 | |||
734 | commit a7b20a8efa28e5f22c26c06cd06c2f12bc863493 | ||
735 | Author: Daniel P. Berrange <berrange@redhat.com> | ||
736 | Date: Mon Oct 9 14:43:42 2017 +0100 | ||
737 | |||
738 | io: monitor encoutput buffer size from websocket GSource | ||
739 | |||
740 | This new general memory usage flaw has been assigned CVE-2017-15124, and is | ||
741 | partially fixed by this patch. | ||
742 | |||
743 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
744 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
745 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
746 | Message-id: 20171218191228.31018-10-berrange@redhat.com | ||
747 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
748 | --- | ||
749 | ui/vnc.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------- | ||
750 | ui/vnc.h | 6 ++++++ | ||
751 | 2 files changed, 70 insertions(+), 8 deletions(-) | ||
752 | |||
753 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
754 | index 4ba7fc076a..9e03cc7c01 100644 | ||
755 | --- a/ui/vnc.c | ||
756 | +++ b/ui/vnc.c | ||
757 | @@ -60,6 +60,7 @@ static QTAILQ_HEAD(, VncDisplay) vnc_displays = | ||
758 | |||
759 | static int vnc_cursor_define(VncState *vs); | ||
760 | static void vnc_release_modifiers(VncState *vs); | ||
761 | +static void vnc_update_throttle_offset(VncState *vs); | ||
762 | |||
763 | static void vnc_set_share_mode(VncState *vs, VncShareMode mode) | ||
764 | { | ||
765 | @@ -766,6 +767,7 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl, | ||
766 | vnc_set_area_dirty(vs->dirty, vd, 0, 0, | ||
767 | vnc_width(vd), | ||
768 | vnc_height(vd)); | ||
769 | + vnc_update_throttle_offset(vs); | ||
770 | } | ||
771 | } | ||
772 | |||
773 | @@ -961,16 +963,67 @@ static int find_and_clear_dirty_height(VncState *vs, | ||
774 | return h; | ||
775 | } | ||
776 | |||
777 | +/* | ||
778 | + * Figure out how much pending data we should allow in the output | ||
779 | + * buffer before we throttle incremental display updates, and/or | ||
780 | + * drop audio samples. | ||
781 | + * | ||
782 | + * We allow for equiv of 1 full display's worth of FB updates, | ||
783 | + * and 1 second of audio samples. If audio backlog was larger | ||
784 | + * than that the client would already suffering awful audio | ||
785 | + * glitches, so dropping samples is no worse really). | ||
786 | + */ | ||
787 | +static void vnc_update_throttle_offset(VncState *vs) | ||
788 | +{ | ||
789 | + size_t offset = | ||
790 | + vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel; | ||
791 | + | ||
792 | + if (vs->audio_cap) { | ||
793 | + int freq = vs->as.freq; | ||
794 | + /* We don't limit freq when reading settings from client, so | ||
795 | + * it could be upto MAX_INT in size. 48khz is a sensible | ||
796 | + * upper bound for trustworthy clients */ | ||
797 | + int bps; | ||
798 | + if (freq > 48000) { | ||
799 | + freq = 48000; | ||
800 | + } | ||
801 | + switch (vs->as.fmt) { | ||
802 | + default: | ||
803 | + case AUD_FMT_U8: | ||
804 | + case AUD_FMT_S8: | ||
805 | + bps = 1; | ||
806 | + break; | ||
807 | + case AUD_FMT_U16: | ||
808 | + case AUD_FMT_S16: | ||
809 | + bps = 2; | ||
810 | + break; | ||
811 | + case AUD_FMT_U32: | ||
812 | + case AUD_FMT_S32: | ||
813 | + bps = 4; | ||
814 | + break; | ||
815 | + } | ||
816 | + offset += freq * bps * vs->as.nchannels; | ||
817 | + } | ||
818 | + | ||
819 | + /* Put a floor of 1MB on offset, so that if we have a large pending | ||
820 | + * buffer and the display is resized to a small size & back again | ||
821 | + * we don't suddenly apply a tiny send limit | ||
822 | + */ | ||
823 | + offset = MAX(offset, 1024 * 1024); | ||
824 | + | ||
825 | + vs->throttle_output_offset = offset; | ||
826 | +} | ||
827 | + | ||
828 | static bool vnc_should_update(VncState *vs) | ||
829 | { | ||
830 | switch (vs->update) { | ||
831 | case VNC_STATE_UPDATE_NONE: | ||
832 | break; | ||
833 | case VNC_STATE_UPDATE_INCREMENTAL: | ||
834 | - /* Only allow incremental updates if the output buffer | ||
835 | - * is empty, or if audio capture is enabled. | ||
836 | + /* Only allow incremental updates if the pending send queue | ||
837 | + * is less than the permitted threshold | ||
838 | */ | ||
839 | - if (!vs->output.offset || vs->audio_cap) { | ||
840 | + if (vs->output.offset < vs->throttle_output_offset) { | ||
841 | return true; | ||
842 | } | ||
843 | break; | ||
844 | @@ -1084,11 +1137,13 @@ static void audio_capture(void *opaque, void *buf, int size) | ||
845 | VncState *vs = opaque; | ||
846 | |||
847 | vnc_lock_output(vs); | ||
848 | - vnc_write_u8(vs, VNC_MSG_SERVER_QEMU); | ||
849 | - vnc_write_u8(vs, VNC_MSG_SERVER_QEMU_AUDIO); | ||
850 | - vnc_write_u16(vs, VNC_MSG_SERVER_QEMU_AUDIO_DATA); | ||
851 | - vnc_write_u32(vs, size); | ||
852 | - vnc_write(vs, buf, size); | ||
853 | + if (vs->output.offset < vs->throttle_output_offset) { | ||
854 | + vnc_write_u8(vs, VNC_MSG_SERVER_QEMU); | ||
855 | + vnc_write_u8(vs, VNC_MSG_SERVER_QEMU_AUDIO); | ||
856 | + vnc_write_u16(vs, VNC_MSG_SERVER_QEMU_AUDIO_DATA); | ||
857 | + vnc_write_u32(vs, size); | ||
858 | + vnc_write(vs, buf, size); | ||
859 | + } | ||
860 | vnc_unlock_output(vs); | ||
861 | vnc_flush(vs); | ||
862 | } | ||
863 | @@ -2288,6 +2343,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len) | ||
864 | break; | ||
865 | } | ||
866 | |||
867 | + vnc_update_throttle_offset(vs); | ||
868 | vnc_read_when(vs, protocol_client_msg, 1); | ||
869 | return 0; | ||
870 | } | ||
871 | diff --git a/ui/vnc.h b/ui/vnc.h | ||
872 | index b9d310e640..8fe69595c6 100644 | ||
873 | --- a/ui/vnc.h | ||
874 | +++ b/ui/vnc.h | ||
875 | @@ -298,6 +298,12 @@ struct VncState | ||
876 | |||
877 | VncClientInfo *info; | ||
878 | |||
879 | + /* We allow multiple incremental updates or audio capture | ||
880 | + * samples to be queued in output buffer, provided the | ||
881 | + * buffer size doesn't exceed this threshold. The value | ||
882 | + * is calculating dynamically based on framebuffer size | ||
883 | + * and audio sample settings in vnc_update_throttle_offset() */ | ||
884 | + size_t throttle_output_offset; | ||
885 | Buffer output; | ||
886 | Buffer input; | ||
887 | /* current output mode information */ | ||
888 | -- | ||
889 | 2.11.0 | ||
890 | |||
891 | |||
892 | From ada8d2e4369ea49677d8672ac81bce73eefd5b54 Mon Sep 17 00:00:00 2001 | ||
893 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
894 | Date: Mon, 18 Dec 2017 19:12:25 +0000 | ||
895 | Subject: [PATCH 11/14] ui: fix VNC client throttling when forced update is | ||
896 | requested | ||
897 | MIME-Version: 1.0 | ||
898 | Content-Type: text/plain; charset=UTF-8 | ||
899 | Content-Transfer-Encoding: 8bit | ||
900 | |||
901 | The VNC server must throttle data sent to the client to prevent the 'output' | ||
902 | buffer size growing without bound, if the client stops reading data off the | ||
903 | socket (either maliciously or due to stalled/slow network connection). | ||
904 | |||
905 | The current throttling is very crude because it simply checks whether the | ||
906 | output buffer offset is zero. This check is disabled if the client has requested | ||
907 | a forced update, because we want to send these as soon as possible. | ||
908 | |||
909 | As a result, the VNC client can cause QEMU to allocate arbitrary amounts of RAM. | ||
910 | They can first start something in the guest that triggers lots of framebuffer | ||
911 | updates eg play a youtube video. Then repeatedly send full framebuffer update | ||
912 | requests, but never read data back from the server. This can easily make QEMU's | ||
913 | VNC server send buffer consume 100MB of RAM per second, until the OOM killer | ||
914 | starts reaping processes (hopefully the rogue QEMU process, but it might pick | ||
915 | others...). | ||
916 | |||
917 | To address this we make the throttling more intelligent, so we can throttle | ||
918 | full updates. When we get a forced update request, we keep track of exactly how | ||
919 | much data we put on the output buffer. We will not process a subsequent forced | ||
920 | update request until this data has been fully sent on the wire. We always allow | ||
921 | one forced update request to be in flight, regardless of what data is queued | ||
922 | for incremental updates or audio data. The slight complication is that we do | ||
923 | not initially know how much data an update will send, as this is done in the | ||
924 | background by the VNC job thread. So we must track the fact that the job thread | ||
925 | has an update pending, and not process any further updates until this job is | ||
926 | has been completed & put data on the output buffer. | ||
927 | |||
928 | This unbounded memory growth affects all VNC server configurations supported by | ||
929 | QEMU, with no workaround possible. The mitigating factor is that it can only be | ||
930 | triggered by a client that has authenticated with the VNC server, and who is | ||
931 | able to trigger a large quantity of framebuffer updates or audio samples from | ||
932 | the guest OS. Mostly they'll just succeed in getting the OOM killer to kill | ||
933 | their own QEMU process, but its possible other processes can get taken out as | ||
934 | collateral damage. | ||
935 | |||
936 | This is a more general variant of the similar unbounded memory usage flaw in | ||
937 | the websockets server, that was previously assigned CVE-2017-15268, and fixed | ||
938 | in 2.11 by: | ||
939 | |||
940 | commit a7b20a8efa28e5f22c26c06cd06c2f12bc863493 | ||
941 | Author: Daniel P. Berrange <berrange@redhat.com> | ||
942 | Date: Mon Oct 9 14:43:42 2017 +0100 | ||
943 | |||
944 | io: monitor encoutput buffer size from websocket GSource | ||
945 | |||
946 | This new general memory usage flaw has been assigned CVE-2017-15124, and is | ||
947 | partially fixed by this patch. | ||
948 | |||
949 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
950 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
951 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
952 | Message-id: 20171218191228.31018-11-berrange@redhat.com | ||
953 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
954 | --- | ||
955 | ui/vnc-auth-sasl.c | 5 +++++ | ||
956 | ui/vnc-jobs.c | 5 +++++ | ||
957 | ui/vnc.c | 28 ++++++++++++++++++++++++---- | ||
958 | ui/vnc.h | 7 +++++++ | ||
959 | 4 files changed, 41 insertions(+), 4 deletions(-) | ||
960 | |||
961 | diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c | ||
962 | index 761493b9b2..8c1cdde3db 100644 | ||
963 | --- a/ui/vnc-auth-sasl.c | ||
964 | +++ b/ui/vnc-auth-sasl.c | ||
965 | @@ -79,6 +79,11 @@ long vnc_client_write_sasl(VncState *vs) | ||
966 | |||
967 | vs->sasl.encodedOffset += ret; | ||
968 | if (vs->sasl.encodedOffset == vs->sasl.encodedLength) { | ||
969 | + if (vs->sasl.encodedRawLength >= vs->force_update_offset) { | ||
970 | + vs->force_update_offset = 0; | ||
971 | + } else { | ||
972 | + vs->force_update_offset -= vs->sasl.encodedRawLength; | ||
973 | + } | ||
974 | vs->output.offset -= vs->sasl.encodedRawLength; | ||
975 | vs->sasl.encoded = NULL; | ||
976 | vs->sasl.encodedOffset = vs->sasl.encodedLength = 0; | ||
977 | diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c | ||
978 | index f7867771ae..e326679dd0 100644 | ||
979 | --- a/ui/vnc-jobs.c | ||
980 | +++ b/ui/vnc-jobs.c | ||
981 | @@ -152,6 +152,11 @@ void vnc_jobs_consume_buffer(VncState *vs) | ||
982 | vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL); | ||
983 | } | ||
984 | buffer_move(&vs->output, &vs->jobs_buffer); | ||
985 | + | ||
986 | + if (vs->job_update == VNC_STATE_UPDATE_FORCE) { | ||
987 | + vs->force_update_offset = vs->output.offset; | ||
988 | + } | ||
989 | + vs->job_update = VNC_STATE_UPDATE_NONE; | ||
990 | } | ||
991 | flush = vs->ioc != NULL && vs->abort != true; | ||
992 | vnc_unlock_output(vs); | ||
993 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
994 | index 9e03cc7c01..4805ac41d0 100644 | ||
995 | --- a/ui/vnc.c | ||
996 | +++ b/ui/vnc.c | ||
997 | @@ -1021,14 +1021,28 @@ static bool vnc_should_update(VncState *vs) | ||
998 | break; | ||
999 | case VNC_STATE_UPDATE_INCREMENTAL: | ||
1000 | /* Only allow incremental updates if the pending send queue | ||
1001 | - * is less than the permitted threshold | ||
1002 | + * is less than the permitted threshold, and the job worker | ||
1003 | + * is completely idle. | ||
1004 | */ | ||
1005 | - if (vs->output.offset < vs->throttle_output_offset) { | ||
1006 | + if (vs->output.offset < vs->throttle_output_offset && | ||
1007 | + vs->job_update == VNC_STATE_UPDATE_NONE) { | ||
1008 | return true; | ||
1009 | } | ||
1010 | break; | ||
1011 | case VNC_STATE_UPDATE_FORCE: | ||
1012 | - return true; | ||
1013 | + /* Only allow forced updates if the pending send queue | ||
1014 | + * does not contain a previous forced update, and the | ||
1015 | + * job worker is completely idle. | ||
1016 | + * | ||
1017 | + * Note this means we'll queue a forced update, even if | ||
1018 | + * the output buffer size is otherwise over the throttle | ||
1019 | + * output limit. | ||
1020 | + */ | ||
1021 | + if (vs->force_update_offset == 0 && | ||
1022 | + vs->job_update == VNC_STATE_UPDATE_NONE) { | ||
1023 | + return true; | ||
1024 | + } | ||
1025 | + break; | ||
1026 | } | ||
1027 | return false; | ||
1028 | } | ||
1029 | @@ -1096,8 +1110,9 @@ static int vnc_update_client(VncState *vs, int has_dirty) | ||
1030 | } | ||
1031 | } | ||
1032 | |||
1033 | - vnc_job_push(job); | ||
1034 | + vs->job_update = vs->update; | ||
1035 | vs->update = VNC_STATE_UPDATE_NONE; | ||
1036 | + vnc_job_push(job); | ||
1037 | vs->has_dirty = 0; | ||
1038 | return n; | ||
1039 | } | ||
1040 | @@ -1332,6 +1347,11 @@ static ssize_t vnc_client_write_plain(VncState *vs) | ||
1041 | if (!ret) | ||
1042 | return 0; | ||
1043 | |||
1044 | + if (ret >= vs->force_update_offset) { | ||
1045 | + vs->force_update_offset = 0; | ||
1046 | + } else { | ||
1047 | + vs->force_update_offset -= ret; | ||
1048 | + } | ||
1049 | buffer_advance(&vs->output, ret); | ||
1050 | |||
1051 | if (vs->output.offset == 0) { | ||
1052 | diff --git a/ui/vnc.h b/ui/vnc.h | ||
1053 | index 8fe69595c6..3f4cd4d93d 100644 | ||
1054 | --- a/ui/vnc.h | ||
1055 | +++ b/ui/vnc.h | ||
1056 | @@ -271,6 +271,7 @@ struct VncState | ||
1057 | |||
1058 | VncDisplay *vd; | ||
1059 | VncStateUpdate update; /* Most recent pending request from client */ | ||
1060 | + VncStateUpdate job_update; /* Currently processed by job thread */ | ||
1061 | int has_dirty; | ||
1062 | uint32_t features; | ||
1063 | int absolute; | ||
1064 | @@ -298,6 +299,12 @@ struct VncState | ||
1065 | |||
1066 | VncClientInfo *info; | ||
1067 | |||
1068 | + /* Job thread bottom half has put data for a forced update | ||
1069 | + * into the output buffer. This offset points to the end of | ||
1070 | + * the update data in the output buffer. This lets us determine | ||
1071 | + * when a force update is fully sent to the client, allowing | ||
1072 | + * us to process further forced updates. */ | ||
1073 | + size_t force_update_offset; | ||
1074 | /* We allow multiple incremental updates or audio capture | ||
1075 | * samples to be queued in output buffer, provided the | ||
1076 | * buffer size doesn't exceed this threshold. The value | ||
1077 | -- | ||
1078 | 2.11.0 | ||
1079 | |||
1080 | |||
1081 | From f887cf165db20f405cb8805c716bd363aaadf815 Mon Sep 17 00:00:00 2001 | ||
1082 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
1083 | Date: Mon, 18 Dec 2017 19:12:26 +0000 | ||
1084 | Subject: [PATCH 12/14] ui: place a hard cap on VNC server output buffer size | ||
1085 | MIME-Version: 1.0 | ||
1086 | Content-Type: text/plain; charset=UTF-8 | ||
1087 | Content-Transfer-Encoding: 8bit | ||
1088 | |||
1089 | The previous patches fix problems with throttling of forced framebuffer updates | ||
1090 | and audio data capture that would cause the QEMU output buffer size to grow | ||
1091 | without bound. Those fixes are graceful in that once the client catches up with | ||
1092 | reading data from the server, everything continues operating normally. | ||
1093 | |||
1094 | There is some data which the server sends to the client that is impractical to | ||
1095 | throttle. Specifically there are various pseudo framebuffer update encodings to | ||
1096 | inform the client of things like desktop resizes, pointer changes, audio | ||
1097 | playback start/stop, LED state and so on. These generally only involve sending | ||
1098 | a very small amount of data to the client, but a malicious guest might be able | ||
1099 | to do things that trigger these changes at a very high rate. Throttling them is | ||
1100 | not practical as missed or delayed events would cause broken behaviour for the | ||
1101 | client. | ||
1102 | |||
1103 | This patch thus takes a more forceful approach of setting an absolute upper | ||
1104 | bound on the amount of data we permit to be present in the output buffer at | ||
1105 | any time. The previous patch set a threshold for throttling the output buffer | ||
1106 | by allowing an amount of data equivalent to one complete framebuffer update and | ||
1107 | one seconds worth of audio data. On top of this it allowed for one further | ||
1108 | forced framebuffer update to be queued. | ||
1109 | |||
1110 | To be conservative, we thus take that throttling threshold and multiply it by | ||
1111 | 5 to form an absolute upper bound. If this bound is hit during vnc_write() we | ||
1112 | forceably disconnect the client, refusing to queue further data. This limit is | ||
1113 | high enough that it should never be hit unless a malicious client is trying to | ||
1114 | exploit the sever, or the network is completely saturated preventing any sending | ||
1115 | of data on the socket. | ||
1116 | |||
1117 | This completes the fix for CVE-2017-15124 started in the previous patches. | ||
1118 | |||
1119 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
1120 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
1121 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
1122 | Message-id: 20171218191228.31018-12-berrange@redhat.com | ||
1123 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
1124 | --- | ||
1125 | ui/vnc.c | 29 +++++++++++++++++++++++++++++ | ||
1126 | 1 file changed, 29 insertions(+) | ||
1127 | |||
1128 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
1129 | index 4805ac41d0..e53e84587a 100644 | ||
1130 | --- a/ui/vnc.c | ||
1131 | +++ b/ui/vnc.c | ||
1132 | @@ -1521,8 +1521,37 @@ gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED, | ||
1133 | } | ||
1134 | |||
1135 | |||
1136 | +/* | ||
1137 | + * Scale factor to apply to vs->throttle_output_offset when checking for | ||
1138 | + * hard limit. Worst case normal usage could be x2, if we have a complete | ||
1139 | + * incremental update and complete forced update in the output buffer. | ||
1140 | + * So x3 should be good enough, but we pick x5 to be conservative and thus | ||
1141 | + * (hopefully) never trigger incorrectly. | ||
1142 | + */ | ||
1143 | +#define VNC_THROTTLE_OUTPUT_LIMIT_SCALE 5 | ||
1144 | + | ||
1145 | void vnc_write(VncState *vs, const void *data, size_t len) | ||
1146 | { | ||
1147 | + if (vs->disconnecting) { | ||
1148 | + return; | ||
1149 | + } | ||
1150 | + /* Protection against malicious client/guest to prevent our output | ||
1151 | + * buffer growing without bound if client stops reading data. This | ||
1152 | + * should rarely trigger, because we have earlier throttling code | ||
1153 | + * which stops issuing framebuffer updates and drops audio data | ||
1154 | + * if the throttle_output_offset value is exceeded. So we only reach | ||
1155 | + * this higher level if a huge number of pseudo-encodings get | ||
1156 | + * triggered while data can't be sent on the socket. | ||
1157 | + * | ||
1158 | + * NB throttle_output_offset can be zero during early protocol | ||
1159 | + * handshake, or from the job thread's VncState clone | ||
1160 | + */ | ||
1161 | + if (vs->throttle_output_offset != 0 && | ||
1162 | + vs->output.offset > (vs->throttle_output_offset * | ||
1163 | + VNC_THROTTLE_OUTPUT_LIMIT_SCALE)) { | ||
1164 | + vnc_disconnect_start(vs); | ||
1165 | + return; | ||
1166 | + } | ||
1167 | buffer_reserve(&vs->output, len); | ||
1168 | |||
1169 | if (vs->ioc != NULL && buffer_empty(&vs->output)) { | ||
1170 | -- | ||
1171 | 2.11.0 | ||
1172 | |||
1173 | |||
1174 | From 6aa22a29187e1908f5db738d27c64a9efc8d0bfa Mon Sep 17 00:00:00 2001 | ||
1175 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
1176 | Date: Mon, 18 Dec 2017 19:12:27 +0000 | ||
1177 | Subject: [PATCH 13/14] ui: add trace events related to VNC client throttling | ||
1178 | MIME-Version: 1.0 | ||
1179 | Content-Type: text/plain; charset=UTF-8 | ||
1180 | Content-Transfer-Encoding: 8bit | ||
1181 | |||
1182 | The VNC client throttling is quite subtle so will benefit from having trace | ||
1183 | points available for live debugging. | ||
1184 | |||
1185 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
1186 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
1187 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
1188 | Message-id: 20171218191228.31018-13-berrange@redhat.com | ||
1189 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
1190 | --- | ||
1191 | ui/trace-events | 7 +++++++ | ||
1192 | ui/vnc.c | 23 +++++++++++++++++++++++ | ||
1193 | 2 files changed, 30 insertions(+) | ||
1194 | |||
1195 | diff --git a/ui/trace-events b/ui/trace-events | ||
1196 | index 1a9f126330..85f74f948b 100644 | ||
1197 | --- a/ui/trace-events | ||
1198 | +++ b/ui/trace-events | ||
1199 | @@ -35,6 +35,13 @@ vnc_client_connect(void *state, void *ioc) "VNC client connect state=%p ioc=%p" | ||
1200 | vnc_client_disconnect_start(void *state, void *ioc) "VNC client disconnect start state=%p ioc=%p" | ||
1201 | vnc_client_disconnect_finish(void *state, void *ioc) "VNC client disconnect finish state=%p ioc=%p" | ||
1202 | vnc_client_io_wrap(void *state, void *ioc, const char *type) "VNC client I/O wrap state=%p ioc=%p type=%s" | ||
1203 | +vnc_client_throttle_threshold(void *state, void *ioc, size_t oldoffset, size_t offset, int client_width, int client_height, int bytes_per_pixel, void *audio_cap) "VNC client throttle threshold state=%p ioc=%p oldoffset=%zu newoffset=%zu width=%d height=%d bpp=%d audio=%p" | ||
1204 | +vnc_client_throttle_incremental(void *state, void *ioc, int job_update, size_t offset) "VNC client throttle incremental state=%p ioc=%p job-update=%d offset=%zu" | ||
1205 | +vnc_client_throttle_forced(void *state, void *ioc, int job_update, size_t offset) "VNC client throttle forced state=%p ioc=%p job-update=%d offset=%zu" | ||
1206 | +vnc_client_throttle_audio(void *state, void *ioc, size_t offset) "VNC client throttle audio state=%p ioc=%p offset=%zu" | ||
1207 | +vnc_client_unthrottle_forced(void *state, void *ioc) "VNC client unthrottle forced offset state=%p ioc=%p" | ||
1208 | +vnc_client_unthrottle_incremental(void *state, void *ioc, size_t offset) "VNC client unthrottle incremental state=%p ioc=%p offset=%zu" | ||
1209 | +vnc_client_output_limit(void *state, void *ioc, size_t offset, size_t threshold) "VNC client output limit state=%p ioc=%p offset=%zu threshold=%zu" | ||
1210 | vnc_auth_init(void *display, int websock, int auth, int subauth) "VNC auth init state=%p websock=%d auth=%d subauth=%d" | ||
1211 | vnc_auth_start(void *state, int method) "VNC client auth start state=%p method=%d" | ||
1212 | vnc_auth_pass(void *state, int method) "VNC client auth passed state=%p method=%d" | ||
1213 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
1214 | index e53e84587a..0a5e629d5d 100644 | ||
1215 | --- a/ui/vnc.c | ||
1216 | +++ b/ui/vnc.c | ||
1217 | @@ -1011,6 +1011,12 @@ static void vnc_update_throttle_offset(VncState *vs) | ||
1218 | */ | ||
1219 | offset = MAX(offset, 1024 * 1024); | ||
1220 | |||
1221 | + if (vs->throttle_output_offset != offset) { | ||
1222 | + trace_vnc_client_throttle_threshold( | ||
1223 | + vs, vs->ioc, vs->throttle_output_offset, offset, vs->client_width, | ||
1224 | + vs->client_height, vs->client_pf.bytes_per_pixel, vs->audio_cap); | ||
1225 | + } | ||
1226 | + | ||
1227 | vs->throttle_output_offset = offset; | ||
1228 | } | ||
1229 | |||
1230 | @@ -1028,6 +1034,8 @@ static bool vnc_should_update(VncState *vs) | ||
1231 | vs->job_update == VNC_STATE_UPDATE_NONE) { | ||
1232 | return true; | ||
1233 | } | ||
1234 | + trace_vnc_client_throttle_incremental( | ||
1235 | + vs, vs->ioc, vs->job_update, vs->output.offset); | ||
1236 | break; | ||
1237 | case VNC_STATE_UPDATE_FORCE: | ||
1238 | /* Only allow forced updates if the pending send queue | ||
1239 | @@ -1042,6 +1050,8 @@ static bool vnc_should_update(VncState *vs) | ||
1240 | vs->job_update == VNC_STATE_UPDATE_NONE) { | ||
1241 | return true; | ||
1242 | } | ||
1243 | + trace_vnc_client_throttle_forced( | ||
1244 | + vs, vs->ioc, vs->job_update, vs->force_update_offset); | ||
1245 | break; | ||
1246 | } | ||
1247 | return false; | ||
1248 | @@ -1158,6 +1168,8 @@ static void audio_capture(void *opaque, void *buf, int size) | ||
1249 | vnc_write_u16(vs, VNC_MSG_SERVER_QEMU_AUDIO_DATA); | ||
1250 | vnc_write_u32(vs, size); | ||
1251 | vnc_write(vs, buf, size); | ||
1252 | + } else { | ||
1253 | + trace_vnc_client_throttle_audio(vs, vs->ioc, vs->output.offset); | ||
1254 | } | ||
1255 | vnc_unlock_output(vs); | ||
1256 | vnc_flush(vs); | ||
1257 | @@ -1328,6 +1340,7 @@ ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen) | ||
1258 | */ | ||
1259 | static ssize_t vnc_client_write_plain(VncState *vs) | ||
1260 | { | ||
1261 | + size_t offset; | ||
1262 | ssize_t ret; | ||
1263 | |||
1264 | #ifdef CONFIG_VNC_SASL | ||
1265 | @@ -1348,11 +1361,19 @@ static ssize_t vnc_client_write_plain(VncState *vs) | ||
1266 | return 0; | ||
1267 | |||
1268 | if (ret >= vs->force_update_offset) { | ||
1269 | + if (vs->force_update_offset != 0) { | ||
1270 | + trace_vnc_client_unthrottle_forced(vs, vs->ioc); | ||
1271 | + } | ||
1272 | vs->force_update_offset = 0; | ||
1273 | } else { | ||
1274 | vs->force_update_offset -= ret; | ||
1275 | } | ||
1276 | + offset = vs->output.offset; | ||
1277 | buffer_advance(&vs->output, ret); | ||
1278 | + if (offset >= vs->throttle_output_offset && | ||
1279 | + vs->output.offset < vs->throttle_output_offset) { | ||
1280 | + trace_vnc_client_unthrottle_incremental(vs, vs->ioc, vs->output.offset); | ||
1281 | + } | ||
1282 | |||
1283 | if (vs->output.offset == 0) { | ||
1284 | if (vs->ioc_tag) { | ||
1285 | @@ -1549,6 +1570,8 @@ void vnc_write(VncState *vs, const void *data, size_t len) | ||
1286 | if (vs->throttle_output_offset != 0 && | ||
1287 | vs->output.offset > (vs->throttle_output_offset * | ||
1288 | VNC_THROTTLE_OUTPUT_LIMIT_SCALE)) { | ||
1289 | + trace_vnc_client_output_limit(vs, vs->ioc, vs->output.offset, | ||
1290 | + vs->throttle_output_offset); | ||
1291 | vnc_disconnect_start(vs); | ||
1292 | return; | ||
1293 | } | ||
1294 | -- | ||
1295 | 2.11.0 | ||
1296 | |||
1297 | |||
1298 | From 30b80fd5269257f55203b7072c505b4ebaab5115 Mon Sep 17 00:00:00 2001 | ||
1299 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
1300 | Date: Mon, 18 Dec 2017 19:12:28 +0000 | ||
1301 | Subject: [PATCH 14/14] ui: mix misleading comments & return types of VNC I/O | ||
1302 | helper methods | ||
1303 | MIME-Version: 1.0 | ||
1304 | Content-Type: text/plain; charset=UTF-8 | ||
1305 | Content-Transfer-Encoding: 8bit | ||
1306 | |||
1307 | While the QIOChannel APIs for reading/writing data return ssize_t, with negative | ||
1308 | value indicating an error, the VNC code passes this return value through the | ||
1309 | vnc_client_io_error() method. This detects the error condition, disconnects the | ||
1310 | client and returns 0 to indicate error. Thus all the VNC helper methods should | ||
1311 | return size_t (unsigned), and misleading comments which refer to the possibility | ||
1312 | of negative return values need fixing. | ||
1313 | |||
1314 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
1315 | Reviewed-by: Darren Kenny <darren.kenny@oracle.com> | ||
1316 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
1317 | Message-id: 20171218191228.31018-14-berrange@redhat.com | ||
1318 | Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> | ||
1319 | --- | ||
1320 | ui/vnc-auth-sasl.c | 8 ++++---- | ||
1321 | ui/vnc-auth-sasl.h | 4 ++-- | ||
1322 | ui/vnc.c | 29 +++++++++++++++-------------- | ||
1323 | ui/vnc.h | 6 +++--- | ||
1324 | 4 files changed, 24 insertions(+), 23 deletions(-) | ||
1325 | |||
1326 | diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c | ||
1327 | index 8c1cdde3db..74a5f513f2 100644 | ||
1328 | --- a/ui/vnc-auth-sasl.c | ||
1329 | +++ b/ui/vnc-auth-sasl.c | ||
1330 | @@ -48,9 +48,9 @@ void vnc_sasl_client_cleanup(VncState *vs) | ||
1331 | } | ||
1332 | |||
1333 | |||
1334 | -long vnc_client_write_sasl(VncState *vs) | ||
1335 | +size_t vnc_client_write_sasl(VncState *vs) | ||
1336 | { | ||
1337 | - long ret; | ||
1338 | + size_t ret; | ||
1339 | |||
1340 | VNC_DEBUG("Write SASL: Pending output %p size %zd offset %zd " | ||
1341 | "Encoded: %p size %d offset %d\n", | ||
1342 | @@ -106,9 +106,9 @@ long vnc_client_write_sasl(VncState *vs) | ||
1343 | } | ||
1344 | |||
1345 | |||
1346 | -long vnc_client_read_sasl(VncState *vs) | ||
1347 | +size_t vnc_client_read_sasl(VncState *vs) | ||
1348 | { | ||
1349 | - long ret; | ||
1350 | + size_t ret; | ||
1351 | uint8_t encoded[4096]; | ||
1352 | const char *decoded; | ||
1353 | unsigned int decodedLen; | ||
1354 | diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h | ||
1355 | index b9d8de1c10..2ae224ee3a 100644 | ||
1356 | --- a/ui/vnc-auth-sasl.h | ||
1357 | +++ b/ui/vnc-auth-sasl.h | ||
1358 | @@ -65,8 +65,8 @@ struct VncDisplaySASL { | ||
1359 | |||
1360 | void vnc_sasl_client_cleanup(VncState *vs); | ||
1361 | |||
1362 | -long vnc_client_read_sasl(VncState *vs); | ||
1363 | -long vnc_client_write_sasl(VncState *vs); | ||
1364 | +size_t vnc_client_read_sasl(VncState *vs); | ||
1365 | +size_t vnc_client_write_sasl(VncState *vs); | ||
1366 | |||
1367 | void start_auth_sasl(VncState *vs); | ||
1368 | |||
1369 | diff --git a/ui/vnc.c b/ui/vnc.c | ||
1370 | index 0a5e629d5d..665a143578 100644 | ||
1371 | --- a/ui/vnc.c | ||
1372 | +++ b/ui/vnc.c | ||
1373 | @@ -1272,7 +1272,7 @@ void vnc_disconnect_finish(VncState *vs) | ||
1374 | g_free(vs); | ||
1375 | } | ||
1376 | |||
1377 | -ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp) | ||
1378 | +size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp) | ||
1379 | { | ||
1380 | if (ret <= 0) { | ||
1381 | if (ret == 0) { | ||
1382 | @@ -1315,9 +1315,9 @@ void vnc_client_error(VncState *vs) | ||
1383 | * | ||
1384 | * Returns the number of bytes written, which may be less than | ||
1385 | * the requested 'datalen' if the socket would block. Returns | ||
1386 | - * -1 on error, and disconnects the client socket. | ||
1387 | + * 0 on I/O error, and disconnects the client socket. | ||
1388 | */ | ||
1389 | -ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen) | ||
1390 | +size_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen) | ||
1391 | { | ||
1392 | Error *err = NULL; | ||
1393 | ssize_t ret; | ||
1394 | @@ -1335,13 +1335,13 @@ ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen) | ||
1395 | * will switch the FD poll() handler back to read monitoring. | ||
1396 | * | ||
1397 | * Returns the number of bytes written, which may be less than | ||
1398 | - * the buffered output data if the socket would block. Returns | ||
1399 | - * -1 on error, and disconnects the client socket. | ||
1400 | + * the buffered output data if the socket would block. Returns | ||
1401 | + * 0 on I/O error, and disconnects the client socket. | ||
1402 | */ | ||
1403 | -static ssize_t vnc_client_write_plain(VncState *vs) | ||
1404 | +static size_t vnc_client_write_plain(VncState *vs) | ||
1405 | { | ||
1406 | size_t offset; | ||
1407 | - ssize_t ret; | ||
1408 | + size_t ret; | ||
1409 | |||
1410 | #ifdef CONFIG_VNC_SASL | ||
1411 | VNC_DEBUG("Write Plain: Pending output %p size %zd offset %zd. Wait SSF %d\n", | ||
1412 | @@ -1442,9 +1442,9 @@ void vnc_read_when(VncState *vs, VncReadEvent *func, size_t expecting) | ||
1413 | * | ||
1414 | * Returns the number of bytes read, which may be less than | ||
1415 | * the requested 'datalen' if the socket would block. Returns | ||
1416 | - * -1 on error, and disconnects the client socket. | ||
1417 | + * 0 on I/O error or EOF, and disconnects the client socket. | ||
1418 | */ | ||
1419 | -ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen) | ||
1420 | +size_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen) | ||
1421 | { | ||
1422 | ssize_t ret; | ||
1423 | Error *err = NULL; | ||
1424 | @@ -1460,12 +1460,13 @@ ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen) | ||
1425 | * when not using any SASL SSF encryption layers. Will read as much | ||
1426 | * data as possible without blocking. | ||
1427 | * | ||
1428 | - * Returns the number of bytes read. Returns -1 on error, and | ||
1429 | - * disconnects the client socket. | ||
1430 | + * Returns the number of bytes read, which may be less than | ||
1431 | + * the requested 'datalen' if the socket would block. Returns | ||
1432 | + * 0 on I/O error or EOF, and disconnects the client socket. | ||
1433 | */ | ||
1434 | -static ssize_t vnc_client_read_plain(VncState *vs) | ||
1435 | +static size_t vnc_client_read_plain(VncState *vs) | ||
1436 | { | ||
1437 | - ssize_t ret; | ||
1438 | + size_t ret; | ||
1439 | VNC_DEBUG("Read plain %p size %zd offset %zd\n", | ||
1440 | vs->input.buffer, vs->input.capacity, vs->input.offset); | ||
1441 | buffer_reserve(&vs->input, 4096); | ||
1442 | @@ -1491,7 +1492,7 @@ static void vnc_jobs_bh(void *opaque) | ||
1443 | */ | ||
1444 | static int vnc_client_read(VncState *vs) | ||
1445 | { | ||
1446 | - ssize_t ret; | ||
1447 | + size_t ret; | ||
1448 | |||
1449 | #ifdef CONFIG_VNC_SASL | ||
1450 | if (vs->sasl.conn && vs->sasl.runSSF) | ||
1451 | diff --git a/ui/vnc.h b/ui/vnc.h | ||
1452 | index 3f4cd4d93d..0c33a5f7fe 100644 | ||
1453 | --- a/ui/vnc.h | ||
1454 | +++ b/ui/vnc.h | ||
1455 | @@ -524,8 +524,8 @@ gboolean vnc_client_io(QIOChannel *ioc, | ||
1456 | GIOCondition condition, | ||
1457 | void *opaque); | ||
1458 | |||
1459 | -ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen); | ||
1460 | -ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen); | ||
1461 | +size_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen); | ||
1462 | +size_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen); | ||
1463 | |||
1464 | /* Protocol I/O functions */ | ||
1465 | void vnc_write(VncState *vs, const void *data, size_t len); | ||
1466 | @@ -544,7 +544,7 @@ uint32_t read_u32(uint8_t *data, size_t offset); | ||
1467 | |||
1468 | /* Protocol stage functions */ | ||
1469 | void vnc_client_error(VncState *vs); | ||
1470 | -ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp); | ||
1471 | +size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp); | ||
1472 | |||
1473 | void start_client_init(VncState *vs); | ||
1474 | void start_auth_vnc(VncState *vs); | ||
1475 | -- | ||
1476 | 2.11.0 | ||
diff --git a/meta/recipes-devtools/qemu/qemu_2.11.0.bb b/meta/recipes-devtools/qemu/qemu_2.11.1.bb index 10760dc2db..f0e20cc125 100644 --- a/meta/recipes-devtools/qemu/qemu_2.11.0.bb +++ b/meta/recipes-devtools/qemu/qemu_2.11.1.bb | |||
@@ -22,7 +22,6 @@ SRC_URI = "http://wiki.qemu-project.org/download/${BP}.tar.bz2 \ | |||
22 | file://apic-fixup-fallthrough-to-PIC.patch \ | 22 | file://apic-fixup-fallthrough-to-PIC.patch \ |
23 | file://linux-user-Fix-webkitgtk-hangs-on-32-bit-x86-target.patch \ | 23 | file://linux-user-Fix-webkitgtk-hangs-on-32-bit-x86-target.patch \ |
24 | file://memfd.patch \ | 24 | file://memfd.patch \ |
25 | file://CVE-2017-15124.patch \ | ||
26 | " | 25 | " |
27 | UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+\..*)\.tar" | 26 | UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+\..*)\.tar" |
28 | 27 | ||
@@ -32,8 +31,8 @@ SRC_URI_append_class-native = " \ | |||
32 | file://cpus.c-qemu_cpu_kick_thread_debugging.patch \ | 31 | file://cpus.c-qemu_cpu_kick_thread_debugging.patch \ |
33 | " | 32 | " |
34 | 33 | ||
35 | SRC_URI[md5sum] = "335994a755bc655e88a87aeb36bfc0b9" | 34 | SRC_URI[md5sum] = "61cf862b6007eba4ac98247776af2e27" |
36 | SRC_URI[sha256sum] = "c4f034c7665a84a1c3be72c8da37f3c31ec063475699df062ab646d8b2e17fcb" | 35 | SRC_URI[sha256sum] = "d9df2213ceed32e91dab7bc9dd19c1af83f91ba72c7aeef7605dfaaf81732ccb" |
37 | 36 | ||
38 | COMPATIBLE_HOST_mipsarchn32 = "null" | 37 | COMPATIBLE_HOST_mipsarchn32 = "null" |
39 | COMPATIBLE_HOST_mipsarchn64 = "null" | 38 | COMPATIBLE_HOST_mipsarchn64 = "null" |