Age | Commit message (Collapse) | Author | Files | Lines |
|
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Avoids:
WARNING: You should add the boolean check kwarg to the run_command call.
It currently defaults to false,
but it will default to true in future releases of meson.
See also: https://github.com/mesonbuild/meson/issues/9300
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
|
|
|
|
> FAILED: tests/test-filter.p/filter.c.o
>
> cc -Itests/test-filter.p -Itests -I../tests -Iusbredirparser
> -I../usbredirparser -I. -I.. -I/usr/include/glib-2.0
> -I/usr/lib64/glib-2.0/include -D_FILE_OFFSET_BITS=64 -Wall
> -Winvalid-pch -O2 -g --param=ssp-buffer-size=4 -D_FORTIFY_SOURCE=2
> -fstack-protector -Werror -MD -MQ tests/test-filter.p/filter.c.o -MF
> tests/test-filter.p/filter.c.o.d -o tests/test-filter.p/filter.c.o -c
> ../tests/filter.c
>
> ../tests/filter.c: In function 'add_tests':
>
> ../tests/filter.c:234:5: error: 'for' loop initial declarations are only allowed in C99 mode
> for (int i = 0; i < count; i++) {
> ^
> ../tests/filter.c:234:5: note: use option -std=c99 or -std=gnu99 to compile your code
Fixes: https://gitlab.freedesktop.org/spice/usbredir/-/issues/32
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
> [4/15] Compiling C object tools/usbredirect.p/usbredirect.c.o
> FAILED: tools/usbredirect.p/usbredirect.c.o
>
> cc -Itools/usbredirect.p -Itools -I../tools -Iusbredirhost -I../usbredirhost -I.
> -I.. -Iusbredirparser -I../usbredirparser -I/usr/include/libusb-1.0
> -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
> -I/usr/include/gio-unix-2.0/ -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch
> -O2 -g --param=ssp-buffer-size=4 -D_FORTIFY_SOURCE=2 -fstack-protector -Werror
> -pthread -Wno-deprecated-declarations -MD -MQ
> tools/usbredirect.p/usbredirect.c.o -MF tools/usbredirect.p/usbredirect.c.o.d -o
> tools/usbredirect.p/usbredirect.c.o -c ../tools/usbredirect.c
>
> ../tools/usbredirect.c: In function ‘main’:
> ../tools/usbredirect.c:647:9: error: implicit declaration of function ‘libusb_set_option’ [-Werror=implicit-function-declaration]
> int ret = libusb_set_option(NULL, LIBUSB_OPTION_LOG_LEVEL, LIBUSB_LOG_LEVEL_NONE);
> ^
> ../tools/usbredirect.c:647:43: error: ‘LIBUSB_OPTION_LOG_LEVEL’ undeclared (first use in this function)
> int ret = libusb_set_option(NULL, LIBUSB_OPTION_LOG_LEVEL, LIBUSB_LOG_LEVEL_NONE);
^
Fixes https://gitlab.freedesktop.org/spice/usbredir/-/issues/32
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
==30494== 24 bytes in 1 blocks are definitely lost in loss record 116 of 288
==30494== at 0x484386F: malloc (vg_replace_malloc.c:393)
==30494== by 0x4883B6E: usbi_add_event_source (io.c:2654)
==30494== by 0x48885E7: op_open (linux_usbfs.c:1410)
==30494== by 0x4888193: libusb_open (core.c:1318)
==30494== by 0x40268E: can_claim_usb_device (usbredirect.c:462)
==30494== by 0x40268E: open_usb_device (usbredirect.c:540)
==30494== by 0x40268E: main (usbredirect.c:599)
==30494==
==30494== 96 bytes in 1 blocks are definitely lost in loss record 262 of 288
==30494== at 0x4848464: calloc (vg_replace_malloc.c:1340)
==30494== by 0x4888161: libusb_open (core.c:1310)
==30494== by 0x40268E: can_claim_usb_device (usbredirect.c:462)
==30494== by 0x40268E: open_usb_device (usbredirect.c:540)
==30494== by 0x40268E: main (usbredirect.c:599)
==30494==
==30494== 809 (128 direct, 681 indirect) bytes in 1 blocks are definitely lost in loss record 284 of 288
==30494== at 0x4848464: calloc (vg_replace_malloc.c:1340)
==30494== by 0x4887C78: usbi_alloc_device (core.c:708)
==30494== by 0x4888961: linux_enumerate_device.isra.0 (linux_usbfs.c:1113)
==30494== by 0x4889114: UnknownInlinedFun (linux_udev.c:299)
==30494== by 0x4889114: UnknownInlinedFun (linux_usbfs.c:458)
==30494== by 0x4889114: op_init (linux_usbfs.c:410)
==30494== by 0x4889E78: libusb_init (core.c:2353)
==30494== by 0x402593: main (usbredirect.c:571)
Fixes: ebfcffbee055ef5ddf981b77240223790dcc0140
Reported-by: Uri Lublin <uril@redhat.com>
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Also adds a couple of asserts to be sure that we are not hitting a
situation where we are creating a watch while another still exists.
Should be fine as it isn't in a hot path and this error should be
catch very early when binary runs.
==26785== 126 (120 direct, 6 indirect) bytes in 1 blocks are definitely lost in loss record 1,696 of 1,830
==26785== at 0x484386F: malloc (vg_replace_malloc.c:393)
==26785== by 0x48FB168: g_malloc (gmem.c:130)
==26785== by 0x4946EAB: g_io_channel_unix_new (giounix.c:618)
==26785== by 0x40303B: create_watch (usbredirect.c:433)
==26785== by 0x402B75: main (usbredirect.c:663)
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
==25772== 16 bytes in 1 blocks are definitely lost in loss record 566 of 1,840
==25772== at 0x4848464: calloc (vg_replace_malloc.c:1340)
==25772== by 0x48FB5F0: g_malloc0 (gmem.c:163)
==25772== by 0x48EF295: g_main_loop_new (gmain.c:4331)
==25772== by 0x402B4D: main (usbredirect.c:694)
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
This patch is a continuation from:
"usbredirect: allow multiple devices by vendor:product"
As we were using libusb_open_device_with_vid_pid(), if an user
requested that device on 2-3 was redirected, we would instead get the
vendor and product info of device on 2-3 and use that info to pick a
usb device. This is wrong when multiple devices that shared
vendor:product are plugged as libbusb_open_device_with_vid_pid()
always return the same (first) device.
This commit now stores bus-device info and uses that to pick the usb
device to redirect.
Related: https://gitlab.freedesktop.org/spice/usbredir/-/issues/29
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Currently, if an user tries to redirect two devices with the same
vendor:product info, the second instance of usbredirect will not
succeed, leading to a segmentation fault.
The core of the problem is that usbredirect is using
libusb_open_device_with_vid_pid() which always returns the first
instance of a given vendor:product, leading the second instance of
usbredirect to give an usb device that is in use to usbredirhost.
This patch fixes the situation, making it possible to run usbredirect
with --device $vendor:$product multiple times. We do a early check
that we can claim the usb device, prior to handle it over to
usbredirhost.
Related: https://gitlab.freedesktop.org/spice/usbredir/-/issues/29
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
|
|
It changed to libusb1 in Fedora 37.
https://fedoraproject.org/wiki/Changes/Rename_libusb_packages_and_deprecated_old_api
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Do not always watch for output buffer.
Watching for output buffer if we don't have nothing to write
(which is the usual case) is consuming a lot of CPU.
This fixes https://gitlab.freedesktop.org/spice/usbredir/-/issues/24.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
|
|
|
|
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
|
|
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Previously, usbredirect would ignore the listen address passed via
`--as <addr>:<port>`, and would always listen on the loopback interface.
This corrects that behavior.
Signed-off-by: Daniel Fullmer <danielrf12@gmail.com>
|
|
This is a followup from previous commit and fixes the following leak.
| 104 (24 direct, 80 indirect) bytes in 1 blocks are definitely lost in loss record 15 of 19
| at 0x484A464: calloc (vg_replace_malloc.c:1328)
| by 0x485A238: usbredirparser_queue (usbredirparser.c:1235)
| by 0x485A571: usbredirparser_init (usbredirparser.c:227)
| by 0x40130B: get_usbredirparser (serializer.c:77)
| by 0x401379: simple (serializer.c:95)
| by 0x48FA3DD: ??? (in /usr/lib64/libglib-2.0.so.0.7200.2)
| by 0x48FA144: ??? (in /usr/lib64/libglib-2.0.so.0.7200.2)
| by 0x48FA8E1: g_test_run_suite (in /usr/lib64/libglib-2.0.so.0.7200.2)
| by 0x48FA94C: g_test_run (in /usr/lib64/libglib-2.0.so.0.7200.2)
| by 0x401161: main (serializer.c:112)
|
| LEAK SUMMARY:
| definitely lost: 24 bytes in 1 blocks
| indirectly lost: 80 bytes in 1 blocks
| possibly lost: 0 bytes in 0 blocks
| still reachable: 25,500 bytes in 17 blocks
| suppressed: 0 bytes in 0 blocks
| Reachable blocks (those to which a pointer was found) are not shown.
| To see them, rerun with: --leak-check=full --show-leak-kinds=all
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
As mentioned in the bug below, the user is trying to migrate QEMU and
it is failing on the unserialization of usbredirparser at the target
host. The user does not have USB attached to the VM at all.
I've added a test that shows that serialization is currently broken.
It fails at the 'pristine' check in usbredirparser_unserialize().
This check was added with e37d86c "Skip empty write buffers when
unserializing parser" and restricted further with 186c4c7 "Avoid
memory leak from ill-formatted serialization data"
The issue here is that usbredirparser's initialization sets some
fields and thus it isn't guaranteed to be pristine.
The parser's basic data is:
| write_buf_count ... : 1
| write_buf ........ : 0xbc03e0
| write_buf_total_size: 80
| data ............. : (nil)
| header_read: ...... : 0
| type_header_read .. : 0
| data_read: ........ : 0
The current fix is to to ignore write_buf checks as, again, they are
not guaranteed to be pristine. usbredirparser library should properly
overwrite them when unserializing the data and if there were pending
buffers, they should be freed.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=2096008
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Do not use -Wp as one can't then easily overwrite the macro with:
-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3.
|
|
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
It was deprecated in 0.9.0 at the same time usbredirect binary was
introduced. Wen can finally removed it.
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Found by covscan:
Error: RESOURCE_LEAK (CWE-772): [#def1] [important]
usbredir-0.12.0/tools/usbredirect.c:55: alloc_fn: Storage is returned from allocation function "g_strsplit".
usbredir-0.12.0/tools/usbredirect.c:55: var_assign: Assigning: "usbid" = storage returned from "g_strsplit(device, "-", 2)".
usbredir-0.12.0/tools/usbredirect.c:76: leaked_storage: Variable "usbid" going out of scope leaks the storage it points to.
# 74| if (i == n) {
# 75| libusb_free_device_list(list, true);
# 76|-> return false;
# 77| }
# 78|
and
Error: RESOURCE_LEAK (CWE-772): [#def2] [important]
usbredir-0.12.0/tools/usbredirect.c:55: alloc_fn: Storage is returned from allocation function "g_strsplit".
usbredir-0.12.0/tools/usbredirect.c:55: var_assign: Assigning: "usbid" = storage returned from "g_strsplit(device, "-", 2)".
usbredir-0.12.0/tools/usbredirect.c:85: leaked_storage: Variable "usbid" going out of scope leaks the storage it points to.
# 83|
# 84| libusb_free_device_list(list, true);
# 85|-> return true;
# 86| }
# 87|
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Avoid unwanted packets.
The test for header length is moved outside the if.
If the header is not complete the number will contain 0 bytes so
a smaller number.
This avoids potential excessive allocations if the header length is
very high.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
|
|
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
This is similar to the application side callback introduced by
a88e197 "usbredirhost: new callback to drop isoc packets", only that
packages are now dropped when usbredirparser owns and queues the
outgoing data.
Resolves: https://gitlab.freedesktop.org/spice/usbredir/-/issues/19
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
The application does not have a way to know how much data is being
queued by usbredirparser due data output being slower than input. This
function is a simple way to get that value.
Also, this function can be used in usbredirhost to allow dropping
isoc packets before calling usbredirparser's functions.
Related: https://gitlab.freedesktop.org/spice/usbredir/-/issues/19
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Application should not be able to set this callback if data queue is
on the library's side.
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
The flag usbredirhost_fl_write_cb_owns_buffer is used to set
usbredirparser's usbredirparser_fl_write_cb_owns_buffer. When this
flag is used, then usbredirect owns the data from the write callback
(usbredir_write_cb) and it should free() that data after using it.
At the moment, we are leaking this data:
> 72,440 bytes in 27 blocks are possibly lost in loss record 1,870 of 1,872
> at 0x484086F: malloc (vg_replace_malloc.c:380)
> by 0x4DCA07E: usbredirparser_queue (usbredirparser.c:1176)
> by 0x4853FF7: usbredirhost_send_stream_data (usbredirhost.c:1092)
> by 0x48573DF: usbredirhost_iso_packet_complete (usbredirhost.c:1544)
> by 0x487DE74: usbi_handle_transfer_completion (io.c:1692)
> by 0x487E2AE: UnknownInlinedFun (linux_usbfs.c:2658)
> by 0x487E2AE: reap_for_handle (linux_usbfs.c:2693)
> by 0x487EC6D: op_handle_events (linux_usbfs.c:2757)
> by 0x487FCE1: handle_events (io.c:2254)
> by 0x4880E36: libusb_handle_events_timeout_completed (io.c:2341)
> by 0x48813F2: libusb_handle_events (io.c:2416)
> by 0x402DC1: thread_handle_libusb_events (usbredirect.c:209)
> by 0x4B41C41: g_thread_proxy (gthread.c:826)
One good use case for usbredirhost_fl_write_cb_owns_buffer is if we
are interested in queueing the data to be sent over the wire in the
application level (like is done in spice-gtk) but this is not needed
in usbredirect so we can remove the flag.
This patch also fixes the scenario where usbredirect crashes after
suspending the vm, due the fact that we are always writting the data
to OS's network stack and when that's full, usbredirparser would
abort() because we are not returning the full number of bytes
requested to be written in the write callback:
> Thread 4 "usbredirect" received signal SIGABRT, Aborted.
> [Switching to Thread 0x7ffff67c4640 (LWP 26138)]
> __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
> 49 return ret;
> (gdb) bt
> #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
> #1 0x00007ffff7a758a4 in __GI_abort () at abort.c:79
> #2 0x00007ffff7a441d5 in usbredirparser_do_write (parser_pub=<optimized out>) at ../usbredirparser/usbredirparser.c:1122
> #3 0x00007ffff7fbda18 in usbredirhost_write_guest_data (host=<optimized out>) at ../usbredirhost/usbredirhost.c:903
> #4 0x0000000000402e87 in usbredir_write_flush_cb (user_data=0x4140a0) at ../tools/usbredirect.c:343
> #5 0x00007ffff7f94e75 in usbi_handle_transfer_completion (itransfer=itransfer@entry=0x458b40,
> status=status@entry=LIBUSB_TRANSFER_COMPLETED) at /usr/src/debug/libusbx-1.0.24-2.fc34.x86_64/libusb/io.c:1692
Instead of crashing, usbredirparser would queue the data till the network
resumes again or the application react to the situation.
Related: https://gitlab.freedesktop.org/spice/usbredir/-/issues/19
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Remove cpp from meson project statement to make C++ optional and avoid
the following build failure when the toolchain does not provide a C++
compiler:
../output-1/build/usbredir-0.11.0/meson.build:1:0: ERROR: Unknown compiler(s): [['/home/buildroot/autobuild/instance-3/output-1/host/bin/arm-linux-g++']]
The following exception(s) were encountered:
Running "/home/buildroot/autobuild/instance-3/output-1/host/bin/arm-linux-g++ --version" gave "[Errno 2] No such file or directory: '/home/buildroot/autobuild/instance-3/output-1/host/bin/arm-linux-g++'"
Indeed C++ is only required for fuzzing which is already handled by
meson through add_languages('cpp', required: true)
Fixes:
- http://autobuild.buildroot.org/results/eca1d8a2b73a769354ab1d24c7996be30f152138
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
|
|
If we fail to unserialize data we need to reset data to avoid
invalid state.
We can accept data only if we had data (data_len > 0), otherwise
reset it.
This also fixes https://gitlab.freedesktop.org/spice/usbredir/-/issues/21.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
|
|
Apply the packet size limit used when reading packets during deserialization.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
The array pointed by data should be big to contain data_len bytes.
Change one field close to the other to make easier to maintain
such invariant in usbredirparser_unserialize.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
|
|
Make sure structure keeps its integrity.
This is useful used with the fuzzer.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
|
|
Add stack_protector option to allow the user to disable it as some
embedded toolchains don't support it which will result in the following
build failure:
/home/giuliobenetti/autobuild/run/instance-3/output-1/host/opt/ext-toolchain/bin/../lib/gcc/sparc-buildroot-linux-uclibc/9.3.0/../../../../sparc-buildroot-linux-uclibc/bin/ld: usbredirparser/libusbredirparser.so.1.1.0.p/usbredirparser.c.o: in function `va_log':
usbredirparser.c:(.text+0x1c4): undefined reference to `__stack_chk_guard'
Fixes:
- http://autobuild.buildroot.org/results/40de5443e98157ad50c6841ea70a835cd5ad4fe9
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
|
|
At commit 060d7914ea the following `fuzzing/usbredirparserfuzz` input triggers
a memory leak:
```
$ base64 -d <<'EOF' | gunzip -c > testcase
H4sIAFDwJ2ECA2NgZIADBQWF/zD2HQgfAv4DgcJ/BTzgP0iRAlwRivL/ePX9
R1VAyBaSjcShB8nPTBA9/xn+g5UAANvH+dkSAQAA
```
The data type header segment is empty while there's (supposed) payload data in
there. The internal buffers must be filled in the order of header, type header
and then data, an invariant important for `usbredirparser_do_read` and violated
by this input.
With this change the input data is read the same way, but if the invariant would
be violated the data read is just ignored.
The parser check at the beginning of `usbredirparser_unserialize` is also
improved and `write_buf_count` is no longer set explicitly.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
At commit 8490a7ac the following `fuzzing/usbredirparserfuzz` input causes
an integer underflow on write_buf_count as it's not updated during
deserialization:
```
$ base64 -d <<'EOF' | gunzip -c > testcase6250181968920576
H4sIAMChImECAzNkwA0YgZgTxPiPBGCSAkgK07b9YgPRLDA+EBvNYoPLuzDg
A3Cj3/xHBTc5IPJgFQz/vwNJznSGBIYQIBtJPxN29n842xbhaohtaXA7GX4z
fHsPFNVmYHiI4S8jBiKAADbfAEMOaEkCMEDArv6fBpX4gzMM/6OoxxHOWMMO
YjYiIFleosvhsxcD/IYofY/dW0AD/xPy1nWc3kpDTUsEnYPD+UQEPzHuxB78
qWDwn0pgP8k6UvGDq0Alu7Foe0+0BYygEDBg+LwtAZhdAUAa2Kf/AwAA
EOF
```
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
At commit 8490a7ac101d the following `fuzzing/usbredirparserfuzz` input causes
the instantiation of empty write buffers:
$ base64 -d <<'EOF' > testcase6474540506021888
QAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAG
AAAAAN7/AAAACAA=
EOF
Empty write buffers trigger all kinds of issues, in part because they cause
calls to write(2) with a zero length.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
Don't try to multiply the number of bytes to read when it's too large:
signed integer overflow: 4 * 538976082 cannot be represented in type 'int'
Given the usual sizes of fuzzing inputs, even a few MiB is too large.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
Add tests option to allow the user to disable them
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
|
|
Similar logic already existed in `parser_write` and before its removal the
log-related code also iterated over input data. With this change utility
functions are introduced to ensure that function inputs are read.
`std::make_unique_for_overwrite` is a C++20 feature not yet available in Clang.
A feature test macro allows for its detection.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
Now that the magic number for the serialization format is a header it can also
be used for fuzzing. As it turned out I had gotten the endianess wrong in commit
58f198e and the unserialization code wasn't actually fuzzed (unless the fuzzer
had somehow found the magic value).
Endianess support is still in the TODO file and so a plain memcpy is sufficient.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
Currently the fuzzer hardcodes the value of the USBREDIRPARSER_SERIALIZE_MAGIC
constant. Move it to usbredirparser.h to allow for its reuse in the fuzzer.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
The log code was commented out and wouldn't be very useful anyway. If fuzzing
finds a problem either the report from the fuzzing environment along with the
reproduction information will be enough or a debugger needs to be involved
anyway.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
While "#pragma once" is not part of the C or C++ standards, it is a construct
supported "by the vast majority of modern compilers"[1]. The previously used
include guards starting with two underscores were also in violation of the
relevant standards: identifiers starting with a double underscore ("__") are
reserved for the implementation[2].
[1] https://en.cppreference.com/w/cpp/preprocessor/impl#.23pragma_once
[2] https://eel.is/c++draft/lex.name#3.1
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
Serializing parsers with large amounts of buffered write data (e.g. in case of
a slow or blocked write destination) would cause "serialize_data" to reallocate
the state buffer whose default size is 64kB (USBREDIRPARSER_SERIALIZE_BUF_SIZE).
The pointer to the position for the write buffer count would then point to
a location outside the buffer where the number of write buffers would be written
as a 32-bit value.
As of QEMU 5.2.0 the serializer is invoked for migrations. Serializations for
migrations may happen regularily such as when using the COLO feature[1].
Serialization happens under QEMU's I/O lock. The guest can't control the state
while the serialization is happening. The value written is the number of
outstanding buffers which would be suceptible to timing and host system system
load. The guest would have to continously groom the write buffers. A useful
value needs to be allocated in the exact position freed during the buffer size
increase, but before the buffer count is written. The author doesn't consider it
realistic to exploit this use-after-free reliably.
[1] https://wiki.qemu.org/Features/COLO
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|
|
Exercise more code paths by unserializing a fuzzed data buffer and serializing
while reading and writing.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
|