From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755526AbdKGB5I (ORCPT ); Mon, 6 Nov 2017 20:57:08 -0500 Received: from szxga06-in.huawei.com ([45.249.212.32]:33585 "EHLO huawei.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1755349AbdKGB5G (ORCPT ); Mon, 6 Nov 2017 20:57:06 -0500 Subject: Re: [PATCH v2] usb:xhci fix panic in xhci_free_virt_devices_depth_first To: Mathias Nyman , Greg KH References: <20171106082023.116787-1-chenyu56@huawei.com> <20171106083152.GB7087@kroah.com> <4a9bb4fa-6e49-7d48-2127-2721bc806255@huawei.com> <20171106113244.GB20217@kroah.com> <5A006AEA.7050907@linux.intel.com> CC: , , , , , , , , From: Chen Yu Message-ID: Date: Tue, 7 Nov 2017 09:55:23 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <5A006AEA.7050907@linux.intel.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.142.63.192] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/11/6 22:00, Mathias Nyman wrote: > 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 a lot for your analysis and suggestion! The check within xhci_free_tt_info() is a better one. I will upload a new patch like the check in xhci_free_tt_info(). > > Thanks > -Mathias > > > . >