aboutsummaryrefslogtreecommitdiff
path: root/ui/vnc-jobs.c
AgeCommit message (Collapse)Author
2020-11-04vnc: fix resource leak when websocket channel errorDing Hui
When we connect to vnc by websocket channel, and disconnect (maybe by some network exception) before handshake, qemu will left CLOSE_WAIT socket and never close it After 04d2529da2 ("ui: convert VNC server to use QIOChannelSocket") and dd154c4d9f ("io: fix handling of EOF / error conditions in websock GSource"), the vnc call qio_channel_add_watch only care about G_IO_IN, but mising G_IO_HUP and G_IO_ERR. When the websocket channel get EOF or error, it cannot callback, because the caller ignore the event, that leads to resource leak We need handle G_IO_HUP and G_IO_ERR event, then cleanup the channel Fixes: 04d2529da2 ("ui: convert VNC server to use QIOChannelSocket") Fixes: dd154c4d9f ("io: fix handling of EOF / error conditions in websock GSource") Cc: qemu-stable@nongnu.org Signed-off-by: Ding Hui <dinghui@sangfor.com.cn> Message-id: 20201029032241.11040-1-dinghui@sangfor.com.cn Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2018-08-24vnc: fix memleak of the "vnc-worker-output" namePeter Wu
Fixes repeated memory leaks of 18 bytes when using VNC: Direct leak of 831024 byte(s) in 46168 object(s) allocated from: ... #4 0x7f6d2f919bdd in g_strdup_vprintf glib/gstrfuncs.c:514 #5 0x56085cdcf660 in buffer_init util/buffer.c:59 #6 0x56085ca6a7ec in vnc_async_encoding_start ui/vnc-jobs.c:177 #7 0x56085ca6b815 in vnc_worker_thread_loop ui/vnc-jobs.c:240 Fixes: 543b95801f98 ("vnc: attach names to buffers") Cc: Gerd Hoffmann <kraxel@redhat.com> CC: qemu-stable@nongnu.org Signed-off-by: Peter Wu <peter@lekensteyn.nl> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 20180807221830.3844-1-peter@lekensteyn.nl Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2018-05-15vnc: add magic cookie to VncStateGerd Hoffmann
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
2018-02-16vnc: fix segfault in closed connection handlingKlim Kireev
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>
2018-01-12ui: fix VNC client throttling when forced update is requestedDaniel P. Berrange
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>
2017-02-08ui/vnc: Drop unused vnc_has_job() and vnc_jobs_clear()Peter Maydell
The functions vnc_has_job() and vnc_jobs_clear() are never used; remove them. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Gonglei <arei.gonglei@huawei.com> Message-id: 1486146260-8092-1-git-send-email-peter.maydell@linaro.org Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2016-02-04ui: Clean up includesPeter Maydell
Clean up includes so that osdep.h is included first and headers which it implies are not included manually. This commit was created with scripts/clean-includes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1454089805-5470-2-git-send-email-peter.maydell@linaro.org
2015-12-18ui: convert VNC server to use QIOChannelSocketDaniel P. Berrange
The minimal first step conversion to use QIOChannelSocket classes instead of directly using POSIX sockets API. This will later be extended to also cover the TLS, SASL and websockets code. Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2015-11-17Merge remote-tracking branch 'remotes/kraxel/tags/pull-vnc-20151116-1' into ↵Peter Maydell
staging vnc: buffer code improvements, bugfixes. # gpg: Signature made Mon 16 Nov 2015 17:20:02 GMT using RSA key ID D3E87138 # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" # gpg: aka "Gerd Hoffmann <gerd@kraxel.org>" # gpg: aka "Gerd Hoffmann (private) <kraxel@gmail.com>" * remotes/kraxel/tags/pull-vnc-20151116-1: vnc: fix mismerge buffer: allow a buffer to shrink gracefully buffer: factor out buffer_adj_size buffer: factor out buffer_req_size vnc: recycle empty vs->output buffer vnc: fix local state init vnc: only alloc server surface with clients connected vnc: use vnc_{width,height} in vnc_set_area_dirty vnc: factor out vnc_update_server_surface vnc: add vnc_width+vnc_height helpers vnc: zap dead code vnc-jobs: move buffer reset, use new buffer move vnc: kill jobs queue buffer vnc: attach names to buffers buffer: add tracing buffer: add buffer_shrink buffer: add buffer_move buffer: add buffer_move_empty buffer: add buffer_init buffer: make the Buffer capacity increase in powers of two Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-11-06ui: Use g_new() & friends where that makes obvious senseMarkus Armbruster
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. This commit only touches allocations with size arguments of the form sizeof(T). Same Coccinelle semantic patch as in commit b45c03f. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2015-11-05vnc: recycle empty vs->output bufferPeter Lieven
If the vs->output buffer is empty it will be dropped by the next qio_buffer_move_empty in vnc_jobs_consume_buffer anyway. So reuse the allocated buffer from this buffer in the worker thread where we otherwise would start with an empty (unallocated buffer). Signed-off-by: Peter Lieven <pl@kamp.de> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1446203414-4013-17-git-send-email-kraxel@redhat.com [ added a comment describing the non-obvious optimization ] Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2015-11-05vnc: fix local state initGerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Peter Lieven <pl@kamp.de> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1446203414-4013-16-git-send-email-kraxel@redhat.com
2015-11-05vnc-jobs: move buffer reset, use new buffer moveGerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Peter Lieven <pl@kamp.de> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1446203414-4013-10-git-send-email-kraxel@redhat.com
2015-11-05vnc: kill jobs queue bufferGerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Peter Lieven <pl@kamp.de> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1446203414-4013-9-git-send-email-kraxel@redhat.com
2015-11-05vnc: attach names to buffersGerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Peter Lieven <pl@kamp.de> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1446203414-4013-8-git-send-email-kraxel@redhat.com
2015-06-22Include monitor/monitor.h exactly where neededMarkus Armbruster
In particular, don't include it into headers. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
2015-03-10ui/vnc: Remove vnc_stop_worker_thread()Thomas Huth
This function is not used anymore, let's remove it. Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2014-03-11Merge remote-tracking branch 'remotes/kraxel/tags/pull-vnc-1' into stagingPeter Maydell
vnc dirty tracking optinizations. various vnc bugfixes. # gpg: Signature made Mon 10 Mar 2014 12:39:54 GMT using RSA key ID D3E87138 # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" # gpg: aka "Gerd Hoffmann <gerd@kraxel.org>" # gpg: aka "Gerd Hoffmann (private) <kraxel@gmail.com>" * remotes/kraxel/tags/pull-vnc-1: ui/vnc: disable adaptive update calculations if not needed ui/vnc: optimize setting in vnc_dpy_update() ui/vnc: optimize clearing in find_and_clear_dirty_height() ui/vnc: optimize dirty bitmap tracking ui/vnc: derive cmp_bytes from VNC_DIRTY_PIXELS_PER_BIT ui/vnc: introduce VNC_DIRTY_PIXELS_PER_BIT macro vnc: fix use-after-free in vnc_update_client_sync vnc: Fix qemu crashed when vnc client disconnect suddenly vnc: Fix tight_detect_smooth_image() for lossless case Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2014-03-10vnc: Fix qemu crashed when vnc client disconnect suddenlyGonglei (Arei)
Hi, When I use RealVNC viewer client (http://www.realvnc.com/) to connect vnc server, the client disconnect suddenly, and I click reconnect button immediately, then the Qemu crashed. In the function vnc_worker_thread_loop, will call vnc_async_encoding_start to set the local vs->output buffer by global queue's buffer. Then send rectangles to the vnc client call function vnc_send_framebuffer_update. Finally, Under normal circumstances, call vnc_async_encoding_end to set the global queue'buffer by the local vs->output conversely. When the vnc client disconnect, the job->vs->csock will be set to -1. And the current prcoess logic will goto disconnected partion without call function vnc_async_encoding_end. But, the function vnc_send_framebuffer_update will call buffer_reserve, which maybe call g_realloc reset the local vs's buffer, meaning the global queue's buffer is modified also. If anyone use the original global queue's buffer memory will cause corruption and then crash qemu. This patch assure the function vnc_async_encoding_end being called even though the vnc client disconnect suddenly. Signed-off-by: Gonglei <arei.gonglei@huawei.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2014-03-09Add a 'name' parameter to qemu_thread_createDr. David Alan Gilbert
If enabled, set the thread name at creation (on GNU systems with pthread_set_np) Fix up all the callers with a thread name Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
2013-03-18vnc: stop using DisplayStateGerd Hoffmann
Rework DisplayStateListener callbacks to not use the DisplayState any more. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2012-12-19misc: move include files to include/qemu/Paolo Bonzini
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2012-11-03Merge branch 'trivial-patches' of git://github.com/stefanha/qemuBlue Swirl
* 'trivial-patches' of git://github.com/stefanha/qemu: pc: Drop redundant test for ROM memory region exec: make some functions static target-ppc: make some functions static ppc: add missing static vnc: add missing static vl.c: add missing static target-sparc: make do_unaligned_access static m68k: Return semihosting errno values correctly cadence_uart: More debug information Conflicts: target-m68k/m68k-semi.c
2012-11-01vnc: add missing staticBlue Swirl
Add missing 'static' qualifiers. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2012-11-01pixman/vnc: use pixman images in vnc.Gerd Hoffmann
The vnc code uses *three* DisplaySurfaces: First is the surface of the actual QemuConsole, usually the guest screen, but could also be a text console (monitor/serial reachable via Ctrl-Alt-<nr> keys). This is left as-is. Second is the current server's view of the screen content. The vnc code uses this to figure which parts of the guest screen did _really_ change to reduce the amount of updates sent to the vnc clients. It is also used as data source when sending out the updates to the clients. This surface gets replaced by a pixman image. The format changes too, instead of using the guest screen format we'll use fixed 32bit rgb framebuffer and convert the pixels on the fly when comparing and updating the server framebuffer. Third surface carries the format expected by the vnc client. That isn't used to store image data. This surface is switched to PixelFormat and a boolean for bigendian byte order. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2012-10-19ui/vnc-jobs.c: Fix minor typos in commentsPeter Maydell
Fix some minor typos/grammar errors in comments. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Stefan Weil <sw@weilnetz.de> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2012-06-27Remove support for non-threaded VNC serverDaniel P. Berrange
QEMU now has a fundamental requirement for pthreads, so there is no compelling reason to retain support for the non-threaded VNC server. Remove the --{enable,disable}-vnc-thread configure arguments, and all CONFIG_VNC_THREAD conditionals Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>