summaryrefslogtreecommitdiffstats
path: root/ui/vnc.c
Commit message (Collapse)AuthorAgeFilesLines
...
* ui: Convert vnc_display_init(), init_keyboard_layout() to ErrorFei Li2018-10-191-5/+10
| | | | | | | | | Signed-off-by: Fei Li <fli@suse.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181017082702.5581-27-armbru@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
* vnc: call sasl_server_init() only when requiredMarc-André Lureau2018-10-011-7/+8
| | | | | | | | | | | | | | | | | | | | | | | VNC server is calling sasl_server_init() during startup of QEMU, even if SASL auth has not been enabled. This may create undesirable warnings like "Could not find keytab file: /etc/qemu/krb5.tab" when the user didn't configure SASL on host and started VNC server. Instead, only initialize SASL when needed. Note that HMP/QMP "change vnc" calls vnc_display_open() again, which will initialize SASL if needed. Fix assignment in if condition, while touching this code. Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1609327 Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Message-id: 20180907063634.359-1-marcandre.lureau@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* qapi: Drop qapi_event_send_FOO()'s Error ** argumentPeter Xu2018-08-281-4/+3Star
| | | | | | | | | | | | | | | | | | | The generated qapi_event_send_FOO() take an Error ** argument. They can't actually fail, because all they do with the argument is passing it to functions that can't fail: the QObject output visitor, and the @qmp_emit callback, which is either monitor_qapi_event_queue() or event_test_emit(). Drop the argument, and pass &error_abort to the QObject output visitor and @qmp_emit instead. Suggested-by: Eric Blake <eblake@redhat.com> Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> Message-Id: <20180815133747.25032-4-peterx@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Commit message rewritten, update to qapi-code-gen.txt corrected] Signed-off-by: Markus Armbruster <armbru@redhat.com>
* ui/vnc: Remove useless parenthesis around DIV_ROUND_UP macroPhilippe Mathieu-Daudé2018-08-241-1/+2
| | | | | | | | | | | | Patch created mechanically by rerunning: $ spatch --sp-file scripts/coccinelle/round.cocci \ --macro-file scripts/cocci-macro-file.h \ --dir . --in-place Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <20180704153919.12432-7-f4bug@amsat.org> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* vnc: remove support for deprecated tls, x509, x509verify optionsDaniel P. Berrangé2018-08-241-91/+0Star
| | | | | | | | | | | | The 'tls-creds' option accepts the name of a TLS credentials object. This replaced the usage of 'tls', 'x509' and 'x509verify' options in 2.5.0. These deprecated options were grandfathered in when the deprecation policy was introduded in 2.10.0, so can now finally be removed. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-id: 20180725092751.21767-3-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* vnc: add magic cookie to VncStateGerd Hoffmann2018-05-151-1/+9
| | | | | | | | Set magic cookie on initialization. Clear on cleanup. Sprinkle a bunch of assert()s checking the cookie, to verify the pointer is valid. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20180507102254.12107-1-kraxel@redhat.com
* vnc: fix use-after-freeGerd Hoffmann2018-04-271-2/+3
| | | | | | | | | | | | | | | When vnc_client_read() return value is -1 vs is not valid any more. Fixes: d49b87f0d1e0520443a990fc610d0f02bc63c556 Reported-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180420084820.3873-1-kraxel@redhat.com
* vnc: deal with surface NULL pointersGerd Hoffmann2018-03-121-0/+10
| | | | | | | | | | | | | | | | | | | | | Secondary displays in multihead setups are allowed to have a NULL DisplaySurface. Typically user interfaces handle this by hiding the window which shows the display in question. This isn't an option for vnc though because it simply hasn't a concept of windows or outputs. So handle the situation by showing a placeholder DisplaySurface instead. Also check in console_select whenever a surface is preset in the first place before requesting an update. This fixes a segfault which can be triggered by switching to an unused display (via vtrl-alt-<nr>) in a multihead setup, for example using -device virtio-vga,max_outputs=2. Cc: Christian Borntraeger <borntraeger@de.ibm.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> Message-id: 20180308161803.6152-1-kraxel@redhat.com
* qapi: Move qapi-schema.json to qapi/, rename generated filesMarkus Armbruster2018-03-021-1/+1
| | | | | | | | | | | | | | | | | | | Move qapi-schema.json to qapi/, so it's next to its modules, and all files get generated to qapi/, not just the ones generated for modules. Consistently name the generated files qapi-MODULE.EXT: qmp-commands.[ch] become qapi-commands.[ch], qapi-event.[ch] become qapi-events.[ch], and qmp-introspect.[ch] become qapi-introspect.[ch]. This gets rid of the temporary hacks in scripts/qapi/commands.py, scripts/qapi/events.py, and scripts/qapi/common.py. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180211093607.27351-28-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> [eblake: Fix trailing dot in tpm.c, undo temporary hack for OSX toolchain] Signed-off-by: Eric Blake <eblake@redhat.com>
* Include less of the generated modular QAPI headersMarkus Armbruster2018-03-021-1/+1
| | | | | | | | | | | | | | | | | | | | | In my "build everything" tree, a change to the types in qapi-schema.json triggers a recompile of about 4800 out of 5100 objects. The previous commit split up qmp-commands.h, qmp-event.h, qmp-visit.h, qapi-types.h. Each of these headers still includes all its shards. Reduce compile time by including just the shards we actually need. To illustrate the benefits: adding a type to qapi/migration.json now recompiles some 2300 instead of 4800 objects. The next commit will improve it further. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180211093607.27351-24-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> [eblake: rebase to master] Signed-off-by: Eric Blake <eblake@redhat.com>
* keymap: consider modifier state when picking a mappingGerd Hoffmann2018-02-221-2/+7
| | | | | | | | | | Pass the modifier state to the keymap lookup function. In case multiple keysym -> keycode mappings exist look at the modifier state and prefer the mapping where the modifier state matches. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Message-id: 20180222070513.8740-6-kraxel@redhat.com
* ui: check VNC audio frequency limit at time of reading from clientDaniel P. Berrangé2018-02-161-9/+13
| | | | | | | | | | | | | | The 'vs->as.freq' value is a signed integer, which is read from an unsigned 32-bit int field on the wire. There is thus a risk of overflow on 32-bit platforms. Move the frequency limit checking to be done at time of read before casting to a signed integer. Reported-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180205114938.15784-4-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: avoid risk of 32-bit int overflow in VNC buffer checkDaniel P. Berrangé2018-02-161-2/+2
| | | | | | | | | | | | | | | | For very large framebuffers, it is theoretically possible for the result of 'vs->throttle_output_offset * VNC_THROTTLE_OUTPUT_LIMIT_SCALE' to exceed the size of a 32-bit int. For this to happen in practice, the video RAM would have to be set to a large enough value, which is not likely today. None the less we can be paranoid against future growth by using division instead of multiplication when checking the limits. Reported-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180205114938.15784-2-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* vnc: fix segfault in closed connection handlingKlim Kireev2018-02-161-1/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On one of our client's node, due to trying to read from closed ioc, a segmentation fault occured. Corresponding backtrace: 0 object_get_class (obj=obj@entry=0x0) 1 qio_channel_readv_full (ioc=0x0, iov=0x7ffe55277180 ... 2 qio_channel_read (ioc=<optimized out> ... 3 vnc_client_read_buf (vs=vs@entry=0x55625f3c6000, ... 4 vnc_client_read_plain (vs=0x55625f3c6000) 5 vnc_client_read (vs=0x55625f3c6000) 6 vnc_client_io (ioc=<optimized out>, condition=G_IO_IN, ... 7 g_main_dispatch (context=0x556251568a50) 8 g_main_context_dispatch (context=context@entry=0x556251568a50) 9 glib_pollfds_poll () 10 os_host_main_loop_wait (timeout=<optimized out>) 11 main_loop_wait (nonblocking=nonblocking@entry=0) 12 main_loop () at vl.c:1909 13 main (argc=<optimized out>, argv=<optimized out>, ... Having analyzed the coredump, I understood that the reason is that ioc_tag is reset on vnc_disconnect_start and ioc is cleaned in vnc_disconnect_finish. Between these two events due to some reasons the ioc_tag was set again and after vnc_disconnect_finish the handler is running with freed ioc, which led to the segmentation fault. The patch checks vs->disconnecting in places where we call qio_channel_add_watch and resets handler if disconnecting == TRUE to prevent such an occurrence. Signed-off-by: Klim Kireev <klim.kireev@virtuozzo.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Message-id: 20180207094844.21402-1-klim.kireev@virtuozzo.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* vnc: remove bogus object_unref on client socketDaniel P. Berrangé2018-02-161-1/+0Star
| | | | | | | | | | | | vnc_listen_io() does not own the reference on the 'cioc' parameter is it passed, so should not be unref'ing it. Fixes: 13e1d0e71e78a925848258391a6e616b6b5ae219 Reported-by: Bandan Das <bsd@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 20180215102602.10864-1-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* Move include qemu/option.h from qemu-common.h to actual usersMarkus Armbruster2018-02-091-0/+1
| | | | | | | | | | | | | | | | | | qemu-common.h includes qemu/option.h, but most places that include the former don't actually need the latter. Drop the include, and add it to the places that actually need it. While there, drop superfluous includes of both headers, and separate #include from file comment with a blank line. This cleanup makes the number of objects depending on qemu/option.h drop from 4545 (out of 4743) to 284 in my "build everything" tree. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-20-armbru@redhat.com> [Semantic conflict with commit bdd6a90a9e in block/nvme.c resolved]
* Eliminate qapi/qmp/types.hMarkus Armbruster2018-02-091-1/+0Star
| | | | | | | | | | | | | | qapi/qmp/types.h is a convenience header to include a number of qapi/qmp/ headers. Since we rarely need all of the headers qapi/qmp/types.h includes, we bypass it most of the time. Most of the places that use it don't need all the headers, either. Include the necessary headers directly, and drop qapi/qmp/types.h. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-9-armbru@redhat.com>
* Drop superfluous includes of qapi/qmp/qerror.hMarkus Armbruster2018-02-091-1/+0Star
| | | | | | | Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-6-armbru@redhat.com>
* Include qapi/error.h exactly where neededMarkus Armbruster2018-02-091-0/+1
| | | | | | | | | | | | | | This cleanup makes the number of objects depending on qapi/error.h drop from 1910 (out of 4743) to 1612 in my "build everything" tree. While there, separate #include from file comment with a blank line, and drop a useless comment on why qemu/osdep.h is included first. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-5-armbru@redhat.com> [Semantic conflict with commit 34e304e975 resolved, OSX breakage fixed]
* ui: convert VNC server to QIONetListenerDaniel P. Berrange2018-02-021-140/+55Star
| | | | | | | | | | | The VNC server already has the ability to listen on multiple sockets. Converting it to use the QIONetListener APIs though, will reduce the amount of code in the VNC server and improve the clarity of what is left. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 20180201164514.10330-1-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: avoid sign extension using client width/heightDaniel P. Berrange2018-01-251-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Pixman returns a signed int for the image width/height, but the VNC protocol only permits a unsigned int16. Effective framebuffer size is determined by the guest, limited by the video RAM size, so the dimensions are unlikely to exceed the range of an unsigned int16, but this is not currently validated. With the current use of 'int' for client width/height, the calculation of offsets in vnc_update_throttle_offset() suffers from integer size promotion and sign extension, causing coverity warnings *** CID 1385147: Integer handling issues (SIGN_EXTENSION) /ui/vnc.c: 979 in vnc_update_throttle_offset() 973 * than that the client would already suffering awful audio 974 * glitches, so dropping samples is no worse really). 975 */ 976 static void vnc_update_throttle_offset(VncState *vs) 977 { 978 size_t offset = >>> CID 1385147: Integer handling issues (SIGN_EXTENSION) >>> Suspicious implicit sign extension: "vs->client_pf.bytes_per_pixel" with type "unsigned char" (8 bits, unsigned) is promoted in "vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel" to type "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits, unsigned). If "vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel" is greater than 0x7FFFFFFF, the upper bits of the result will all be 1. 979 vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel; Change client_width / client_height to be a size_t to avoid sign extension and integer promotion. Then validate that dimensions are in range wrt the RFB protocol u16 limits. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 20180118155254.17053-1-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: mix misleading comments & return types of VNC I/O helper methodsDaniel P. Berrange2018-01-121-14/+15
| | | | | | | | | | | | | | | While the QIOChannel APIs for reading/writing data return ssize_t, with negative value indicating an error, the VNC code passes this return value through the vnc_client_io_error() method. This detects the error condition, disconnects the client and returns 0 to indicate error. Thus all the VNC helper methods should return size_t (unsigned), and misleading comments which refer to the possibility of negative return values need fixing. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-14-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: add trace events related to VNC client throttlingDaniel P. Berrange2018-01-121-0/+23
| | | | | | | | | | | The VNC client throttling is quite subtle so will benefit from having trace points available for live debugging. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-13-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: place a hard cap on VNC server output buffer sizeDaniel P. Berrange2018-01-121-0/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous patches fix problems with throttling of forced framebuffer updates and audio data capture that would cause the QEMU output buffer size to grow without bound. Those fixes are graceful in that once the client catches up with reading data from the server, everything continues operating normally. There is some data which the server sends to the client that is impractical to throttle. Specifically there are various pseudo framebuffer update encodings to inform the client of things like desktop resizes, pointer changes, audio playback start/stop, LED state and so on. These generally only involve sending a very small amount of data to the client, but a malicious guest might be able to do things that trigger these changes at a very high rate. Throttling them is not practical as missed or delayed events would cause broken behaviour for the client. This patch thus takes a more forceful approach of setting an absolute upper bound on the amount of data we permit to be present in the output buffer at any time. The previous patch set a threshold for throttling the output buffer by allowing an amount of data equivalent to one complete framebuffer update and one seconds worth of audio data. On top of this it allowed for one further forced framebuffer update to be queued. To be conservative, we thus take that throttling threshold and multiply it by 5 to form an absolute upper bound. If this bound is hit during vnc_write() we forceably disconnect the client, refusing to queue further data. This limit is high enough that it should never be hit unless a malicious client is trying to exploit the sever, or the network is completely saturated preventing any sending of data on the socket. This completes the fix for CVE-2017-15124 started in the previous patches. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-12-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: fix VNC client throttling when forced update is requestedDaniel P. Berrange2018-01-121-4/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The VNC server must throttle data sent to the client to prevent the 'output' buffer size growing without bound, if the client stops reading data off the socket (either maliciously or due to stalled/slow network connection). The current throttling is very crude because it simply checks whether the output buffer offset is zero. This check is disabled if the client has requested a forced update, because we want to send these as soon as possible. As a result, the VNC client can cause QEMU to allocate arbitrary amounts of RAM. They can first start something in the guest that triggers lots of framebuffer updates eg play a youtube video. Then repeatedly send full framebuffer update requests, but never read data back from the server. This can easily make QEMU's VNC server send buffer consume 100MB of RAM per second, until the OOM killer starts reaping processes (hopefully the rogue QEMU process, but it might pick others...). To address this we make the throttling more intelligent, so we can throttle full updates. When we get a forced update request, we keep track of exactly how much data we put on the output buffer. We will not process a subsequent forced update request until this data has been fully sent on the wire. We always allow one forced update request to be in flight, regardless of what data is queued for incremental updates or audio data. The slight complication is that we do not initially know how much data an update will send, as this is done in the background by the VNC job thread. So we must track the fact that the job thread has an update pending, and not process any further updates until this job is has been completed & put data on the output buffer. This unbounded memory growth affects all VNC server configurations supported by QEMU, with no workaround possible. The mitigating factor is that it can only be triggered by a client that has authenticated with the VNC server, and who is able to trigger a large quantity of framebuffer updates or audio samples from the guest OS. Mostly they'll just succeed in getting the OOM killer to kill their own QEMU process, but its possible other processes can get taken out as collateral damage. This is a more general variant of the similar unbounded memory usage flaw in the websockets server, that was previously assigned CVE-2017-15268, and fixed in 2.11 by: commit a7b20a8efa28e5f22c26c06cd06c2f12bc863493 Author: Daniel P. Berrange <berrange@redhat.com> Date: Mon Oct 9 14:43:42 2017 +0100 io: monitor encoutput buffer size from websocket GSource This new general memory usage flaw has been assigned CVE-2017-15124, and is partially fixed by this patch. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-11-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: fix VNC client throttling when audio capture is activeDaniel P. Berrange2018-01-121-8/+64
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The VNC server must throttle data sent to the client to prevent the 'output' buffer size growing without bound, if the client stops reading data off the socket (either maliciously or due to stalled/slow network connection). The current throttling is very crude because it simply checks whether the output buffer offset is zero. This check must be disabled if audio capture is enabled, because when streaming audio the output buffer offset will rarely be zero due to queued audio data, and so this would starve framebuffer updates. As a result, the VNC client can cause QEMU to allocate arbitrary amounts of RAM. They can first start something in the guest that triggers lots of framebuffer updates eg play a youtube video. Then enable audio capture, and simply never read data back from the server. This can easily make QEMU's VNC server send buffer consume 100MB of RAM per second, until the OOM killer starts reaping processes (hopefully the rogue QEMU process, but it might pick others...). To address this we make the throttling more intelligent, so we can throttle when audio capture is active too. To determine how to throttle incremental updates or audio data, we calculate a size threshold. Normally the threshold is the approximate number of bytes associated with a single complete framebuffer update. ie width * height * bytes per pixel. We'll send incremental updates until we hit this threshold, at which point we'll stop sending updates until data has been written to the wire, causing the output buffer offset to fall back below the threshold. If audio capture is enabled, we increase the size of the threshold to also allow for upto 1 seconds worth of audio data samples. ie nchannels * bytes per sample * frequency. This allows the output buffer to have a mixture of incremental framebuffer updates and audio data queued, but once the threshold is exceeded, audio data will be dropped and incremental updates will be throttled. This unbounded memory growth affects all VNC server configurations supported by QEMU, with no workaround possible. The mitigating factor is that it can only be triggered by a client that has authenticated with the VNC server, and who is able to trigger a large quantity of framebuffer updates or audio samples from the guest OS. Mostly they'll just succeed in getting the OOM killer to kill their own QEMU process, but its possible other processes can get taken out as collateral damage. This is a more general variant of the similar unbounded memory usage flaw in the websockets server, that was previously assigned CVE-2017-15268, and fixed in 2.11 by: commit a7b20a8efa28e5f22c26c06cd06c2f12bc863493 Author: Daniel P. Berrange <berrange@redhat.com> Date: Mon Oct 9 14:43:42 2017 +0100 io: monitor encoutput buffer size from websocket GSource This new general memory usage flaw has been assigned CVE-2017-15124, and is partially fixed by this patch. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-10-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: refactor code for determining if an update should be sent to the clientDaniel P. Berrange2018-01-121-7/+20
| | | | | | | | | | | | The logic for determining if it is possible to send an update to the client will become more complicated shortly, so pull it out into a separate method for easier extension later. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-9-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: correctly reset framebuffer update state after processing dirty regionsDaniel P. Berrange2018-01-121-1/+1
| | | | | | | | | | | | | | | | | | | | | | | According to the RFB protocol, a client sends one or more framebuffer update requests to the server. The server can reply with a single framebuffer update response, that covers all previously received requests. Once the client has read this update from the server, it may send further framebuffer update requests to monitor future changes. The client is free to delay sending the framebuffer update request if it needs to throttle the amount of data it is reading from the server. The QEMU VNC server, however, has never correctly handled the framebuffer update requests. Once QEMU has received an update request, it will continue to send client updates forever, even if the client hasn't asked for further updates. This prevents the client from throttling back data it gets from the server. This change fixes the flawed logic such that after a set of updates are sent out, QEMU waits for a further update request before sending more data. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-8-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: introduce enum to track VNC client framebuffer update request stateDaniel P. Berrange2018-01-121-10/+11
| | | | | | | | | | | | Currently the VNC servers tracks whether a client has requested an incremental or forced update with two boolean flags. There are only really 3 distinct states to track, so create an enum to more accurately reflect permitted states. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-7-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: avoid pointless VNC updates if framebuffer isn't dirtyDaniel P. Berrange2018-01-121-1/+1
| | | | | | | | | | | | | | | The vnc_update_client() method checks the 'has_dirty' flag to see if there are dirty regions that are pending to send to the client. Regardless of this flag, if a forced update is requested, updates must be sent. For unknown reasons though, the code also tries to sent updates if audio capture is enabled. This makes no sense as audio capture state does not impact framebuffer contents, so this check is removed. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-5-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: remove redundant indentation in vnc_client_updateDaniel P. Berrange2018-01-121-55/+57
| | | | | | | | | | | Now that previous dead / unreachable code has been removed, we can simplify the indentation in the vnc_client_update method. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-4-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: remove unreachable code in vnc_update_clientDaniel P. Berrange2018-01-121-5/+1Star
| | | | | | | | | | | | | | | | | | | | | | | A previous commit: commit 5a8be0f73d6f60ff08746377eb09ca459f39deab Author: Gerd Hoffmann <kraxel@redhat.com> Date: Wed Jul 13 12:21:20 2016 +0200 vnc: make sure we finish disconnect Added a check for vs->disconnecting at the very start of the vnc_update_client method. This means that the very next "if" statement check for !vs->disconnecting always evaluates true, and is thus redundant. This in turn means the vs->disconnecting check at the very end of the method never evaluates true, and is thus unreachable code. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-3-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: remove 'sync' parameter from vnc_update_clientDaniel P. Berrange2018-01-121-8/+3Star
| | | | | | | | | | | There is only one caller of vnc_update_client and that always passes false for the 'sync' parameter. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171218191228.31018-2-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* vnc: fix debug spellingMarc-André Lureau2018-01-121-1/+1
| | | | | | Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20171220140618.12701-1-marcandre.lureau@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: Always remove an old VNC channel watch before adding a new oneBrandon Carpenter2017-10-041-0/+4
| | | | | | | | Also set saved handle to zero when removing without adding a new watch. Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
* ui: add tracing of VNC authentication processDaniel P. Berrange2017-09-291-18/+18
| | | | | | | | | Trace anything related to authentication in the VNC protocol handshake Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 20170921121528.23935-3-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: add tracing of VNC operations related to QIOChannelDaniel P. Berrange2017-09-291-3/+8
| | | | | | | | | Trace anything which opens/closes/wraps a QIOChannel in the VNC server. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 20170921121528.23935-2-berrange@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* qapi: Mechanically convert FOO_lookup[...] to FOO_str(...)Markus Armbruster2017-09-041-3/+3
| | | | | | Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1503564371-26090-14-git-send-email-armbru@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
* vnc: use DIV_ROUND_UPMarc-André Lureau2017-08-311-1/+1
| | | | | | | | I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Richard Henderson <rth@twiddle.net>
* vnc: use QEMU_ALIGN_DOWNMarc-André Lureau2017-08-311-4/+4
| | | | | | | | I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Richard Henderson <rth@twiddle.net>
* ui/vnc: fix leak of SocketAddress **Philippe Mathieu-Daudé2017-07-311-18/+18
| | | | | | | | | | Extract the (correct) cleaning code as a new function vnc_free_addresses() then use it to remove the memory leaks. Reported-by: Clang Static Analyzer Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
* vnc: Set default kbd delay to 10msAlexander Graf2017-07-171-1/+1
| | | | | | | | | | | | | | | | | | The current VNC default keyboard delay is 1ms. With that we're constantly typing faster than the guest receives keyboard events from an XHCI attached USB HID device. The default keyboard delay time in the input layer however is 10ms. I don't know how that number came to be, but empirical tests on some OpenQA driven ARM systems show that 10ms really is a reasonable default number for the delay. This patch moves the VNC delay also to 10ms. That way our default is much safer (good!) and also consistent with the input layer default (also good!). Signed-off-by: Alexander Graf <agraf@suse.de> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1499863425-103133-1-git-send-email-agraf@suse.de Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: Remove inclusion of "hw/qdev.h"Thomas Huth2017-06-211-1/+0Star
| | | | | | | | Looks like #include "hw/qdev.h" is not needed here, so remove it. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-id: 1497894617-12143-1-git-send-email-thuth@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* vnc: replace hweight_long() with ctpopl()Cédric Le Goater2017-05-121-3/+3
| | | | | | | | | | ctpopl() has a better implementation than hweight_long() and ui/vnc.c being the last user of hweight_long(), we can simply remove it. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1489415605-13105-1-git-send-email-clg@kaod.org Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* ui: Support non-zero minimum values for absolute input axesPhilippe Voinov2017-05-111-2/+2
| | | | | | | | | | This patch refactors ui/input.c to support absolute axis minimum values other than 0. All dependent calls to qemu_input_queue_abs have been updated to explicitly supply 0 as the axis minimum value. Signed-off-by: Philippe Voinov <philippevoinov@gmail.com> Message-id: 20170505133952.29885-1-philippevoinov@gmail.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* sockets: Limit SocketAddressLegacy to external interfacesMarkus Armbruster2017-05-091-70/+68Star
| | | | | | | | | | | | | | | | | SocketAddressLegacy is a simple union, and simple unions are awkward: they have their variant members wrapped in a "data" object on the wire, and require additional indirections in C. SocketAddress is the equivalent flat union. Convert all users of SocketAddressLegacy to SocketAddress, except for existing external interfaces. See also commit fce5d53..9445673 and 85a82e8..c5f1ae3. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1493192202-3184-7-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Minor editing accident fixed, commit message and a comment tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
* sockets: Rename SocketAddress to SocketAddressLegacyMarkus Armbruster2017-05-091-52/+52
| | | | | | | | | | | | | | | The next commit will rename SocketAddressFlat to SocketAddress, and the commit after that will replace most uses of SocketAddressLegacy by SocketAddress, replacing most of this commit's renames right back. Note that checkpatch emits a few "line over 80 characters" warnings. The long lines are all temporary; the SocketAddressLegacy replacement will shorten them again. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1493192202-3184-5-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
* io vnc sockets: Clean up SocketAddressKind switchesMarkus Armbruster2017-04-031-6/+12
| | | | | | | | | | | | | | | | | | We have quite a few switches over SocketAddressKind. Some have case labels for all enumeration values, others rely on a default label. Some abort when the value isn't a valid SocketAddressKind, others report an error then. Unify as follows. Always provide case labels for all enumeration values, to clarify intent. Abort when the value isn't a valid SocketAddressKind, because the program state is messed up then. Improve a few error messages while there. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 1490895797-29094-4-git-send-email-armbru@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
* nbd sockets vnc: Mark problematic address family tests TODOMarkus Armbruster2017-04-031-0/+1
| | | | | | | | | | | | | | | | | | | | Certain features make sense only with certain address families. For instance, passing file descriptors requires AF_UNIX. Testing SocketAddress's saddr->type == SOCKET_ADDRESS_KIND_UNIX is obvious, but problematic: it can't recognize AF_UNIX when type == SOCKET_ADDRESS_KIND_FD. Mark such tests of saddr->type TODO. We may want to check the address family with getsockname() there. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1490895797-29094-2-git-send-email-armbru@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
* vnc: allow to connect with add_client when -vnc noneMarc-André Lureau2017-04-031-4/+4
| | | | | | | | | | | | | Do not skip VNC initialization, in particular of auth method when vnc is configured without sockets, since we should still allow connections through QMP add_client. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1434551 Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20170328160646.21250-1-marcandre.lureau@redhat.com Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
generated by cgit v1.2.3-55-g7522 (git 2.39.0) at 2024-12-18 13:37:55 +0100