Commit Graph

3310 Commits

Author SHA1 Message Date
Frediano Ziglio
3b9d1eadf7 red-channel-client: Fix indentation
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-12-04 12:15:56 +00:00
Frediano Ziglio
ac41e6097a red-channel-client: Reuse "monitor" variable
"&rcc->priv->connectivity_monitor" is cached in "monitor" variable.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-12-04 12:15:56 +00:00
Frediano Ziglio
0fc5af3a06 Do not check for NULL calling red_watch_remove and red_timer_remove
These functions already check for NULL.
They are used mainly for cleanup, so cold path of code so speed
in case of NULL is not important (and usually should not be NULL).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-12-04 12:15:56 +00:00
Frediano Ziglio
250d4480ff red-channel-client: Reduce indentation in red_channel_client_handle_outgoing
When write fails, all cases of the switch are handled.
Make this clear.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2019-12-04 12:15:56 +00:00
Frediano Ziglio
7f501b0df1 ci: Fix a typo in comment
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-11-26 09:22:14 +00:00
Frediano Ziglio
9480df32fe ci: Disable leak error from Valgrind
For some reason under Fedora 31 the g_socket_client_connect call
from test-listen test caused Valgrind to detect a leak.
Note that the like at gtype.c (g_socket_client_connect) specifically
marks a memory block as malloc-like. Not sure if it's an issue
with Valgrind or with GLib, as a workaround disable the leak report.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-11-25 16:58:13 +00:00
Frediano Ziglio
d0614859cf ci: Fix for test-listen leak detection
This suppression was present in former glib.supp version however
the suppression in glib.supp was updated to only catch "reachable"
leaks.
However in test-listen test the allocation is done in a thread
causing the leak to become "definite".

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-11-25 16:58:11 +00:00
Frediano Ziglio
80f16f2f26 ci: Update glib.supp file
Sync with Glib master file.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-11-25 16:58:06 +00:00
Frediano Ziglio
34455d47a9 build: Cleanup old library flags
OpenGL and Slirp requirement were removed much time ago.
OpenGL was removed by c5c176a5c7
(cfr "server: remove OpenGL", Set 2013).
Slirp was removed by ef9a8bf053
(cfr "Remove tunneling support", Oct 2013).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
2019-11-25 13:20:18 +00:00
Rosen Penev
1921d57160 reds: Fix compilation without deprecated OpenSSL 1.0.2 APIs
The threading API for 1.0 was replaced with the THREADID API.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-11-06 08:59:13 +00:00
Rosen Penev
5bc932f7a7 reds: Fix compilation without deprecated OpenSSL 1.1 APIs
Missing headers for BN_ and RSA_ functions.

Initialization is deprecated with 1.1.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-11-06 08:59:13 +00:00
Frediano Ziglio
cfdb255b41 smart-channel-client: Remove spicec client reference from comment
spicec is an obsolete client.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-21 09:14:29 +01:00
Frediano Ziglio
5308ab84b9 event-loop: Change internal core interface
Allow to modify/cancel timers/watches without having to retrieve
the code interface.
This will make sure that you are not using the wrong interface.
Simplify code to deal with timers/watches.
Remove the requirement to have the core interface available
for removing timers/watches.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-10 10:55:52 +01:00
Frediano Ziglio
97d2d1f129 event-loop: Move adapter interface from reds.c
Put more event loop code in event-loop.c.
This is a preparation patch for the next one.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-10 10:55:38 +01:00
Frediano Ziglio
ebe676697f smartcard: Reset vheader value
The buffer could change inside smartcard_read_buf_prepare.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-10 10:51:59 +01:00
Frediano Ziglio
60a61be549 smartcard: Fix statement termination
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-10 10:51:48 +01:00
Frediano Ziglio
d8d5c48ad6 test-smardcard: Improve test coverage
Using coverage utility exercise more code paths:
- message from channel with wrong type;
- remove message from channel with already removed reader;
- init message from channel (ignored);
- data from devices, ADPU;
- error from device;
- messages split in different ways;
- invalid reader_id values.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:25 +01:00
Frediano Ziglio
344ce666cf test-smartcard: Add test for Smartcard device
Create Smardcard device.
Connect to it and test some messages are parsed and processed
as expected.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:25 +01:00
Frediano Ziglio
b4e508880a test-stream-device: Factor out VMC emulation
Allows to reuse code for emulating a character device.
It will be used for Smardcard test.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:25 +01:00
Frediano Ziglio
07d9328f89 smartcard: Fix parsing multiple messages from the device
This patch handles the scenario when a single read to guest device
brings multiple requests to be handled. When this happens, we will
iterate till all requests are handled and no more requests can be read
from guest device.

If the remaining buffer contains a full request we don't need to read
other bytes (note that there could be no bytes left), just parse the
request.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:22 +01:00
Frediano Ziglio
6fa12db104 smartcard: Fix copying remaining request
Use memmove instead of memcpy as the buffer can overlap if the second
request if bigger than the first.
"buf_pos" points to the point of the buffer after we read, if we want
the first part of the next request is "buf_pos - remaining".
Same consideration setting "buf_pos" for the next iteration.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 10:33:17 +01:00
Frediano Ziglio
57d02c2d17 smartcard: Do not crash if reader_id is invalid
Avoid client to trigger crash. The value of smartcard_readers_get
is checked for NULL so returning it it's not an issue.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 08:53:10 +01:00
Frediano Ziglio
18877d1af8 smartcard-channel-client: Use log instead of printf
More coherent. Also it's not good for a library to output on
standard output.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 08:53:06 +01:00
Frediano Ziglio
8d4d4e3a39 smartcard-channel-client: Remove properties code
Not used

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-09 08:45:25 +01:00
Frediano Ziglio
d546696424 red-channel-client: Use SpiceMsgcAckSync structure
red_channel_client_handle_message is called after parsing the
message so it's not necessary to check it again or parse manually.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 17:12:59 +01:00
Frediano Ziglio
2b56398c34 red-channel-client: Remove useless check
Message is checked by generated message parser.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 17:12:59 +01:00
Frediano Ziglio
774d466cf8 red-channel: Make parser function compulsory
As base messages require parsing better channels should always use
the generated parser.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 17:12:59 +01:00
Frediano Ziglio
bae2d1f00f smartcard: Use generated parse for messages
The generated code handle possible endianess mismatch and check
for message format.
The copy back to "write_buf" allows to use that buffer to send
data back to device.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 16:01:56 +01:00
Frediano Ziglio
33d81bc3fa smartcard-channel-client: Remove unused parameter
"name" parameter of smartcard_channel_client_add_reader it's not
used.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-10-08 15:34:14 +01:00
Frediano Ziglio
abbd985c78 spicevmc: Fix g_object_new call for 32 bit machines
"self-tokens" property is 64 bit and must be passed as 64 bit on
32 bit machines to avoid memory corruptions.
This was introduced by 01de3b8922 ("spicevmc: Avoids DoS if
guest device is not able to get data faster enough"), detected by CI.

It caused this error (split into multiple lines):

  (./test-leaks:15879): GLib-GObject-CRITICAL **: 14:03:59.650: \
  g_object_new_is_valid_property: object class 'RedCharDeviceSpiceVmc' has \
  no property named '\xb0/@\xf3\u0001'

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 16:22:46 +01:00
Frediano Ziglio
01de3b8922 spicevmc: Avoids DoS if guest device is not able to get data faster enough
This fix half (one direction) of
https://gitlab.freedesktop.org/spice/spice/issues/29.
Specifically if you have attempt to transfer a file from the client
using WebDAV.
Previously the queue to the device was unbound. If device was not
getting data fast enough the server started queuing data.
To easily test this you can suspend the WebDAV daemon while transferring
a big file from the client.
To simplify the code and reduce the changes server buffers are
used. This as client token implementation is written with agent
in mind. While when we exhaust server tokens RedCharDevice doesn't
close the client when we exhaust client tokens RedCharDevice closes
the client which in this case it's not wanted.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 13:38:01 +01:00
Frediano Ziglio
726b1824e3 red-channel-client: Allows to block receiving data
If the client is keeping sending data while we can't handle them
(for instance because we need to forward to a device but the
device is not fast enough to receive that amount of data) allows
to stop RedChannelClient to read data.
This after a bit will stop the client sending data as its output
buffer will become full.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 13:37:55 +01:00
Frediano Ziglio
1157588cc1 spicevmc: Avoids DoS if client is not able to get data faster enough
This fix half (one direction) of
https://gitlab.freedesktop.org/spice/spice/issues/29.
Specifically if you have attempt to transfer a file to the client
using WebDAV.
Previously the queue to the client was unbound. If client was not
getting data fast enough the server started queuing data.
To easily test this you can use a tool to limit bandwidth and
transfer a big file (I used a "dd if=/dev/zero bs=1M of=test") from
the guest.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 13:12:58 +01:00
Frediano Ziglio
540f70351d spicevmc: Do not use RedCharDevice pipe items handling
As we don't use any token there's no reason to not queue directly instead
of passing through RedCharDevice.
This will make easier to limit the queue which currently is unlimited.

RedCharDevice flow control has some problems:
- it's really designed with VDI in mind. This for SpiceVMC would cause
  code implementation to be confusing having to satisfy the agent token
  handling;
- it's using items as unit not allowing counting bytes;
- it duplicates some queue management between RedChannelClient;
- it's broken (see comments in char-device.h);
- it forces "clients" to behave in some way not altering for instance the
  queued items (which is very useful if items can be collapsed together);
- it replicates the token handling on the device queue too. This could
  seems right but is not that if you have a network card going @ 1 GBit/s
  you are able to upload more than 1 Gbit/s just using multiple
  connections. The kernel will use a single queue for the network interface
  limiting and sharing de facto the various buffers between the multiple
  connections.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-24 13:12:52 +01:00
Victor Toso
c83420feae main-channel-client: style fixup, indentation of return
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-24 09:46:13 +02:00
Frediano Ziglio
4f2d90a784 red-qxl: Make sure we have at least one monitor
It does not make sense to have a graphic card without a monitor.
In spice_qxl_set_max_monitors we prevent to set 0 monitors, do
the same in spice_qxl_set_device_info.

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1691721.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-19 14:54:53 +01:00
Frediano Ziglio
b862bee93f smartcard: Use RedChannelClient as the type for RedCharDevice client
As now is an opaque type for RedCharDevice use the type that
better suits us.
This avoid useless conversions or look ups.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-17 12:17:31 +01:00
Frediano Ziglio
14fe2c3766 char-device: Don't use RedClient API
RedClient was an opaque structure for RedCharDevice.
It started to be used when RedsState started to contain all
the global state.
Make it opaque again using a new RedCharDeviceClientOpaque.
The RedCharDeviceClientOpaque define in the header allows users
of the class to override the type to get a more safe type
than RedClient.
The define at the beginning of C file is to make sure we don't
use the opaque type as a specific one.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-17 12:17:26 +01:00
Frediano Ziglio
6ccf0e4800 red-channel-client: Inline red_channel_client_get_channel in macro
Inline red_channel_client_get_channel in spice_channel_client_error
macro.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-17 09:40:29 +01:00
Frediano Ziglio
609398fb77 reds: Inline reds_mig_switch function
No much reason for not inlining it, it's quite small and do
not reduce readability.
Note that spice_server_migrate_switch is deprecated and not
used by Qemu since commit 67be6726b6459472103ee87ceaf2e8e97c154965
(cfr "spice: raise requirement to 0.12" September 2012).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-09-17 09:40:24 +01:00
Frediano Ziglio
2d425ee6ae reds: Fix indentation of spice_server_char_device_add_interface
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-02 10:52:09 +01:00
Kevin Pouget
9ccf7bee7a stream-channel: Add preferred video codec capability
This patch enables the SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE
capability for the stream-channel.

video_stream_parse_preferred_codecs: new function for parsing the
SPICE protocol message. This code used to in inside
dcc_handle_preferred_video_codec_type.

struct StreamChannelClient::client_preferred_video_codecs: new field.

Signed-off-by: Kevin Pouget <kpouget@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-09-02 09:53:27 +01:00
Uri Lublin
a9d53f3cd1 test-loop: increment a variable outside of spice_assert
spice_assert is a macro and covscan reports that:
  Argument "++twice_remove_called" of spice_assert() has a side effect.

Doesn't matter if there is a side effects or not,
it's a good practice and it makes covscan happy, so
increment the variable one line above.

Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-08-22 21:27:35 +01:00
Uri Lublin
e9c8b25904 valgrind/spice.supp: add missing ...
There may to be another function (cache_alt_names) between
  gnutls_x509_ext_import_subject_alt_names and
  gnutls_x509_crt_import

cache_alt_names is a static function in gnutls/lib/x509/x509.c
used only in gnutls_x509_crt_import and may be inlined by
the compiler.

Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2019-08-22 08:54:00 +01:00
Frediano Ziglio
80bb7eb774 stat-file: Use proper macro for container computation
This is currently more style patch as the "value" field is the
first field of SpiceStatNode structure, so has offset 0. However
to compute the containing structure it better to use the proper
macro to avoid confusion.
If the offset won't be 0 the subtraction would compute the
wrong pointer as the offset is expressed in bytes but the
element size are uint64_t.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2019-08-22 08:53:23 +01:00
Frediano Ziglio
85461f59cc Remove reference to removed ABI
spice_server_migrate_client_state was removed by

  commit 3c6b4e415f
  Author: Marc-André Lureau <marcandre.lureau@gmail.com>
  Date:   Fri Oct 24 17:16:35 2014 +0200

    Remove spice-experimental

    Remove unneded symbols that nobody should be using anyway.
    ABI is modified with this patch, but the library version is not bumped.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-08-15 11:16:42 +01:00
Frediano Ziglio
461288534f glz-encode: Remove obsolete reference segment
The GLZ code is basically LZ code (in spice-common) sharing image
segments between multiple images.
The code for RLE check in LZ (common/lz_compress_tmpl.c) is dealing
with both RLE and dictionary matches being:

    if (!distance) {
        /* zero distance means a run */
        PIXEL x = *ref;
        while ((ip < ip_bound) && (ref < ref_limit)) { // TODO: maybe separate a run from
                                                       //       the same seg or from different
                                                       //       ones in order to spare
                                                       //       ref < ref_limit

in GLZ that part was copied to RLE part removing the need for the
second segment ("ref"). However the comment and setting ref variables
were not removed. Remove the old code to avoid confusions.

This also remove a Covscan warning as ref_limit was set not not used
(reported by Uri Lublin).
The warning was:

CLANG warning: "Value stored to 'ref_limit' is never read"

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2019-08-12 10:48:02 +01:00
Frediano Ziglio
2f6e2a8ef9 Use (u)intptr_t for virtual addresses
On LLP64 platforms (like Windows) a virtual address cannot be
represented by a "unsigned long" type, so use uintptr_t which is
defined as an integral type large like a pointer.
"address_delta" and "addr_delta" are a difference of pointers so use
same type size.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com>
2019-08-12 08:16:31 +01:00
Frediano Ziglio
c289a6eee3 red-replay-qxl: Fix replay on 32 bit systems
On 32 systems pointers are 32 bit while QXLPHYSICAL is always
64 bit.
Using pointer -> intptr_t -> QXLPHYSICAL conversion cause pointers
to have higher 32 bit set to 1 if the address is >= 0x80000000.
This is possible depending on address space.
The QXLPHYSICAL is split in 3 sections:
- slot ID;
- generation;
- virtual address.
Current utility using record file (spice-server-replay) set slot ID
and generation to 0 so if the higher bits become all 1 slot ID and
generation won't be 0 causing the utility to fail.
Use pointer -> uintptr_t -> QXLPHYSICAL conversion to avoid this
issue.
Note that for opposite conversion (QXLPHYSICAL_TO_PTR) the conversion
does not change, type is changed just for consistency.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com>
2019-08-11 10:18:11 +01:00
Frediano Ziglio
3ebc6d4a43 replay: Remove some leak and a FIXME
Threads are joined by spice_server_destroy, just need to release
last resources.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com>
2019-08-11 10:10:35 +01:00