Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753223AbdGSHxh (ORCPT ); Wed, 19 Jul 2017 03:53:37 -0400 Received: from mga09.intel.com ([134.134.136.24]:20063 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753139AbdGSHxf (ORCPT ); Wed, 19 Jul 2017 03:53:35 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,380,1496127600"; d="asc'?scan'208";a="1197127501" From: Felipe Balbi To: "He\, Bo" , "linux-kernel\@vger.kernel.org" , "linux-usb\@vger.kernel.org" Cc: "gregkh\@linuxfoundation.org" , "peter.chen\@nxp.com" , "k.opasiak\@samsung.com" , "stefan\@agner.ch" , "felixhaedicke\@web.de" , "colin.king\@canonical.com" , "rogerq\@ti.com" , "f.fainelli\@gmail.com" , "Zhang\, Yanmin" Subject: RE: [PATCH] usb: gadget: udc: fix the kernel NULL pointer in composite_setup In-Reply-To: References: <87pocy81og.fsf@linux.intel.com> Date: Wed, 19 Jul 2017 10:50:58 +0300 Message-ID: <87eftc6f19.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2907 Lines: 98 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, (please don't top-post and also break your lines at 80-columns ;-) "He, Bo" writes: > 1. the issue reproduced very rarely, we run reboot test > reproduce the issue, it reproduced two times on two board after > more than 1500 cycles reboot. That's fine, we, somehow, got a use-after-free on the tracepoints. I'm interested in fixing that without touching udc-core since that's a dwc3-only bug. > 2. the kernel version is 4.4, the test case is cold reboot, I think it's= not android patches cause it, it's the interrupt thread run after the udc-= >driver->unbind. Yeah, I need you to try v4.13-rc1. v4.4 is *really* old. I can't accept your patch unless I'm certain the bug still exists. > 3. I check more drivers, like amd5536_udc_stop, at91_stop, > atmel_usba_stop, bcm63xx_udc_stop, s3c_hsudc_stop, all the > interrupt disable will be in the udc_stop(), so we need > guarantee to stop the interrupt then release the resource. Right, we also disable the interrupt on ->udc_stop(). See below: static void __dwc3_gadget_stop(struct dwc3 *dwc) { dwc3_gadget_disable_irq(dwc); __dwc3_gadget_ep_disable(dwc->eps[0]); __dwc3_gadget_ep_disable(dwc->eps[1]); } static int dwc3_gadget_stop(struct usb_gadget *g) { struct dwc3 *dwc =3D gadget_to_dwc(g); unsigned long flags; int epnum; spin_lock_irqsave(&dwc->lock, flags); if (pm_runtime_suspended(dwc->dev)) goto out; __dwc3_gadget_stop(dwc); for (epnum =3D 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) { struct dwc3_ep *dep =3D dwc->eps[epnum]; if (!dep) continue; if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) continue; wait_event_lock_irq(dep->wait_end_transfer, !(dep->flags & DWC3_EP_END_TRANSFER_PENDING), dwc->lock); } out: dwc->gadget_driver =3D NULL; spin_unlock_irqrestore(&dwc->lock, flags); free_irq(dwc->irq_gadget, dwc->ev_buf); return 0; } =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAllvD2IACgkQzL64meEa mQZzgBAAnqZYv1X+fKSVHNEQvuepi9LM1btq1mMCCwiGg5NoOxjcMRR5MRPyAOgx le+RIu/ZhlZ4HYBonYnMbfOaMoYKixMc5tDAyjAtA7TtN7ACDUDU4XS31uZ7MZhN tt/4T2IdpQ6dG+wvlFeUUH1wvxku1JARe8EB0eCy+9Wm9tgzAD+D5B9DUgo4VJF7 vWINdY9dM1dDlw9AW7QhvMfIPQr5lHAH5lIrsUwFGWV6E99obqhAK6RB7Fx8ENb/ 8yzMYfVmQKA9KLHxpfTBKxO0ja6OzSgPh3o4ZJK4VqdO0qZ+JbZV1aMtDrE2otwx qCmao5ShM19ERh7804ogHFux5UkMBHiCJfnmHc3GOx/xvul2q5O4CVFyChSc3Uqf OBoOv7PMyI27joLZtTAvm6VKRWqGAlFayK3wLrJWoUFvEGTRg3jmhI303k5GQisd vPI29VIxNsVi9oIWiE+/H3VXLk2cICwqFdZGJGpoAaSrla525LWZWAO21Z3+EGQT ozxcvjX6QEfANTyicXWrdhSGYSSKB8rFyFpw+MmuGs1Y9xudJwrEgYoFevEnbD2i 2ygqTJVtQe+NEdQ6d2OxPTmcQCYWxIJHIU5HnS9N8j8EMmq1sqqQjjxvrosys8Tk de/Yt9p+IcCOLvnA/CIyYoL2+eOBWpzYuoVAcMfWxhehV+jFgvQ= =JpSZ -----END PGP SIGNATURE----- --=-=-=--