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>
