When GLZ code attempts to send a 64 bit integer the 8 top bit of
the lower (32 bits) part of the number are stripped due to a bug.
This was discovered by Zhongqiang Huang <useprxf@gmail.com>
Reported-by: Zhongqiang Huang <useprxf@gmail.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
This issue caused the glitches using the rectangular selection
tool in PaintShop 6.
The line was removed accidentally by "red_parse_qxl: fix throwing
away drawables that have masks" (812b65984d)
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pavelgrunt@gmail.com>
Acked-by: Uri Lublin <uril@redhat.com>
It was also possible for a malicious client to set
VDAgentMonitorsConfig::num_of_monitors to a number larger
than the actual size of VDAgentMOnitorsConfig::monitors.
This would lead to buffer overflows, which could allow the guest to
read part of the host memory. This might cause write overflows in the
host as well, but controlling the content of such buffers seems
complicated.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Total message size received from the client was unlimited. There is
a 2kiB size check on individual agent messages, but the MonitorsConfig
message can be split in multiple chunks, and the size of the
non-chunked MonitorsConfig message was never checked. This could easily
lead to memory exhaustion on the host.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
This allows the connection to early fail in case initial bytes
are not correct.
This allows for instance VNC client to graceful fail connecting
to a spice-server. This happens easily as the two protocols
share the same range of ports.
This resolves rhbz#1416692.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Caller is supposed the function return a buffer able to store
size bytes.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
The limits for capabilities are specified using 32 bit unsigned integers.
This could cause possible integer overflows causing buffer overflows.
For instance the sum of num_common_caps and num_caps can be 0 avoiding
additional checks.
As the link message is now capped to 4096 and the capabilities are
contained in the link message limit the capabilities to 1024
(capabilities are expressed in number of uint32_t items).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
The limit for link message is specified using a 32 bit unsigned integer.
This could cause possible DoS due to excessive memory allocations and
some possible crashes.
For instance a value >= 2^31 causes a spice_assert to be triggered in
async_read_handler (reds-stream.c) due to an integer overflow at this
line:
int n = async->end - async->now;
This could be easily triggered with a program like
#!/usr/bin/env python
import socket
import time
from struct import pack
server = '127.0.0.1'
port = 5900
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.connect((server, port))
data = pack('<4sIII', 'REDQ', 2, 2, 0xaaaaaaaa)
s.send(data)
time.sleep(1)
without requiring any authentication (the same can be done
with TLS).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Eventually, during a seamless migration, qemu may finish to migrate
before the spice client even finished to connect all channels to
destination and informed the server. In this case,
main_channel_client_migrate_src_complete() will fall back to
switch-host method, and reds_mig_fill_wait_disconnect() is called to
complete the migration (disconnecting all channels).
reds_mig_cleanup() is called when all channels are disconnected, but
reds->mig_wait_connect is still TRUE, and it will call
migrate_connect_complete() instead of the expected
migrate_end_complete(). Setting reds->mig_wait_connect to FALSE when
reds_mig_fill_wait_disconnect() solves the issue.
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1352836
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
There is no guarantee in the code that this can't be hit, so we should
cope with it (the condition can be reached easily by running the server
without waiting for blocked clients or pipe size)
The following commit will attempt to address this.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
(cherry picked from commit 5c7e248445)
This is related to CVE-2016-0749
==529== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60040009c098 at pc 0x7fffee0eda6d bp 0x7fffffffcd00 sp 0x7fffffffccf0
WRITE of size 4 at 0x60040009c098 thread T0
#0 0x7fffee0eda6c in smartcard_char_device_notify_reader_add /home/elmarco/pkg/spice/spice-0.12.4/server/smartcard.c:334
#1 0x7fffee0ef783 in smartcard_add_reader /home/elmarco/pkg/spice/spice-0.12.4/server/smartcard.c:642
#2 0x7fffee0f0568 in smartcard_channel_handle_message /home/elmarco/pkg/spice/spice-0.12.4/server/smartcard.c:757
#3 0x7fffee032f3f in red_peer_handle_incoming /home/elmarco/pkg/spice/spice-0.12.4/server/red_channel.c:304
#4 0x7fffee033216 in red_channel_client_receive /home/elmarco/pkg/spice/spice-0.12.4/server/red_channel.c:322
#5 0x7fffee03bf1f in red_channel_client_event /home/elmarco/pkg/spice/spice-0.12.4/server/red_channel.c:1561
#6 0x555555c3c53d in qemu_iohandler_poll /home/elmarco/src/qemu/iohandler.c:143
#7 0x555555c3b800 in main_loop_wait /home/elmarco/src/qemu/main-loop.c:504
#8 0x5555556f160c in main_loop /home/elmarco/src/qemu/vl.c:1818
#9 0x5555556f160c in main /home/elmarco/src/qemu/vl.c:4394
#10 0x7fffed80eb14 in __libc_start_main /usr/src/debug/glibc-2.17-c758a686/csu/libc-start.c:274
#11 0x5555556f9c20 in _start (/home/elmarco/src/qemu/x86_64-softmmu/qemu-system-x86_64+0x1a5c20)
0x60040009c098 is located 0 bytes to the right of 8-byte region [0x60040009c090,0x60040009c098)
allocated by thread T0 here:
#0 0x7ffff4e612be in __interceptor_realloc /usr/src/debug/gcc-4.8.5-20150702/obj-x86_64-redhat-linux/x86_64-redhat-linux/libsanitizer/asan/../../../../libsanitizer/asan/asan_malloc_linux.cc:92
#1 0x7fffee121308 in spice_realloc /home/elmarco/pkg/spice/spice-0.12.4/spice-common/common/mem.c:123
#2 0x7fffee004a48 in __spice_char_device_write_buffer_get /home/elmarco/pkg/spice/spice-0.12.4/server/char_device.c:516
#3 0x7fffee004e87 in spice_char_device_write_buffer_get /home/elmarco/pkg/spice/spice-0.12.4/server/char_device.c:557
#4 0x7fffee0ed8b9 in smartcard_char_device_notify_reader_add /home/elmarco/pkg/spice/spice-0.12.4/server/smartcard.c:325
#5 0x7fffee0ef783 in smartcard_add_reader /home/elmarco/pkg/spice/spice-0.12.4/server/smartcard.c:642
#6 0x7fffee0f0568 in smartcard_channel_handle_message /home/elmarco/pkg/spice/spice-0.12.4/server/smartcard.c:757
#7 0x7fffee032f3f in red_peer_handle_incoming /home/elmarco/pkg/spice/spice-0.12.4/server/red_channel.c:304
#8 0x7fffee033216 in red_channel_client_receive /home/elmarco/pkg/spice/spice-0.12.4/server/red_channel.c:322
#9 0x7fffee03bf1f in red_channel_client_event /home/elmarco/pkg/spice/spice-0.12.4/server/red_channel.c:1561
#10 0x555555c3c53d in qemu_iohandler_poll /home/elmarco/src/qemu/iohandler.c:143
SUMMARY: AddressSanitizer: heap-buffer-overflow /home/elmarco/pkg/spice/spice-0.12.4/server/smartcard.c:334 smartcard_char_device_notify_reader_add
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
There is an unref when the message is sent.
This is related to CVE-2016-0749
==17204== ERROR: AddressSanitizer: heap-use-after-free on address 0x6008000144a8 at pc 0x7fffee0ce245 bp 0x7fffffffc630 sp 0x7fffffffc620
READ of size 4 at 0x6008000144a8 thread T0
#0 0x7fffee0ce244 in smartcard_unref_vsc_msg_item /home/elmarco/src/spice/spice/server/smartcard.c:608
#1 0x7fffee0cb451 in smartcard_unref_msg_to_client /home/elmarco/src/spice/spice/server/smartcard.c:178
#2 0x7fffedfcdf14 in spice_char_device_read_from_device /home/elmarco/src/spice/spice/server/char-device.c:330
#3 0x7fffedfd1763 in spice_char_device_wakeup /home/elmarco/src/spice/spice/server/char-device.c:901
#4 0x7fffee05da98 in spice_server_char_device_wakeup /home/elmarco/src/spice/spice/server/reds.c:2990
#5 0x55555593fa34 in spice_chr_write /home/elmarco/src/qemu/spice-qemu-char.c:189
#6 0x5555559375f1 in qemu_chr_fe_write /home/elmarco/src/qemu/qemu-char.c:220
#7 0x555555b3b682 in ccid_card_vscard_send_msg.isra.2 /home/elmarco/src/qemu/hw/usb/ccid-card-passthru.c:76
#8 0x555555b3c466 in ccid_card_vscard_send_error /home/elmarco/src/qemu/hw/usb/ccid-card-passthru.c:91
#9 0x555555b3c466 in ccid_card_vscard_handle_message /home/elmarco/src/qemu/hw/usb/ccid-card-passthru.c:242
#10 0x555555b3c466 in ccid_card_vscard_read /home/elmarco/src/qemu/hw/usb/ccid-card-passthru.c:289
#11 0x55555593f169 in vmc_write /home/elmarco/src/qemu/spice-qemu-char.c:41
#12 0x7fffedfcee6d in spice_char_device_write_to_device /home/elmarco/src/spice/spice/server/char-device.c:477
#13 0x7fffedfcfd31 in spice_char_device_write_buffer_add /home/elmarco/src/spice/spice/server/char-device.c:629
#14 0x7fffee0ce9df in smartcard_channel_write_to_reader /home/elmarco/src/spice/spice/server/smartcard.c:675
#15 0x7fffee0cc7db in smartcard_char_device_notify_reader_add /home/elmarco/src/spice/spice/server/smartcard.c:341
#16 0x7fffee0ce4f3 in smartcard_add_reader /home/elmarco/src/spice/spice/server/smartcard.c:648
#17 0x7fffee0cf2e2 in smartcard_channel_handle_message /home/elmarco/src/spice/spice/server/smartcard.c:763
#18 0x7fffedffe21f in red_peer_handle_incoming /home/elmarco/src/spice/spice/server/red-channel.c:307
#19 0x7fffedffe4f6 in red_channel_client_receive /home/elmarco/src/spice/spice/server/red-channel.c:325
#20 0x7fffee00726c in red_channel_client_event /home/elmarco/src/spice/spice/server/red-channel.c:1566
#21 0x555555c3c53d in qemu_iohandler_poll /home/elmarco/src/qemu/iohandler.c:143
#22 0x555555c3b800 in main_loop_wait /home/elmarco/src/qemu/main-loop.c:504
#23 0x5555556f160c in main_loop /home/elmarco/src/qemu/vl.c:1818
#24 0x5555556f160c in main /home/elmarco/src/qemu/vl.c:4394
#25 0x7fffed7d0b14 in __libc_start_main /usr/src/debug/glibc-2.17-c758a686/csu/libc-start.c:274
#26 0x5555556f9c20 in _start (/home/elmarco/src/qemu/x86_64-softmmu/qemu-system-x86_64+0x1a5c20)
0x6008000144a8 is located 24 bytes inside of 40-byte region [0x600800014490,0x6008000144b8)
freed by thread T0 here:
#0 0x7ffff4e61009 in __interceptor_free /usr/src/debug/gcc-4.8.5-20150702/obj-x86_64-redhat-linux/x86_64-redhat-linux/libsanitizer/asan/../../../../libsanitizer/asan/asan_malloc_linux.cc:61
#1 0x7fffee0ce2a1 in smartcard_unref_vsc_msg_item /home/elmarco/src/spice/spice/server/smartcard.c:610
#2 0x7fffee0cdd58 in smartcard_channel_release_pipe_item /home/elmarco/src/spice/spice/server/smartcard.c:548
#3 0x7fffee000668 in red_channel_client_release_item /home/elmarco/src/spice/spice/server/red-channel.c:602
#4 0x7fffee0006ef in red_channel_client_release_sent_item /home/elmarco/src/spice/spice/server/red-channel.c:609
#5 0x7fffee0007b5 in red_channel_peer_on_out_msg_done /home/elmarco/src/spice/spice/server/red-channel.c:620
#6 0x7fffedffed7e in red_peer_handle_outgoing /home/elmarco/src/spice/spice/server/red-channel.c:385
#7 0x7fffee0057bb in red_channel_client_send /home/elmarco/src/spice/spice/server/red-channel.c:1294
#8 0x7fffee0076e6 in red_channel_client_begin_send_message /home/elmarco/src/spice/spice/server/red-channel.c:1605
#9 0x7fffee0cdccd in smartcard_channel_send_item /home/elmarco/src/spice/spice/server/smartcard.c:541
#10 0x7fffee000570 in red_channel_client_send_item /home/elmarco/src/spice/spice/server/red-channel.c:588
#11 0x7fffee005bfb in red_channel_client_push /home/elmarco/src/spice/spice/server/red-channel.c:1347
#12 0x7fffee007ef7 in red_channel_client_pipe_add_push /home/elmarco/src/spice/spice/server/red-channel.c:1673
#13 0x7fffee0cde4d in smartcard_channel_client_pipe_add_push /home/elmarco/src/spice/spice/server/smartcard.c:571
#14 0x7fffee0cb567 in smartcard_send_msg_to_client /home/elmarco/src/spice/spice/server/smartcard.c:187
#15 0x7fffedfcdba2 in spice_char_device_send_msg_to_clients /home/elmarco/src/spice/spice/server/char-device.c:282
#16 0x7fffedfcdea4 in spice_char_device_read_from_device /home/elmarco/src/spice/spice/server/char-device.c:329
#17 0x7fffedfd1763 in spice_char_device_wakeup /home/elmarco/src/spice/spice/server/char-device.c:901
#18 0x7fffee05da98 in spice_server_char_device_wakeup /home/elmarco/src/spice/spice/server/reds.c:2990
#19 0x55555593fa34 in spice_chr_write /home/elmarco/src/qemu/spice-qemu-char.c:189
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
Primary surface, as additional surfaces, can be used to access
host memory from the guest using invalid parameters.
The removed warning is not enough to prevent all cases. Also a warning
is not enough to stop an escalation to happen.
The red_validate_surface do different checks to make sure surface
request is valid and not cause possible buffer/integer overflows:
- format is valid;
- width is not large to cause overflow compared to stride;
- stride is not -2^31 (a number which negate is still <0);
- stride * height does not overflow.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1312980.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Make possible to reuse it outside red-parse-qxl.c.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
These function are called on both sides of dispatcher so the
increment/decrement of the counter is done in multiple threads.
This caused the counter to not get incremented correctly and
freed the structure too early, leaving a dangling pointer in
the other thread.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1253375.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
This fixes a crash if red_channel_client disconnect is called
handling a message.
This can happen for instance while handling SPICE_MSGC_ACK which calls
red_channel_client_push which tries to detect write errors while writing
to a socket (for instance socket disconnection).
Messages are read in a loop and red_channel_client_disconnect would
cause rcc->stream to be NULL which will result in a use-after-free
problem (stream in red_peer_handle_incoming will use cached stream value).
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
This public API is no longer needed as the keepalive interval does not
need to be configurable. This API was never in a stable 0.12 release, so
it's OK to remove it now.
When redirecting a USB webcam over a slow link, it's currently possible
to hit an assertion in spice-server by running cheese (application using
the webcam), killing the client with ctrl+c and then restarting the
client:
qemu-kvm: spicevmc.c:324: spicevmc_red_channel_alloc_msg_rcv_buf:
Assertion `!state->recv_from_client_buf' failed.
This happens when red_peer_handle_incoming tries to allocate memory for
a message using spicevmc:
handler->msg = handler->cb->alloc_msg_buf(handler->opaque, msg_type,
msg_size);
red_peer_handle_incoming() is called when there is client data to be
read, and does
- call alloc_msg_buf() to allocate memory for the message
- read the message
- if the read was partial, return early, the main loop will call again
red_peer_handle_incoming() when there is more data available for that
channel
- parse the message
- call release_msg_buf() to free the message
For channels based on spicevmc (usbredir and port), alloc_msg_buf()
stores message data in SpiceVmcState::recv_from_client_buf and before
allocating new memory, it asserts that it's NULL.
This is what causes this crash in the following scenario:
- SpiceVmc::alloc_msg_buf() is called and allocates memory for a new
message in SpiceVmcState::recv_from_client_buf
- red_peer_handle_incoming() returns early as all the spicevmc message
data hasn't been received yet
- the client gets killed
- the main channel notices the disconnect and calls
main_dispatcher_client_disconnect() which will disconnect all the
channels
- SpiceVmc::on_disconnect is called
- after the new client connects, SpiceVmc::alloc_msg_buf() is called,
notices that SpiceVmcState::recv_from_client_buf is already set, and
asserts()
This commit makes sure the partial SpiceVmcState::recv_from_client_buf
data is cleared on disconnect so that the assert does not trigger.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1264113
In some case the member specified to SPICE_CONTAINEROF was not
exactly the same type of the pointer passed.
This can cause issues if structure changes so use proper member.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
This avoids compilation errors with -Werror on 32 bit systems as the
pointer size differs from that of a QXLPHYSICAL.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
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>
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>
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>
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.
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
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
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
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
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>
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
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>
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>
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
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>
