Remove that hideous template header that should really be regular code
since it's specialized and instanciated only for pixmap.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
There was not check for data_size field so one could set data to
a small set of data and data_size much bigger than size of data
leading to buffer overflow.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
The guest can attempt to increase the number of segments while
spice-server is reading them.
Make sure we don't copy more then the allocated segments.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
start pointer points to a QXLPathSeg structure.
Before reading from the structure, make sure the structure is contained
in the memory range checked.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Limit number of chunks to a given amount to avoid guest trying to
allocate too much memory. Using circular or nested chunks lists
guest could try to allocate huge amounts of memory.
Considering the list can be infinite and guest can change data this
also prevents strange security attacks from guest.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Free linked list if client tries to do nasty things
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Do not read multiple times data from guest as this can be changed by
other guest vcpus. This causes races and security problems if these
data are used for buffer allocation or checks.
Actually, the 'data' member can't change during read as it is just a
pointer to a fixed array contained in qxl. However, this change will
make it clear that there can be no race condition.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
If bpp is int the formula can lead to weird overflows. width and height
are uint16_t so the formula is:
size_t = u16 * (u16 * int + const_int) / const_int;
so it became
size_t = (int) u16 * ((int) u16 * int + const_int) / const_int;
However the (int) u16 * (int) u16 can then became negative to overflow.
Under 64 bit architectures size_t is 64 and int usually 32 so converting
this negative 32 bit number to a unsigned 64 bit lead to a very big
number as the signed is extended and then converted to unsigned.
Using unsigned arithmetic prevent extending the sign.
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>
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.
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>
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>
