1. 11 Jan, 2023 3 commits
    • Victor Toso's avatar
      usbredirect: Fix leak of libusb_device_handle · 39fca2b6
      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's avatarUri Lublin <uril@redhat.com>
      Signed-off-by: Victor Toso's avatarVictor Toso <victortoso@redhat.com>
      39fca2b6
    • Victor Toso's avatar
      usbredirect: fix leak of GIOChannel · 8a265067
      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's avatarVictor Toso <victortoso@redhat.com>
      8a265067
    • Victor Toso's avatar
      usbredirect: fix leak of GMainLoop · 87c39a8d
      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's avatarVictor Toso <victortoso@redhat.com>
      87c39a8d
  2. 02 Jan, 2023 2 commits
    • Victor Toso's avatar
      usbredirect: use the correct bus-device · c2fc30ec
      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's avatarVictor Toso <victortoso@redhat.com>
      c2fc30ec
    • Victor Toso's avatar
      usbredirect: allow multiple devices by vendor:product · ebfcffbe
      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's avatarVictor Toso <victortoso@redhat.com>
      ebfcffbe
  3. 22 Dec, 2022 1 commit
  4. 19 Dec, 2022 1 commit
  5. 07 Oct, 2022 2 commits
  6. 17 Sep, 2022 1 commit
  7. 02 Aug, 2022 1 commit
  8. 28 Jul, 2022 1 commit
  9. 24 Jun, 2022 2 commits
    • Victor Toso's avatar
      usbredirparser: reset parser's fields on unserialize · b93c4cae
      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's avatarVictor Toso <victortoso@redhat.com>
      b93c4cae
    • Victor Toso's avatar
      usbredirparser: Fix unserialize on pristine check · 6bf41a23
      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...
      6bf41a23
  10. 04 Apr, 2022 1 commit
  11. 17 Jan, 2022 2 commits
  12. 11 Jan, 2022 1 commit
    • Victor Toso's avatar
      usbredirect: fix leak on bad input · dffc41c3
      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's avatarVictor Toso <victortoso@redhat.com>
      dffc41c3
  13. 30 Dec, 2021 1 commit
  14. 04 Nov, 2021 3 commits
  15. 28 Oct, 2021 2 commits
    • Victor Toso's avatar
      usbredirhost: restrain setting buffered_output_size_cb · f298f497
      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's avatarVictor Toso <victortoso@redhat.com>
      f298f497
    • Victor Toso's avatar
      usbredirect: let usbredirparser owns the write buffer · a01820bc
      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...
      a01820bc
  16. 16 Oct, 2021 1 commit
    • Fabrice Fontaine's avatar
      meson.build: make C++ optional · 55fc307d
      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's avatarFabrice Fontaine <fontaine.fabrice@gmail.com>
      55fc307d
  17. 02 Oct, 2021 1 commit
  18. 29 Sep, 2021 2 commits
  19. 28 Sep, 2021 1 commit
  20. 24 Sep, 2021 1 commit
  21. 28 Aug, 2021 2 commits
    • hansmi's avatar
      Avoid memory leak from ill-formatted serialization data · 186c4c79
      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: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      186c4c79
    • hansmi's avatar
      usbredirparser: Use consistent indentation · c3c67aba
      hansmi authored
      
      
      Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      c3c67aba
  22. 23 Aug, 2021 2 commits
    • hansmi's avatar
      Update write buffer count during deserialization · 060d7914
      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: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      060d7914
    • hansmi's avatar
      Skip empty write buffers when unserializing parser · e37d86c2
      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: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      e37d86c2
  23. 22 Aug, 2021 1 commit
  24. 21 Aug, 2021 1 commit
  25. 13 Aug, 2021 4 commits