linux-kernel - Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 30 Nov 2016 16:09:11 +0200
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     Baolin Wang <baolin.wang@...aro.org>, mathias.nyman@...el.com,
        gregkh@...uxfoundation.org
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        broonie@...nel.org
Subject: Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command
 timer to watch stop endpoint command

On 30.11.2016 11:02, Baolin Wang wrote:
> If the hardware never responds to the stop endpoint command, the
> URBs will never be completed, and we might hang the USB subsystem.
> The original watchdog timer is used to watch if one stop endpoint
> command is timeout, if timeout, then the watchdog timer will set
> XHCI_STATE_DYING, try to halt the xHCI host, and give back all
> pending URBs.
>
> But now we already have one command timer to control command timeout,
> thus we can also use the command timer to watch the stop endpoint
> command, instead of one duplicate watchdog timer which need to be
> removed.
>
> Meanwhile we don't need the 'stop_cmds_pending' flag to identy if
> this is the last stop endpoint command of one endpoint. Since we
> can make sure we only set one stop endpoint command for one endpoint
> by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove
> this flag.
>
> We also need to clean up the command queue before trying to halt the
> xHCI host in xhci_stop_endpoint_command_timeout() function.

This isn't a bad idea.

There are anyway some corner cases and details that need to be
checked, such as suspend (which will clear the command queue), module unload
and abrupt host removal (like pci hotplug removal of host controller)
we need to make sure we can trust the command timer to always return the canceled URB

-Mathias