Do not read multiple times data from guest as this could be changed
by other vcpu threads.
This causes races and security problems if these data are used for
buffer allocation or checks.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Do not read multiple time an array size that can be changed.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
The overflow may lead to buffer overflow as the row size computed from
width (bitmap->x) can be bigger than the size in bytes (bitmap->stride).
This can make spice-server accept the invalid sizes.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Not security risk as just for read.
However, this could be used to attempt integer overflows in the
following lines.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Check format is valid.
Check stride is at least the size of required bytes for a row.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Prevent integer overflow when computing image sizes.
Image index computations are done using 32 bit so this can cause easily
security issues. MAX_DATA_CHUNK is larger than the virtual
card limit, so this is not going to cause change in behaviours.
Comparing size calculation results with MAX_DATA_CHUNK will allow us to
catch overflows.
Prevent guest from allocating large amount of memory.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
A driver can overwrite surface state creating a surface with the same
id of a previous one.
Also can try to destroy surfaces that are not created.
Both requests cause invalid internal states that could lead to crashes
or memory corruptions.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Do not just give warning and continue to use an invalid index into
an array.
Resolves: CVE-2015-5260
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
replay.c: In function 'replay_channel_event':
replay.c:226:16: error: zero-length gnu_printf format string
[-Werror=format-zero-length]
g_printerr("");
The patches adding a way for the client to set its preferred compression
method added a new capability so that the server can indicate support
for this feature. However, spice-server was not setting this capability
on its display channel, which means clients are not going to try to send
'preferred-compression' messages even though the user request it.
virt-manager can add webdav channel for a while now.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
For security reasons do not assume guest do not change structures it
pass to Qemu.
Guest could change count field while Qemu is copying QXLMonitorsConfig
structure leading to heap corruption.
This patch avoid it reading count only once.
This patch solves CVE-2015-3247.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
arch_warn was set to 1 only if architecture is not x86, x64 or arm.
Update the message as we actually mainly test x64.
Define the warning message and do the architecture checks in the
same place so that they are easier to keep in sync.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This patch contains a bit of small optimizations.
It avoid boolean operations which could involve branches replacing
with binary operations (equal/all_ident -> some_differences).
The other optimization avoids the use of ABS. First the way the macro
was used (with a large expression) was not easy to optimize by the
compiler.
Then instead of using ABS a much simpler range check is used so instead
of (ABS(n) >= k) a ((n) <= -k || (n) >= k) is used. This looks small
but modern compilers can translate this not in range check in a couple
of machine instructions (and a single compare).
Using operf on same samples (using spice-server-replay) and trying 2 runs
I got
run 1 2
-------------------------
before 104441 106267
after 92387 91083
So the performance increase is about 13%.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Due to how the MIN macro is defined the function was called twice
unless the compiler could demonstrate that was returning the same
value (which actually is impossible as function as clock_gettime
are not deterministic).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Do not access to timer after we call the associated function.
Some of these callbacks can call spice_timer_remove making the pointer
pointing to freed data.
This happen for instance when the client is disconnecting.
This does not cause memory corruption on current allocator
implementations as all freeing/accessing happen on a single thread quite
closely and allocators use different pools for different thread.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Avoid to use typedef twice for the same type as some compiler
complaints about it.
SpiceTimer and SpiceWatch are defined in server/spice-core.h
as an abstract type which should be defined by some code (as
server/tests/basic_event_loop.c does).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
spice_server_destroy calls reds_exit which is called also at exit time
(is registered with atexit) so avoid to keep dangling pointers.
Currently this does not happen as spice_server_destroy is not called
by Qemu.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
On 32 bit machine timespec->tv_sec (time_t) is 32 bit.
Also 1000 * 1000 * 1000 is 32 bit.
The multiplication of 2 32 bit integers gives a 32 bit integer, however
this can overflow.
Converting the first factor to 64 bit before the multiplication solves
the issue.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
This script was used at make distcheck time to verify consistency of the
version number defined in configure.ac and in spice-server headers.
Since commit ab12cf414c, these 2 version numbers can no longer be out of
sync, so we can drop this script.
Commit 3c6b4e41 removed spice-experimental.h as this header was not
used, nor supposed to be used. However, QEMU had been including it
(without using any of its symbols) until commit v2.3.0-rc0~135^2~1
As this is fairly recent (Nov 2014), building older QEMUs with new
spice-server releases, or even bisecting QEMU will be broken as they
will be looking for a no-longer available header.
This commit readds a spice-experimental.h file, however it only contains
a #warning indicating this file is deprecated. This means older QEMU
will build now, but only if they were configured with --disable-werror.
spice-server headers expose SpiceImageCompression which is only available
from recent spice-protocol releases.
This dependency must be expressed in Requires and not Requires.private
Actually not causing problems as when used is always followed by another
terminator but better to fix the definition.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Mutex/conditional require Glib 2.32 which is not available in RHEL6.
Use plain pthread to make this module compatible with RHEL6.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Fabiano Fidencio <ffidenci@redhat.com>
A logical and (&&) was used instead of a bit one (&).
Was working just as is the only flag defined.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Fabiano Fidencio <ffidenci@redhat.com>
usage: spice-server-replay -p <port> -c <client command line> <cmdfile>
will run the commands from cmdfile ignoring timestamps, right after a
connection is established from the client, and will SIGINT the client
on end of cmdfile, and exit itself after waiting for the client.
spicy-stats from spice-gtk is useful for testing, it prints the summary
of the traffic on each channel.
You can also run with no client by doing:
spice-server-replay <cmdfile>
For example, the 300 MB file (compressed to 4 MB with xz -9) available
at [1] produces the following output:
spicy-stats total bytes read:
total bytes read:
inputs: 214
display: 1968983
cursor: 390
main: 256373
You could run it directly like so:
curl http://annarchy.freedesktop.org/~alon/win7_boot_shutdown.cmd.xz | \
xzcat | server/tests/spice-server-replay -p 12345 -c `which spicy-stats` -
Known Problems:
* Implementation is wrong. Should do a single device->host conversion
(i.e. get_virt), and then marshall/demarshall that (i.e. RedDrawable).
* segfault on file read done resulting in the above spicy-stats not
being reproducable (well, up to 1% yes).
[1] http://annarchy.freedesktop.org/~alon/win7_boot_shutdown.cmd.xz
Now based on glib including using an asyncqueue for reading the playback
file, and proper freeing of the allocated commands, with --slow,
--compression and a progress timer, and doesn't use more then nsurfaces.
Signed-off-by: Alon Levy <alon@pobox.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
if the environment variable in the title is set and can be
opened for writing a log of all display commands (no cursor
commands yet) and any QXLWorker calls (particularily primary
create and destroy) will be logged to that file, and possible
to replay using the replay utility introduced later.
For an example file (4 MB download, 300 MB after unpack with xz,
these 300 MB are themselves reduced from 1.2GB using zlib compression
for any chunk):
(old file without a header)
http://annarchy.freedesktop.org/~alon/win7_boot_shutdown.cmd.xz
Signed-off-by: Alon Levy <alon@pobox.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
