USB pass-through problems

USB pass-through problems

[BALATON Zoltan]

Hello,

I've seen a case when QEMU hangs with a passed through USB device. This is 
with -device usb-ehci and pass through with usb-host. This works until the 
attached USB device reboots (so likely it disconnects and reconnects) at 
which point QEMU hangs and need to be SIGKILL-ed to end (that's a bit hard 
to do with mouse and keyboard grabbed). I've got this stack trace:

#0  0x00007f23e7bd4949 in poll () at /lib64/libc.so.6
#1  0x00007f23e8bfa9a5 in  () at /lib64/libusb-1.0.so.0
#2  0x00007f23e8bfbb13 in libusb_handle_events_timeout_completed () at /lib64/libusb-1.0.so.0
#3  0x000055e09854b7da in usb_host_abort_xfers (s=0x55e09b036dd0) at hw/usb/host-libusb.c:963
#4  0x000055e09854b87a in usb_host_close (s=0x55e09b036dd0) at hw/usb/host-libusb.c:977
#5  0x000055e09854b92e in usb_host_nodev_bh (opaque=0x55e09b036dd0) at hw/usb/host-libusb.c:998
#6  0x000055e098757500 in aio_bh_call (bh=0x55e099ad9cc0) at util/async.c:136
#7  0x000055e09875760a in aio_bh_poll (ctx=0x55e0996c2620) at util/async.c:164
#8  0x000055e09875cb2a in aio_dispatch (ctx=0x55e0996c2620) at util/aio-posix.c:380
#9  0x000055e098757a3d in aio_ctx_dispatch (source=0x55e0996c2620, callback=0x0, user_data=0x0) at util/async.c:306
#10 0x00007f23e8c59665 in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#11 0x000055e09875b0a9 in glib_pollfds_poll () at util/main-loop.c:219
#12 0x000055e09875b123 in os_host_main_loop_wait (timeout=0) at util/main-loop.c:242
#13 0x000055e09875b228 in main_loop_wait (nonblocking=0) at util/main-loop.c:518
#14 0x000055e0982c91f8 in qemu_main_loop () at softmmu/vl.c:1664
#15 0x000055e098162e7e in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at softmmu/main.c:49

so the problem may be in libusb but QEMU should not hang completely. The 
host is Linux with libusb 1.0.22.

Another problem I've seen with xhci is here: 
https://bugs.launchpad.net/qemu/+bug/1810000
but since the title does not clearly say it's a usb issue maybe you've 
missed that.

Regards,
BALATON Zoltan

Re: USB pass-through problems

[Gerd Hoffmann]

On Wed, May 27, 2020 at 09:44:54PM +0200, BALATON Zoltan wrote:
> Hello,
> 
> I've seen a case when QEMU hangs with a passed through USB device. This is
> with -device usb-ehci and pass through with usb-host. This works until the
> attached USB device reboots (so likely it disconnects and reconnects) at
> which point QEMU hangs and need to be SIGKILL-ed to end (that's a bit hard
> to do with mouse and keyboard grabbed). I've got this stack trace:
> 
> #0  0x00007f23e7bd4949 in poll () at /lib64/libc.so.6
> #1  0x00007f23e8bfa9a5 in  () at /lib64/libusb-1.0.so.0
> #2  0x00007f23e8bfbb13 in libusb_handle_events_timeout_completed () at /lib64/libusb-1.0.so.0
> #3  0x000055e09854b7da in usb_host_abort_xfers (s=0x55e09b036dd0) at hw/usb/host-libusb.c:963
> #4  0x000055e09854b87a in usb_host_close (s=0x55e09b036dd0) at hw/usb/host-libusb.c:977
> #5  0x000055e09854b92e in usb_host_nodev_bh (opaque=0x55e09b036dd0) at hw/usb/host-libusb.c:998
> #6  0x000055e098757500 in aio_bh_call (bh=0x55e099ad9cc0) at util/async.c:136
> #7  0x000055e09875760a in aio_bh_poll (ctx=0x55e0996c2620) at util/async.c:164
> #8  0x000055e09875cb2a in aio_dispatch (ctx=0x55e0996c2620) at util/aio-posix.c:380
> #9  0x000055e098757a3d in aio_ctx_dispatch (source=0x55e0996c2620, callback=0x0, user_data=0x0) at util/async.c:306
> #10 0x00007f23e8c59665 in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
> #11 0x000055e09875b0a9 in glib_pollfds_poll () at util/main-loop.c:219
> #12 0x000055e09875b123 in os_host_main_loop_wait (timeout=0) at util/main-loop.c:242
> #13 0x000055e09875b228 in main_loop_wait (nonblocking=0) at util/main-loop.c:518
> #14 0x000055e0982c91f8 in qemu_main_loop () at softmmu/vl.c:1664
> #15 0x000055e098162e7e in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at softmmu/main.c:49
> 
> so the problem may be in libusb but QEMU should not hang completely. The
> host is Linux with libusb 1.0.22.

Hmm, does reverting 76d0a9362c6a6a7d88aa18c84c4186c9107ecaef change
behavior?

cheers,
  Gerd

Re: USB pass-through problems

[BALATON Zoltan]

On Thu, 28 May 2020, Gerd Hoffmann wrote:
> On Wed, May 27, 2020 at 09:44:54PM +0200, BALATON Zoltan wrote:
>> Hello,
>>
>> I've seen a case when QEMU hangs with a passed through USB device. This is
>> with -device usb-ehci and pass through with usb-host. This works until the
>> attached USB device reboots (so likely it disconnects and reconnects) at
>> which point QEMU hangs and need to be SIGKILL-ed to end (that's a bit hard
>> to do with mouse and keyboard grabbed). I've got this stack trace:
>>
>> #0  0x00007f23e7bd4949 in poll () at /lib64/libc.so.6
>> #1  0x00007f23e8bfa9a5 in  () at /lib64/libusb-1.0.so.0
>> #2  0x00007f23e8bfbb13 in libusb_handle_events_timeout_completed () at /lib64/libusb-1.0.so.0
>> #3  0x000055e09854b7da in usb_host_abort_xfers (s=0x55e09b036dd0) at hw/usb/host-libusb.c:963
>> #4  0x000055e09854b87a in usb_host_close (s=0x55e09b036dd0) at hw/usb/host-libusb.c:977
>> #5  0x000055e09854b92e in usb_host_nodev_bh (opaque=0x55e09b036dd0) at hw/usb/host-libusb.c:998
>> #6  0x000055e098757500 in aio_bh_call (bh=0x55e099ad9cc0) at util/async.c:136
>> #7  0x000055e09875760a in aio_bh_poll (ctx=0x55e0996c2620) at util/async.c:164
>> #8  0x000055e09875cb2a in aio_dispatch (ctx=0x55e0996c2620) at util/aio-posix.c:380
>> #9  0x000055e098757a3d in aio_ctx_dispatch (source=0x55e0996c2620, callback=0x0, user_data=0x0) at util/async.c:306
>> #10 0x00007f23e8c59665 in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
>> #11 0x000055e09875b0a9 in glib_pollfds_poll () at util/main-loop.c:219
>> #12 0x000055e09875b123 in os_host_main_loop_wait (timeout=0) at util/main-loop.c:242
>> #13 0x000055e09875b228 in main_loop_wait (nonblocking=0) at util/main-loop.c:518
>> #14 0x000055e0982c91f8 in qemu_main_loop () at softmmu/vl.c:1664
>> #15 0x000055e098162e7e in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at softmmu/main.c:49
>>
>> so the problem may be in libusb but QEMU should not hang completely. The
>> host is Linux with libusb 1.0.22.
>
> Hmm, does reverting 76d0a9362c6a6a7d88aa18c84c4186c9107ecaef change
> behavior?

Yes it does. Reverting that patch fixes the problem, no hang and device 
reconnects without problem.

Thanks,
BALATON Zoltan

Re: USB pass-through problems

[Gerd Hoffmann]

> > > #2  0x00007f23e8bfbb13 in libusb_handle_events_timeout_completed () at /lib64/libusb-1.0.so.0
> > > #3  0x000055e09854b7da in usb_host_abort_xfers (s=0x55e09b036dd0) at hw/usb/host-libusb.c:963

> > Hmm, does reverting 76d0a9362c6a6a7d88aa18c84c4186c9107ecaef change
> > behavior?
> 
> Yes it does. Reverting that patch fixes the problem, no hang and device
> reconnects without problem.

Hmm.  Looks like an libusb bug to me, it seems to not call the
completion callback for the canceled transfers (which it should do
according to the docs), so qemu waits for this to happen forever.

We can certainly add a limit here (see below), question is how to
handle the canceled but not completed transfers then.  I suspect
we have to leak them to make sure we don't get use-after-free
access from libusb ...

cheers,
  Gerd

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index e28441379d99..4c3b5b140d9d 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -944,30 +944,45 @@ fail:
         libusb_close(s->dh);
         s->dh = NULL;
         s->dev = NULL;
     }
     return -1;
 }
 
 static void usb_host_abort_xfers(USBHostDevice *s)
 {
     USBHostRequest *r, *rtmp;
+    int limit = 100;
 
     QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) {
         usb_host_req_abort(r);
     }
 
     while (QTAILQ_FIRST(&s->requests) != NULL) {
         struct timeval tv;
         memset(&tv, 0, sizeof(tv));
         tv.tv_usec = 2500;
         libusb_handle_events_timeout(ctx, &tv);
+        if (--limit == 0) {
+            /*
+             * Don't wait forever for libusb calling the complete
+             * callback (which will unlink and free the request).
+             *
+             * Leaking memory here, to make sure libusb will not
+             * access memory which we have released already.
+             */
+            QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) {
+                fprintf(stderr, "%s: leaking usb request %p\n", __func__, r);
+                QTAILQ_REMOVE(&s->requests, r, next);
+            }
+            return;
+        }
     }
 }
 
 static int usb_host_close(USBHostDevice *s)
 {
     USBDevice *udev = USB_DEVICE(s);
 
     if (s->dh == NULL) {
         return -1;
     }

Re: USB pass-through problems

[BALATON Zoltan]

On Thu, 28 May 2020, Gerd Hoffmann wrote:
>>>> #2  0x00007f23e8bfbb13 in libusb_handle_events_timeout_completed () at /lib64/libusb-1.0.so.0
>>>> #3  0x000055e09854b7da in usb_host_abort_xfers (s=0x55e09b036dd0) at hw/usb/host-libusb.c:963
>
>>> Hmm, does reverting 76d0a9362c6a6a7d88aa18c84c4186c9107ecaef change
>>> behavior?
>>
>> Yes it does. Reverting that patch fixes the problem, no hang and device
>> reconnects without problem.
>
> Hmm.  Looks like an libusb bug to me, it seems to not call the
> completion callback for the canceled transfers (which it should do
> according to the docs), so qemu waits for this to happen forever.
>
> We can certainly add a limit here (see below), question is how to
> handle the canceled but not completed transfers then.  I suspect
> we have to leak them to make sure we don't get use-after-free
> access from libusb ...

Also works,

Tested-by: BALATON Zoltan <balaton@eik.bme.hu>

Got only one "usb_host_abort_xfers: leaking usb request" message.

Regards,
BALATON Zoltan

> cheers,
>  Gerd
>
> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index e28441379d99..4c3b5b140d9d 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -944,30 +944,45 @@ fail:
>         libusb_close(s->dh);
>         s->dh = NULL;
>         s->dev = NULL;
>     }
>     return -1;
> }
>
> static void usb_host_abort_xfers(USBHostDevice *s)
> {
>     USBHostRequest *r, *rtmp;
> +    int limit = 100;
>
>     QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) {
>         usb_host_req_abort(r);
>     }
>
>     while (QTAILQ_FIRST(&s->requests) != NULL) {
>         struct timeval tv;
>         memset(&tv, 0, sizeof(tv));
>         tv.tv_usec = 2500;
>         libusb_handle_events_timeout(ctx, &tv);
> +        if (--limit == 0) {
> +            /*
> +             * Don't wait forever for libusb calling the complete
> +             * callback (which will unlink and free the request).
> +             *
> +             * Leaking memory here, to make sure libusb will not
> +             * access memory which we have released already.
> +             */
> +            QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) {
> +                fprintf(stderr, "%s: leaking usb request %p\n", __func__, r);
> +                QTAILQ_REMOVE(&s->requests, r, next);
> +            }
> +            return;
> +        }
>     }
> }
>
> static int usb_host_close(USBHostDevice *s)
> {
>     USBDevice *udev = USB_DEVICE(s);
>
>     if (s->dh == NULL) {
>         return -1;
>     }
>
>