summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVictor Toso <victortoso@redhat.com>2021-08-25 11:01:24 +0200
committerVictor Toso <victortoso@redhat.com>2021-10-28 08:59:52 +0200
commita01820bc856fc56e0e32d5c2ef4db3f1f8365065 (patch)
tree97e1c771fd97c0ee8ad9750b2a662b80239fb860
parent55fc307d23d657b52433d1c8508467d0589754d5 (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.c2
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;