Commit Graph

1299 Commits

Author SHA1 Message Date
Francois Gouget
5331b7cb2b server: Simplify the next chunk initialization in red_replay_data_chunks()
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-01-13 12:08:17 +01:00
Francois Gouget
5a27d06ab4 server: Fix a pointer to uint64_t cast in spice_replay_next_cmd()
This avoids a compilation error with -Werror on 32 bit systems as the
pointer size differs from that of an uint64_t.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
2016-01-13 12:08:17 +01:00
Francois Gouget
f831984888 server: Use '%zu' to print size_t variables
The size_t definition is different between 32 and 64 bit systems so that
neither '%u' nor '%lu' work for both. '%zu' should be used instead.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-01-13 12:08:17 +01:00
Francois Gouget
e8f89571be server: Use PRI macros in printf for 32/64 bit compatibility
Some integer type definitions are different between 32 and 64 bit
systems which causes problems in printf. The PRI macros automatically
provide the printf format appropriate for the system.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-01-13 12:08:17 +01:00
Victor Toso
2c6db246d6 spicevmc: set state of DeviceInstance to NULL
After spice_char_device_state_destroy is called spicevmc should not keep
reference to that memory. state->chardev_st and sin->st point to the
same SpiceCharDeviceState and both should be set to NULL when it is
destroyed.
2016-01-13 12:08:17 +01:00
Victor Toso
14182e29af char-device: set to NULL freed pointers on destroy
As SpiceCharDeviceState is only unref'ed on
spice_char_device_state_destroy the same device could be destroyed more
then once so the pointers that are freed should be set to NULL.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1281455
2016-01-13 12:08:17 +01:00
Victor Toso
e3c541c009 char-device: free all memory pool when no clients
When no client is connect we should not need to keep the memory pool
used by char-device. In most situations this is not significant but
when using webdav this could mean freeing MAX_POOL_SIZE bytes

Related: https://bugs.freedesktop.org/show_bug.cgi?id=91350
2016-01-13 12:08:17 +01:00
Victor Toso
73e1b3c18b char-device: Define a memory pool limit
Otherwise the amount of unused memory could grow while transfering big
chunks of data. This change only means that once the memory was used it
will not be stored again after the limit was reached.

Related: https://bugs.freedesktop.org/show_bug.cgi?id=91350
2016-01-13 12:08:17 +01:00
Victor Toso
565d6f5d6a char-device: fix usage of free/unref on WriteBuffer
There are places were the could should definetly free the
SpiceCharDeviceWriteBuffer and places that it should only unref it. The
current use of spice_char_device_write_buffer_free was missleading.

This patch creates the spice_char_device_write_buffer_unref and properly
call these two functions.

Related: https://bugs.freedesktop.org/show_bug.cgi?id=91350
2016-01-13 12:08:17 +01:00
Lukas Venhoda
bf9fd235b9 dcc: Don't disconnect channel, when compression is not supported
Don't disconnect the display channel, when unsupported compression is
requested from the client. Not changing the compression is enough.

https://bugs.freedesktop.org/show_bug.cgi?id=92821
Acked-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-01-13 12:08:09 +01:00
Pavel Grunt
b172850200 spicevmc: Return early when instance has no device state
Same approach as in spice_server_char_device_wakeup().
Avoid segmentation fault when the webdav channel (spice port channel) is
used with the vnc display:
 #0  0x00007ffff7aab734 in spice_char_device_state_opaque_get (dev=0x0)
     at char_device.c:720
 #1  0x00007ffff7b0850c in spice_server_port_event (sin=<optimized out>, event=<optimized out>) at spicevmc.c:578
 #2  0x0000555555787ba4 in set_guest_connected (port=<optimized out>, guest_connected=1) at hw/char/virtio-console.c:89
 #3  0x0000555555678d7c in control_out (len=<optimized out>, buf=0x55555775c3a0, vser=0x5555578d1540) at /home/pgrunt/RH/qemu/hw/char/virtio-serial-bus.c:404
 #4  0x0000555555678d7c in control_out (vdev=0x5555578d1540, vq=0x555557941bc8)
     at /home/pgrunt/RH/qemu/hw/char/virtio-serial-bus.c:441
 #5  0x000055555588eb98 in aio_dispatch (ctx=0x5555562e1a50) at aio-posix.c:160
 #6  0x00005555558829ee in aio_ctx_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at async.c:226
 #7  0x00007ffff2010e3a in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
 #8  0x000055555588d8fb in main_loop_wait () at main-loop.c:211
 #9  0x000055555588d8fb in main_loop_wait (timeout=<optimized out>)
     at main-loop.c:256
 #10 0x000055555588d8fb in main_loop_wait (nonblocking=<optimized out>)
     at main-loop.c:504
 #11 0x000055555561b664 in main () at vl.c:1891
2016-01-07 18:24:09 +01:00
Pavel Grunt
a21410330d reds: Do not abort due to wrong header
Just prevent the buggy client from connecting.

 #0  0x00007fffe83b2a98 in raise () at /lib64/libc.so.6
 #1  0x00007fffe83b469a in abort () at /lib64/libc.so.6
 #2  0x00007ffff7b1533d in spice_logv (log_domain=0x7ffff7b87226 "Spice", log_level=SPICE_LOG_LEVEL_ERROR, strloc=0x7ffff7b92aba "reds.c:1373", function=0x7ffff7b94f40 <__FUNCTION__.31775> "reds_send_link_ack", format=0x7ffff7b871fe "assertion `%s' failed", args=args@entry=0x7fffffffcb68) at log.c:109
 #3  0x00007ffff7b15468 in spice_log (log_domain=log_domain@entry=0x7ffff7b87226 "Spice", log_level=log_level@entry=SPICE_LOG_LEVEL_ERROR, strloc=strloc@entry=0x7ffff7b92aba "reds.c:1373", function=function@entry=0x7ffff7b94f40 <__FUNCTION__.31775> "reds_send_link_ack", format=format@entry=0x7ffff7b871fe "assertion `%s' failed") at log.c:123
 #4  0x00007ffff7aee335 in reds_handle_read_link_done (link=0x555556b27c70)
     at reds.c:1373
 #5  0x00007ffff7aee335 in reds_handle_read_link_done (opaque=0x555556b27c70)
     at reds.c:2139
 #6  0x000055555588acc6 in qemu_iohandler_poll ()
 #7  0x000055555588a8e1 in main_loop_wait ()
 #8  0x0000555555614064 in main ()

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1281442

Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-01-07 18:23:53 +01:00
Frediano Ziglio
8e60c5df3d replay: better help for -s option
-s accepts an option to delay command queueing.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Marc-André Lureau <mlureau@redhat.com>
2016-01-07 18:18:15 +01:00
Jeremy White
f44a63b8ec Flush display commands before validating the surface.
This fixes a display glitch in xspice which is caused when
a surface create is queued, but then a direct call to update
the area is issued.  Unless we flush the queue, the surface
does not exist, and we fail.

Signed-off-by: Jeremy White <jwhite@codeweavers.com>
2015-12-14 12:44:58 +01:00
Frediano Ziglio
aa09f04717 dcc: do not cause problem with multiple threads.
With multiple cards configured you can have multiple workers running in
different thread.
With such configuration static variables not syncronized could lead
to undefined behavior.

Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2015-12-11 18:56:54 +01:00
Marc-André Lureau
2862f0820d smartcard: include libcacard.h if possible
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
2015-12-11 18:49:10 +01:00
Christophe Fergeau
bfa5e5ce40 build-sys: Adjust to new spice-common spice-deps.m4
This commit also updates the spice-common submodule

Christophe Fergeau (7):
      Add marshaller test case
      build-sys: Use ${PKG_CONFIG} rather than pkg-config
      build-sys: Rework SPICE_CHECK_* m4 macros
      build-sys: Add gio-2.0 to SPICE_CHECK_GLIB2
      build-sys: Fix error in SPICE_CHECK_LZ4 description
      build-sys: Set automake conditional in SPICE_CHECK_SMARTCARD
      build-sys: Rename SUPPORT_GL to HAVE_GL

Javier Celaya (1):
      Fix linearization of several marshallers with one item

Lukas Venhoda (3):
      ssl-verify: Only check addr length when using IP addr
      m4: Require glib version >= 2.22
      ssl-verify: Changed IPv4 hostname to IPv6
2015-12-11 18:42:18 +01:00
Frediano Ziglio
337108469d remove small leak in MJPEG code
cinfo.dest is allocated in spice_jpeg_mem_dest but never freed.
Note that jpeg_destroy_compress does not free this field as is
supposed to be a buffer provided by jpeg caller.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2015-12-11 18:41:58 +01:00
Christophe Fergeau
7c5e5b02ba Add missing license headers 2015-12-11 18:41:19 +01:00
Christophe Fergeau
ecb2428adb syntax-check: Remove unused #include <strings.h> 2015-12-11 18:41:19 +01:00
Christophe Fergeau
80c3b866ff syntax-check: Add missing #include <config.h> 2015-12-11 18:41:09 +01:00
Christophe Fergeau
bb165ac3ab syntax-check: Don't use tabs for indentation 2015-12-11 18:39:49 +01:00
snir sheriber
b3898b4861 fix spelling mistakes in comments (reseting to resetting & dummym to dummy)
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2015-12-11 18:39:31 +01:00
Jeremy White
9e88d66565 Update the .gitignore files for the new manual,
for a few newly generated tests, and for the spice-server.h.
2015-12-11 18:39:27 +01:00
Frediano Ziglio
e60a3beb3c Simplify pointer computation
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2015-10-12 14:11:22 +01:00
Frediano Ziglio
6e3547f8b1 Prevent leak if size from red_get_data_chunks don't match in red_get_image
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2015-10-06 11:11:11 +01:00
Frediano Ziglio
b3be589ab3 Prevent data_size to be set independently from data
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>
2015-10-06 11:11:11 +01:00
Frediano Ziglio
2b6695f122 Avoid race condition copying segments in red_get_path
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>
2015-10-06 11:11:11 +01:00
Frediano Ziglio
2693e0497e Make sure we can read QXLPathSeg structures
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>
2015-10-06 11:11:11 +01:00
Frediano Ziglio
a447c4f2ac Fix some possible overflows in red_get_string for 32 bit
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2015-10-06 11:11:11 +01:00
Frediano Ziglio
7d69184037 Prevent DoS from guest trying to allocate too much data on host for chunks
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>
2015-10-06 11:11:11 +01:00
Frediano Ziglio
f3605979ce Prevent memory leak if red_get_data_chunks_ptr fails
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>
2015-10-06 11:11:11 +01:00
Frediano Ziglio
3738478ed7 Fix race condition in red_get_data_chunks_ptr
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>
2015-10-06 11:11:11 +01:00
Frediano Ziglio
caec52dc77 Fix integer overflow computing glyph_size in red_get_string
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>
2015-10-06 11:11:11 +01:00
Frediano Ziglio
dfaedec789 Fix race condition in red_get_string
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>
2015-10-06 11:11:10 +01:00
Frediano Ziglio
9235c84e0f Fix race in red_get_image
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>
2015-10-06 11:11:10 +01:00
Frediano Ziglio
3dfd1a0828 Fix race condition on red_get_clip_rects
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>
2015-10-06 11:11:10 +01:00
Frediano Ziglio
0f58e9da56 Prevent 32 bit integer overflow in bitmap_consistent
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>
2015-10-06 11:11:10 +01:00
Frediano Ziglio
68a742aaa8 Fix buffer reading overflow
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>
2015-10-06 11:11:10 +01:00
Frediano Ziglio
1eb93baa3c Check properly surface to be created
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>
2015-10-06 11:11:10 +01:00
Frediano Ziglio
ac5f64a80a Fix some integer overflow causing large memory allocations
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>
2015-10-06 11:11:10 +01:00
Frediano Ziglio
0205a6ce63 Define a constant to limit data from guest.
This limit will prevent guest trying to do nasty things and DoS to host.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2015-10-06 11:11:10 +01:00
Frediano Ziglio
097c638b12 worker: avoid double free or double create of surfaces
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>
2015-10-06 11:07:15 +01:00
Frediano Ziglio
dd558bb833 worker: validate correctly surfaces
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>
2015-10-06 11:07:15 +01:00
Marc-André Lureau
f2ea57335e worker: make it clear it returns from process when no cmd
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2015-10-05 14:45:16 +01:00
Marc-André Lureau
5e80266cb9 server: remove useless includes
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2015-10-02 10:13:45 +01:00
Marc-André Lureau
bb969255c9 worker: count in drawable_new() 2015-09-29 18:59:54 +01:00
Jonathon Jongsma
61d458119e PALLET -> PALETTE
Use the correct spelling for the enumeration
2015-09-29 18:59:54 +01:00
Christophe Fergeau
5eaf659aa3 tests: Fix -Werror=format-zero-length build failure
replay.c: In function 'replay_channel_event':
replay.c:226:16: error: zero-length gnu_printf format string
[-Werror=format-zero-length]
     g_printerr("");
2015-09-29 18:59:54 +01:00
Christophe Fergeau
1aa5185de2 display: Advertise preferred compression cap
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.
2015-09-24 11:06:42 +02:00