linux-kernel - Re: [PATCH v2] usb:xhci fix panic in xhci_free_virt_devices_depth

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:   Mon, 6 Nov 2017 20:36:32 +0800
From:   Chen Yu <chenyu56@...wei.com>
To:     Greg KH <gregkh@...uxfoundation.org>
CC:     <wangbinghui@...ilicon.com>, <mathias.nyman@...el.com>,
        <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <fanning4@...ilicon.com>, <lirui39@...ilicon.com>,
        <yangdi10@...ilicon.com>, <groeck@...gle.com>,
        <john.stultz@...aro.org>
Subject: Re: [PATCH v2] usb:xhci fix panic in
 xhci_free_virt_devices_depth_first



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;
+       }
+

Best regards
Yu Chen