Skip to content

GitLab

Switch branch/tag
  1. 04 Nov, 2021 3 commits
  3. 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
  5. 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
  7. 02 Oct, 2021 1 commit
  9. 29 Sep, 2021 2 commits
  11. 28 Sep, 2021 1 commit
  13. 24 Sep, 2021 1 commit
  15. 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
  17. 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
  19. 22 Aug, 2021 1 commit
  21. 21 Aug, 2021 1 commit
  23. 13 Aug, 2021 5 commits
  25. 12 Aug, 2021 1 commit
  27. 09 Aug, 2021 1 commit
  29. 08 Aug, 2021 1 commit
    • hansmi's avatar
      Avoid use-after-free in serialization · 03c519ff
      hansmi authored 
      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: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      03c519ff
  31. 08 Jul, 2021 6 commits
  33. 20 Jun, 2021 1 commit
    • hansmi's avatar
      Add fuzzer for filters · 70978842
      hansmi authored 
      

Filters are exposed to the peer and parser bugs could be dangerous. None were
found during the implementation of this fuzzer.

A minor change is made to the filter code to reject separators which aren't
exactly one character long (as documented as a requirement).
Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      70978842
  35. 19 Jun, 2021 2 commits
    • hansmi's avatar
      Verify that rule separators are not empty · 68d143f5
      hansmi authored 
      

The `usbredirfilter_rules_to_string` function always dereferences its separator
arguments, thus requiring at least one character.

Parsing a rule string in `usbredirfilter_string_to_rules` can only work
correctly when there's at least one separator character for both rules and
tokens.
Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      68d143f5
    • hansmi's avatar
      Use single stateless function for filter tests · b0ee1209
      hansmi authored 
      

Recent commits have added unittests for the filter parsing code. The test code
contained two similar, but different implementations of table-driven tests. With
the way the code was written it was rather hard to identify which exact case
failed.

With this change every test case runs under its own "testpath", making it
recognizable from the output (e.g. "ok 28 /filter/rules/bad/17"). In addition
the evaluated filter is emitted in escaped form as a debug message.

The previous implementation of the test for acceptable filter rules was
stateful, i.e. changing the token or rule separator would have an effect on
later test cases. In order to have a predictable test situation it's better
to always fully specify a full test case.
Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      b0ee1209
  37. 18 Jun, 2021 3 commits
  39. 09 Jun, 2021 1 commit
  41. 07 Jun, 2021 2 commits