diff options
author | Victor Toso <victortoso@redhat.com> | 2021-08-25 11:01:24 +0200 |
---|---|---|
committer | Victor Toso <victortoso@redhat.com> | 2021-10-28 08:59:52 +0200 |
commit | a01820bc856fc56e0e32d5c2ef4db3f1f8365065 (patch) | |
tree | 97e1c771fd97c0ee8ad9750b2a662b80239fb860 | |
parent | 55fc307d23d657b52433d1c8508467d0589754d5 (diff) |
usbredirect: let usbredirparser owns the write buffer
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>
-rw-r--r-- | tools/usbredirect.c | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/tools/usbredirect.c b/tools/usbredirect.c index af417b2..98e5a8c 100644 --- a/tools/usbredirect.c +++ b/tools/usbredirect.c @@ -519,7 +519,7 @@ main(int argc, char *argv[]) self, PACKAGE_STRING, self->verbosity, - usbredirhost_fl_write_cb_owns_buffer); + 0); if (!self->usbredirhost) { g_warning("Error starting usbredirhost"); goto err_init; |