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
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>
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 a7b20a8efa
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>
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>
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
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>
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>
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>
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
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
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
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
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>
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>
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>
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>
* '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
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>
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>
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>
