Commit Graph

1995 Commits

Author SHA1 Message Date
Frediano Ziglio
69628ea137 improve primary surface parameter checks
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>
2016-06-08 09:59:54 +01:00
Frediano Ziglio
790d8f3e53 factor out red_validate_surface function to validate surface parameters
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>
2016-06-08 09:59:31 +01:00
Frediano Ziglio
a051a4d884 Move dcc glz field initialization into dcc_new
There is no reason to initialize these fields in two different places.
Note that an usage between dcc_new and the function where these
field were initialised would have resulted in a crash.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-07 23:09:45 +01:00
Frediano Ziglio
ddd3b286ad record: Make red_record_cursor_cmd static
This function is never used outside this compile unit.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-07 22:43:48 +01:00
Frediano Ziglio
2b58c60bb1 replay: Handle cursor commands
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-07 22:43:30 +01:00
Victor Toso
9df00ebdc8 char-device: fix mismatch of client tokens
As the tokens counter were not being reset you could enter in a
situation where client thinks it has more tokens then server which
would eventually lead to client's disconnection from 0c5eca97f1
onwards (before it was crashing).

It is easy to check the above situation if you track the amount of
tokens you have in the client and simply kill and restart the agent
while doing some file transfer: the client could reach more then 13
tokens which should not really be possible.

Based on patch from Frediano Ziglio <fziglio@redhat.com>

Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-06-07 18:06:31 +01:00
Frediano Ziglio
b46fcbb3ab replay: Load cursor commands
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-06 09:29:12 +01:00
Frediano Ziglio
52f9850be8 worker: Record cursor commands
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-06 09:29:12 +01:00
Frediano Ziglio
3f4ba5e5f5 record: Support cursor commands
Use red_record_cursor_cmd to be able to record cursor commands.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-06 09:28:39 +01:00
Frediano Ziglio
18c99e008f avoid integer underflow under 32 bit architectures
The segment_size computation on 32 bit can lead to big numbers which
can lead to negative offset. Let'ss avoid to underrun the buffer,
as it's not checked.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
2016-06-06 08:58:52 +01:00
Frediano Ziglio
3dd93a1f5f fix integer overflows in red_get_path
Use 64 bit arithmetic to avoid overflows.
The multiplication between count and a constant can overflow.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Fabiano Fidêncio <fidencio@redhat.com>
2016-06-05 09:44:42 +01:00
Frediano Ziglio
4c171dbd0c Remove unused parameter
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
2016-06-04 10:10:08 +01:00
Frediano Ziglio
e8d7fecbc2 Remove only written field
EncoderData::dcc field is never read back.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-03 16:31:20 +01:00
Francois Gouget
ff1a381f60 red-parse-qxl: Check consistency of QXL_DRAW_COPY operations
The source area should not extend outside the source bitmap, or have
swapped coordinates.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2016-06-03 11:37:48 +01:00
Jonathon Jongsma
3037b078f5 Remove global "allowed" variable
This variable was always the same value as
dispatcher_allows_client_mouse.

Acked-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-02 16:06:41 -05:00
Frediano Ziglio
852fc16086 Remove only written lock_count variable
Possibly used for debugging or an initial recursive lock.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-02 17:04:30 +01:00
Frediano Ziglio
a1715a01d4 channel: Remove clients_num and use g_list_length
This fixes a regression introduced by
4028fb1c79, where clients_num
was not updated correctly anymore.

There is no reason to cache the number of elements on that list,
as the expected number of clients is quite small.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
2016-06-01 16:56:16 +01:00
Frediano Ziglio
795d274e2b smartcard: Avoid crash on remove_interface call
SpiceCharDeviceInstance is not a GObject, the GObject is char_device->st
as the above line is stating.
The crash never happen as Qemu never calls spice_server_remove_interface
for smartcards.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-01 16:24:27 +01:00
Frediano Ziglio
80f2d7731d Simplify setting char device instance
Setting "sin" property is equivalent to call
red_char_device_reset_dev_instance so there is no need for a if/else
as the code is doing mostly (beside setting agent_attached) the
same thing

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-01 16:24:24 +01:00
Frediano Ziglio
6fbefcc3c3 Remove obsolete comment
AGENT_CONNECT with token count is implemented by agent_connected_tokens
message.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-01 16:21:07 +01:00
Frediano Ziglio
8a314520cc Fix some typos in comments
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-01 16:09:44 +01:00
Jonathon Jongsma
27d18b659f Replace RedClient::channels with GList
Allows us to not expose the client_link in RedChannelClient.

Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-05-31 16:29:04 +01:00
Frediano Ziglio
0c5eca97f1 Handle flow control without crashing for agent
RedCharDevice used for the agent has flow control enabled.
This make possible for red_char_device_write_buffer_get to return NULL.
Handle such situation without crashing avoiding NULL dereference.

This fixes https://bugs.freedesktop.org/show_bug.cgi?id=95416.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2016-05-31 16:26:33 +01:00
Frediano Ziglio
5d2fb6a897 Avoid getting channel from client
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2016-05-27 18:02:29 +01:00
Frediano Ziglio
5841a2abda Move upcast conversion to a safer place
Upcast conversions are by definition unsafe. There are however some
location where such conversion are more safe. In this case send_item
callback is registered specifically for this type of RedChannel making
the conversion more reliable.
The other conversion (CursorChannel -> RedChannel) became safe.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-27 09:41:01 +01:00
Frediano Ziglio
8d25e03371 Add some comments to cursor-channel.h header
Explain usage of the class.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-27 09:32:58 +01:00
Frediano Ziglio
20373582ac Avoid double conversion
Avoid converting from RedChannelClient* to DisplayChannelClient* and
DisplayChannelClient* to RedChannelClient* just to use RedChannelClient*
('display_channel->clients' is a list of RedChannelClients).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2016-05-26 16:43:31 +01:00
Frediano Ziglio
a95251c30b Remove useless double conversion
Avoid converting RedChannelClient* to CursorChannelClient* and then
CursorChannelClient* to RedChannelClient* just to use RedChannelClient*.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2016-05-26 16:37:25 +01:00
Frediano Ziglio
8f7419a711 replay: rename red_record_dev_input_primary_surface_create
Rename red_record_dev_input_primary_surface_create to
red_record_primary_surface_create.
Name is more concise and was not clear what "dev_input" was supposed
to specify.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-26 16:29:06 +01:00
Frediano Ziglio
82caf14025 replay: allows to specify a filter for record output
This allows compression using external programs or any type
of filters.

To use it set SPICE_WORKER_RECORD_FILTER environment to the
filter command you want to use. The command is executed with
g_spawn_async_with_pipes (which uses execve) so is not a shell
command although the command is parsed using g_shell_parse_argv
which split arguments as shell does.

One easy way to use it is to just use a compressor like gzip with

  export SPICE_WORKER_RECORD_FILENAME=/tmp/qemu_record.gz
  export SPICE_WORKER_RECORD_FILTER=gzip
  qemu ...

The filter will receive the recording on standard input and is
supposed to write in output filename (which is the standard output).
You can use additional arguments in SPICE_WORKER_RECORD_FILTER to
specify for instance compression level.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-26 16:28:55 +01:00
Frediano Ziglio
751a0aeb51 replay: better record encapsulation
Remove global/static from red_record_qxl.c.
Defined a structure and use it to hold record state.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-26 16:28:43 +01:00
Frediano Ziglio
1d3cd7d617 Hide CursorChannelClient implementation details
The existence of this class can be hidden to user of CursorChannel class

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-26 11:02:53 +01:00
Frediano Ziglio
6ad393d82b Make cursor_channel_client_new static
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-26 10:23:35 +01:00
Frediano Ziglio
c0a8b69822 Move cursor_connect to CursorChannel
This function was handling mainly CursorChannel state

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-26 10:23:33 +01:00
Frediano Ziglio
24b3a8e73c Make some function static
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-05-25 15:41:58 +01:00
Frediano Ziglio
ca5db16cb8 dc: use FOREACH_CLIENT macro for loop
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
25d4a66519 dc: inline dpi computation
Actually not exactly the same as this fix a bug if base is not the
first element (in this case if dpi_ring_item is NULL dpi is not)

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
0c91530b07 dc: join the two loop
They tested the same condition, no break in inside loop (which would
exit all loops)

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
85d3af793d dc: reuse code inside loop
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
a7b0162015 dc: convert break to continue
Both jump to check worker_ring_item (internal or external loops)

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
5fbac6c8cb dc: use while again
Convert from for(;;) { if () break; ... } to while

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
0010656ea4 dc: if link is NULL jumping outside loop will check variable and exit
The outer loop will exit too.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
9a85e3cf34 dc: if dpi is not NULL even dpi_ring_item is not NULL
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
643bf445c5 dc: dpi is always a value computed from dpi_ring_item
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
dec4b7eca5 dc: move code inside if
these lines are only executed then the condition are satisfied

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
7447a9e66c dc: dcc used is always a value from current link
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
a3ad49b326 dc: link is never null here
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
1d39275c3d dc: the link condition make this case terminate loop
dpi_ring_item is changed but ignored

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
a5a9b38f46 dc: split if
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00
Frediano Ziglio
2d4e1e5e0a dc: convert to infinite loop
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
2016-05-25 14:44:02 +01:00