- 11 Jan, 2023 3 commits
-
-
Victor Toso authored
==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: ebfcffbe Reported-by:
Uri Lublin <uril@redhat.com> Signed-off-by:
Victor Toso <victortoso@redhat.com>
-
Victor Toso authored
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>
-
Victor Toso authored
==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>
-
- 02 Jan, 2023 2 commits
-
-
Victor Toso authored
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: #29 Signed-off-by:
Victor Toso <victortoso@redhat.com>
-
Victor Toso authored
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: #29 Signed-off-by:
Victor Toso <victortoso@redhat.com>
-
- 22 Dec, 2022 1 commit
-
-
John Call authored
-
- 19 Dec, 2022 1 commit
-
-
Victor Toso authored
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>
-
- 07 Oct, 2022 2 commits
-
-
Frediano Ziglio authored
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 #24 . Signed-off-by:
Frediano Ziglio <freddy77@gmail.com>
-
Frediano Ziglio authored
-
- 17 Sep, 2022 1 commit
-
-
Frediano Ziglio authored
Signed-off-by:
Frediano Ziglio <freddy77@gmail.com>
-
- 02 Aug, 2022 1 commit
-
-
Victor Toso authored
Signed-off-by:
Victor Toso <victortoso@redhat.com>
-
- 28 Jul, 2022 1 commit
-
-
Daniel Fullmer authored
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>
-
- 24 Jun, 2022 2 commits
-
-
Victor Toso authored
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>
-
Victor Toso authored
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 e37d86c2 "Skip empty write buffers when unserializing parser" and restricted further with 186c4c79 "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 pristi...
-
- 04 Apr, 2022 1 commit
-
-
Martin Liska authored
Do not use -Wp as one can't then easily overwrite the macro with: -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3.
-
- 17 Jan, 2022 2 commits
-
-
Victor Toso authored
Signed-off-by:
Victor Toso <victortoso@redhat.com>
-
Victor Toso authored
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>
-
- 11 Jan, 2022 1 commit
-
-
Victor Toso authored
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>
-
- 30 Dec, 2021 1 commit
-
-
Frediano Ziglio authored
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>
-
- 04 Nov, 2021 3 commits
-
-
Victor Toso authored
Signed-off-by:
Victor Toso <victortoso@redhat.com>
-
Victor Toso authored
This is similar to the application side callback introduced by a88e197b "usbredirhost: new callback to drop isoc packets", only that packages are now dropped when usbredirparser owns and queues the outgoing data. Resolves: #19 Signed-off-by:
Victor Toso <victortoso@redhat.com>
-
Victor Toso authored
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: #19 Signed-off-by:
Victor Toso <victortoso@redhat.com>
-
- 28 Oct, 2021 2 commits
-
-
Victor Toso authored
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>
-
Victor Toso authored
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...
-
- 16 Oct, 2021 1 commit
-
-
Fabrice Fontaine authored
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>
-
- 02 Oct, 2021 1 commit
-
-
Frediano Ziglio authored
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 #21 . Signed-off-by:
Frediano Ziglio <freddy77@gmail.com>
-
- 29 Sep, 2021 2 commits
-
-
hansmi authored
Apply the packet size limit used when reading packets during deserialization. Signed-off-by:
Michael Hanselmann <public@hansmi.ch>
-
Frediano Ziglio authored
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>
-
- 28 Sep, 2021 1 commit
-
-
Frediano Ziglio authored
Make sure structure keeps its integrity. This is useful used with the fuzzer. Signed-off-by:
Frediano Ziglio <freddy77@gmail.com>
-
- 24 Sep, 2021 1 commit
-
-
Fabrice Fontaine authored
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>
-
- 28 Aug, 2021 2 commits
-
-
hansmi authored
At commit 060d7914 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>
-
hansmi authored
Signed-off-by:
Michael Hanselmann <public@hansmi.ch>
-
- 23 Aug, 2021 2 commits
-
-
hansmi authored
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>
-
hansmi authored
At commit 8490a7ac 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>
-
- 22 Aug, 2021 1 commit
-
-
hansmi authored
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>
-
- 21 Aug, 2021 1 commit
-
-
Fabrice Fontaine authored
Add tests option to allow the user to disable them Signed-off-by:
Fabrice Fontaine <fontaine.fabrice@gmail.com>
-
- 13 Aug, 2021 4 commits
-
-
hansmi authored
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>
-
hansmi authored
Signed-off-by:
Michael Hanselmann <public@hansmi.ch>
-
hansmi authored
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 58f198e8 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>
-
hansmi authored
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>
-