Some systems use 32-bits, other 64-bits.
Some systems use signed integers, other unsigned integers.
Compute maximum constant based on time_t type.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
These conversions allowed extra conversions to for range loops in
addition to replacing macro usage with .size().
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
display_channel_surface_id_unref resets the surface so
display_channel_surface_has_canvas will return false.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Store pointers to surface object in "surfaces" and allows to
have different surfaces with same ID in memory.
The surface was keep "busy" if there was pending drawing around.
Consider the following case:
1- receive drawing command
2- queue command on DCCs
3- destroy surface
4- send draw
Previously at point 4) you would have to use a surface from
"surfaces" which was destroyed, that is we would have to maintain
the pointer (and canvas) to the surface until reference counter
was 0.
However consider this case:
1- receive drawing command
2- queue command on DCCs
3- destroy surface
4- create surface
5- send draw
What would happen in point 4) ?
We could not change the surface as it will be used by point 5).
To avoid this the code attempts to wait the commands to release the
surface. However this can be an issue, you can't force the clients
to receive pending data if network is slow.
So this patch change this allowing to create surfaces while the old
version will still be used.
This is also more clean from the reference pointer prospective,
as the reference is increased for a specific surface.
Note that now instead of checking for canvas to not be NULL a
simple check for surface pointer is enough.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Mostly left on dcc-send.cpp.
Other minor too.
The change in BitmapData seems odd but the id for cached image
was not used so the only information left was the surface.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Instead of computing the value inside the function to then
compute also in the caller return it.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
As we use reference counting is more direct to use direct pointers.
Also this will allow to have a surface in a "released state"
reducing the complexity of code destroying a surface.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Similar to shared_ptr_counted but avoid atomic counter and
polymorphism.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Makes sure a message is sent with the proper message number.
Reduce mistakes associating types with wrong registered number.
Make easier to add new messages in the future.
Avoids having to type the message number every time, it's inferred
from the type.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Use a template to deduct type and avoid casts in every handler.
The reinterpret_cast seems strong but is safe as converting
a function with 2 typed pointers and a void return into a function
with 2 void pointers and a void return.
The unsafety left (not a regressions) is the association between
handler number and message type.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
When spice_server_destroy is called with pending clients (currently
not a big issue, usually the programs using SPICE server are
exiting then), some leaks can happen.
This is due to the fact that some dispatcher messages are queued
to handle some serialization but then they are neved executed as
the entire system is closed.
Close all connections and handle all main dispatcher messages
to remove these leaks.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
display_channel -> display.
In all the rest of the file display is used.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This will allow to use not trivial objects inside the structure.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Since the conversion to a for range loop, there's no point to this
macro.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Since the conversion to a for range loop, there's no point to this
macro.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Using Unix sockets and no-Glibc C libraries (like Musl) getnameinfo
will fail causing SASL code to fail initialization.
Replicate Glibc behavior and report "localhost" as host and an
empty port string.
This fixes https://gitlab.freedesktop.org/spice/spice/-/issues/58.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
This removes:
In function ‘stream_channel_get_supported_codecs’,
inlined from ‘on_connect’ at ../server/stream-channel.cpp:364:60:
../server/stream-channel.cpp:326:31: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
326 | out_codecs[num++] = codec;
| ^
../server/stream-channel.cpp: In member function ‘on_connect’:
/usr/include/spice-1/spice/stream-device.h:209:13: note: destination object ‘codecs’ of size 0
209 | uint8_t codecs[0];
| ^
Reported by by Tomasz Kłoczko in
https://gitlab.freedesktop.org/spice/spice/-/issues/44
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
The build fails with slibtool while GNU libtool hide the issue
by silently ignoring -no-undefined.
ax_pthread.m4 is sourced from:
e68e8f6f62/m4/ax_pthread.m4
Downstream issue: https://bugs.gentoo.org/780027
Signed-off-by: orbea <orbea@riseup.net>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with readability-uppercase-literal-suffix
Avoids readability problems between lower case l and uppercase I.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Usually buffers to raw data are passed using void* pointers
to avoid casts and mark the buffer as raw.
Use them for read_safe and write_safe to avoid useless casts
in caller code.
As a minor convert a parameter to bool as changing the same
lines.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Found with google-readability-casting
https://google.github.io/styleguide/cppguide.html#Casting
Makes the operation clearer.
This commit uses const_cast where needed.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
The leak is detected by Valgrind on Fedora 34as:
==19603== 400 bytes in 1 blocks are possibly lost in loss record 2,296 of 2,441
==19603== at 0x4845464: calloc (vg_replace_malloc.c:1117)
==19603== by 0x40135FB: _dl_allocate_tls (in /usr/lib64/ld-2.33.so)
==19603== by 0x57EB008: pthread_create@@GLIBC_2.2.5 (in /usr/lib64/libpthread-2.33.so)
==19603== by 0x53A1130: UnknownInlinedFun (gthread-posix.c:1323)
==19603== by 0x53A1130: g_thread_new_internal (gthread.c:931)
==19603== by 0x53C4953: g_thread_pool_start_thread.constprop.0 (gthreadpool.c:477)
==19603== by 0x53A2902: g_thread_pool_push (gthreadpool.c:691)
==19603== by 0x519AE11: g_task_run_in_thread_sync (gtask.c:1593)
==19603== by 0x80D8A74: ??? (in /usr/lib64/gio/modules/libgiolibproxy.so)
==19603== by 0x5181966: g_proxy_address_enumerator_next (gproxyaddressenumerator.c:176)
==19603== by 0x519281A: g_socket_client_connect (gsocketclient.c:1098)
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
At red-parse-qxl.cpp#L535
if (qxl_flags & QXL_BITMAP_DIRECT) {
red->u.bitmap.data = red_get_image_data_flat(slots, group_id,
qxl->bitmap.data,
bitmap_size);
Since qxl->bitmap.data may from the guest, an attacker can make the
memslot_get_virt() check in red_get_image_data_flat() fail and
return a nullptr.
Then at red-parse-qxl.cpp#L550
if (qxl_flags & QXL_BITMAP_UNSTABLE) {
red->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_UNSTABLE;
}
qxl_flags is assigned as qxl->bitmap.flags before, which can also be
controlled by the attacker, resulting in a NULL pointer dereference.
This dereference seems to be introduced by commit 5ac88aa7.
Signed-off-by: Qiuhao Li <Qiuhao.Li@outlook.com>
FreeBSD's setsockopt() behaves just like Dawrin, i.e. sets errno to
EINVAL instead of ENOTSUP, so extend the Darwin workaround to work for
FreeBSD as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
On FreeBSD, netinet/in.h needs to be included to use IPPROTO_TCP.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Compiler error for cross builds using mingw-w64:
In file included from /usr/share/mingw-w64/include/winnt.h:150,
from /usr/share/mingw-w64/include/minwindef.h:163,
from /usr/share/mingw-w64/include/windef.h:9,
from /usr/share/mingw-w64/include/windows.h:69,
from /usr/share/mingw-w64/include/winsock2.h:23,
from ../../../server/spice-core.h:29,
from ../../../server/spice.h:24,
from ../../../server/spice-wrapped.h:35,
from ../../../server/red-common.h:35,
from ../../../server/jpeg-encoder.c:22:
/usr/share/mingw-w64/include/basetsd.h:31:22: error: conflicting types for ‘INT32’
typedef signed int INT32,*PINT32;
^~~~~
In file included from /usr/x86_64-w64-mingw32/sys-root/mingw/include/jpeglib.h:31,
from ../../../server/jpeg-encoder.c:20:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/jmorecfg.h:179:14: note: previous declaration of ‘INT32’ was here
typedef long INT32;
^~~~~
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with performance-move-const-arg
Allows better optimization as the compiler does not have to deal with an
rvalue reference. Especially in C++17 where std::move can prevent copy
elision.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with modernize-use-nullptr
NULL in C++ is 0 whereas it is a void pointer in C. Avoids implicit
conversions.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with performance-for-range-copy
Avoids unnecessary copying when the loop does not modify the variable.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with readability-container-size-empty
This has the potential for extra performance as it's not checking for
every single element.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with performance-for-range-copy
Avoids unnecessary copying when the loop does not modify the variable.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with modernize-use-equals-default
default allows extra optimization compared to an empty con/destructor.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Found with modernize-use-override
This can be useful as compilers can generate a compile time error when:
The base class implementation function signature changes.
The user has not created the override with the correct
signature.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Fix this error reported by some older Gnu C++ compilers:
./server/tests/test-display-base.cpp:818:1: sorry, unimplemented: non-trivial designated initializers not supported
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Like char devices, QXL devices need to be explicily started.
For some historical reason, char devices are started when in running
state. See commi bf1d9007b. Reading that commit comments, there was a
plan to provide an API to stop/start devices invidually, but that never
happened. Whether that API would really be useful now, I wonder.
For now, just follow the char devices behaviour and start QXL devices
added when vm_running.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.uuuuucom>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
If the worker is already started, don't assert and just return.
This fixes calling spice_server_vm_start() multiple times.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
- remove entry for g-type-register-static: the "possible" flag has been
re-introduced in glib.supp
- add new entries for several calloc issues
Signed-off-by: Julien Ropé <jrope@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Reduce code duplication.
Also this improve support for big endian machines as
agent_check_message fix also message endianess.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
Big endian machines on server are not officially supported but won't
hurt.
Messages from client are always encoded as little endian (as all
SPICE protocol).
Convert fields from little endian to host endian on some places
where numbers are used and not just binary copied.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
This brings in the following changes:
Frediano Ziglio (18):
snd_codec: Use better types for snd_codec_is_capable
snd_codec: Use better type for function result
snd_codec: Use better type for snd_codec_create mode
backtrace: Do not attempt to compile if spice_backtrace is empty
Avoid usage of GCC extension for __VA_ARGS__ where possible
helper-fuzzer-demarshallers: Provide replacement for ftello for MSVC
quic_tmpl: Remove unused bpc parameter
ssl_verify: Do not check IP if we fail to resolve it
proto: Add support for side mouse buttons
quic: Constify a parameter
quic: Fix typo in comment
agent: Extend agent_check_message to support VDAgentMonitorsConfig extension
quic: Check we have some data to start decoding quic image
quic: Check image size in quic_decode_begin
quic: Check RLE lengths
quic: Avoid possible buffer overflow in find_bucket
test-quic: Add fuzzer capabilities to the test
test-quic: Add test cases for quic fuzzer
Haochen Tong (2):
pixman_utils: fix clang "unused functions" warning
marshal: fix clang "missing field initializer" warning on generated files
Marc-André Lureau (1):
agent: fix vdagent monitor flag filtering
Changes in sound.cpp are required due to improved types.
In particular some security related changes are imported.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
In configure.ac the micro version is incremented if there is a
forth component in the version.
In Meson this was wrongly translated to "if contains git".
This fixes Gitlab issue #46.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Remove some possible warning, like
reds.cpp:216:18: warning: 'remove_client' overrides a member function but is not marked 'override' [clang-diagnostic-inconsistent-missing-override]
virtual void remove_client(RedCharDeviceClientOpaque *opaque);
^
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
Some compiler could generate some warning, like
reds.cpp:2678:5: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
strcpy(buf, pass);
^
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Jakub Janků <jjanku@redhat.com>
Extend mouse button bits support.
Allows to support up to 16 buttons.
Partly based on a patch from Kevin Pouget (RED_MOUSE_BUTTON_STATE_TO_AGENT
macro, updated on new protocol constants).
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Kevin Pouget <kpouget@redhat.com>
The regression was introduced by
commit 22fc6a48e6
red-channel-client: Change GQueue into a std::list
Starts using smart pointers in the queue.
dcc_add_surface_area_image added a new pipe item in front if pos was
NULL. Before the commit red_add_lossless_drawable_dependencies passed
a pointer to the last element in the pipe adding the new item on the
back. The change caused the item to be inserted in front instead of
back. Restore the insert position.
This caused in some condition the pipe to grow uncontrollably (like
using a large notepad window on Windows 7 and moving it around).
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Let the compiler decide.
Apparently it causes CI makecheck to fail (see below).
Seems to me it's a gcc problem -- was working with version 9,
fails with version 10.
Making the function a non-inline one "fixes" this issue.
Alternatively adding to the structure "int dummy" above the "int type"
"fixes" the build too (but I went with the non-inline solution).
==== error ====
CC mjpeg-encoder.lo
In file included from /usr/include/string.h:495,
from ../../server/red-common.h:24,
from ../../server/mjpeg-encoder.c:25:
In function ‘memset’,
inlined from ‘mjpeg_encoder_reset_quality’ at ../../server/mjpeg-encoder.c:385:5,
inlined from ‘mjpeg_encoder_quality_eval_stop.part.0’ at ../../server/mjpeg-encoder.c:999:5:
/usr/include/bits/string_fortified.h:71:10: error: ‘__builtin_memset’ offset [788, 871] from the object at ‘encoder’ is out of the bounds of referenced subobject ‘type’ with type ‘int’ at offset 784 [-Werror=array-bounds]
71 | return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../server/mjpeg-encoder.c: In function ‘mjpeg_encoder_quality_eval_stop.part.0’:
../../server/mjpeg-encoder.c:96:9: note: subobject ‘type’ declared here
96 | int type;
| ^~~~
cc1: all warnings being treated as errors
Acked-by: Frediano Ziglio <fziglio@redhat.com>
