From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753206AbdKFN4H (ORCPT ); Mon, 6 Nov 2017 08:56:07 -0500 Received: from mga07.intel.com ([134.134.136.100]:55874 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752591AbdKFN4G (ORCPT ); Mon, 6 Nov 2017 08:56:06 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,352,1505804400"; d="scan'208";a="918084668" Subject: Re: [PATCH v2] usb:xhci fix panic in xhci_free_virt_devices_depth_first To: Chen Yu , Greg KH References: <20171106082023.116787-1-chenyu56@huawei.com> <20171106083152.GB7087@kroah.com> <4a9bb4fa-6e49-7d48-2127-2721bc806255@huawei.com> <20171106113244.GB20217@kroah.com> Cc: wangbinghui@hisilicon.com, mathias.nyman@intel.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, fanning4@hisilicon.com, lirui39@hisilicon.com, yangdi10@hisilicon.com, groeck@google.com, john.stultz@linaro.org From: Mathias Nyman Message-ID: <5A006AEA.7050907@linux.intel.com> Date: Mon, 6 Nov 2017 16:00:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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