Messages in this thread | | | Subject | Re: [PATCH v2] usb:xhci fix panic in xhci_free_virt_devices_depth_first | From | Mathias Nyman <> | Date | Mon, 6 Nov 2017 16:00:10 +0200 |
| |
On 06.11.2017 14:36, Chen Yu wrote: > > > On 2017/11/6 19:32, Greg KH wrote: >>> A simple process is as below: >>> xhci_plat_probe() >>> | >>> usb_add_hcd() xhci_plat_remove() >>> | | >>> find some device usb_remove_hcd() >>> | | >>> hub_port_connect() -> usb_alloc_dev() usb_disconnect() >>> | | >>> before hub_enable_device() xhci_stop() >>> | >>> xhci_mem_cleanup() >>> | >>> xhci_free_virt_devices_depth_first() >>> | >>> real_port is 0 access xhci->rh_bw[vdev->real_port-1] >>> >>> The problem came from https://bugs.96boards.org/show_bug.cgi?id=535 >>> Also look at crbug.com/700041 >> >> Then the bug needs to be fixed, throwing a huge kernel trace message >> into the kernel log is not "fixing" the problem at all, right? >> >> thanks, >> >> greg k-h >> >> . >> > > You are right, the way that xhci_plat_remove() to be called needs to be fixed. > But there is still possibility for this crash. > What do you think if just add an "xhci_warn" instead of "WARN_ON"? > + if (!vdev->real_port) { > + xhci_warn(xhci, "Bad vdev->real_port\n"); > + goto out; > + } > + >
This patch solves the issue, just drop all the error messages.
vdev->real_port is not set until the the device enable/address stage, and we know it won't have any children yet then, so no need to worry about a child having tt pointers to this device.
The "goto out" to xhci_free_virt_device() you do is fine here.
xhci_plat_remove() is the .remove callback for the xhci platform driver. It might get called before a device is properly enabled/addressed. Not really a error. A unlikely but possible situation.
xhci_free_tt_info() already has a similar check
Thanks -Mathias
| |