linux-kernel - Re: [PATCH v2] vhci_hcd: USB port can get stuck in the disabled state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CALdjXpCaLQps7_XM05grMAScm9EC-tMP6bqf-+OSQvVOX4fhig@mail.gmail.com>
Date:   Thu, 22 Jul 2021 16:19:16 +1000
From:   Michael <msbroadf@...il.com>
To:     linux-usb@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] vhci_hcd: USB port can get stuck in the disabled state

You can pass through any bluetooth dongle (or xbox wireless dongle)
through usbip and it will cause this issue.

For example, here is one of my customers
https://www.virtualhere.com/comment/9432#comment-9432 with the issue.

The device is in the  VDEV_ST_USED state when a reset occurs and so
its never re-enabled

Furthermore there is a bug in the line pr_err("vhci_device speed not set\n");
(L479) because resetting a full-speed device is not an error.


 On Thu, 22 Jul 2021 at 11:26, Shuah Khan <skhan@...uxfoundation.org> wrote:
>
> On 7/21/21 5:55 PM, Michael Broadfoot wrote:
> > When a remote usb device is attached to the local Virtual USB
> > Host Controller Root Hub port, the bound device driver may send a
> > port reset command. For example to initialize firmware (eg. btusb does this).
> > This port reset command can be sent at any time, however the VHCI hcd
> > root hub is only expecting reset to occur before the device receives
> > SET_ADDRESS. The USB port should always be enabled after a reset
> > (because the port is virtual and there is no possibility of hardware errors)
> >
> >
> >
> > Signed-off-by: Michael Broadfoot <msbroadf@...il.com>
> > ---
> >   drivers/usb/usbip/vhci_hcd.c | 13 ++++---------
> >   1 file changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> > index 4ba6bcdaa8e9..d3cda1b2c15a 100644
> > --- a/drivers/usb/usbip/vhci_hcd.c
> > +++ b/drivers/usb/usbip/vhci_hcd.c
> > @@ -455,15 +455,10 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> >                       vhci_hcd->port_status[rhport] &= ~(1 << USB_PORT_FEAT_RESET);
> >                       vhci_hcd->re_timeout = 0;
> >
> > -                     if (vhci_hcd->vdev[rhport].ud.status ==
> > -                         VDEV_ST_NOTASSIGNED) {
> > -                             usbip_dbg_vhci_rh(
> > -                                     " enable rhport %d (status %u)\n",
> > -                                     rhport,
> > -                                     vhci_hcd->vdev[rhport].ud.status);
> > -                             vhci_hcd->port_status[rhport] |=
> > -                                     USB_PORT_STAT_ENABLE;
> > -                     }
>
> VDEV_ST_NOTASSIGNED status indicates that the vdev is in use without
> address assigned - in other words port is initializing.
>
> This is part of the attach request handling when user requests to
> attach to a remote device. attach_store() will change the status
> to VDEV_ST_NOTASSIGNED and then initiate port_connect sequence.
>
> We don't want to touch the vdev when it is in other states.
>
> > +                     usbip_dbg_vhci_rh(" enable rhport %d (status %u)\n",
> > +                                       rhport,
> > +                                       vhci_hcd->vdev[rhport].ud.status);
> > +                     vhci_hcd->port_status[rhport] |= USB_PORT_STAT_ENABLE;
> >
> >                       if (hcd->speed < HCD_USB3) {
> >                               switch (vhci_hcd->vdev[rhport].speed) {
> >
>
> How did you find this problem? Does the port get into stuck state
> while attaching to a remote usbip device on the host?
>
> thanks,
> -- Shuah