From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753185AbbIKPFw (ORCPT ); Fri, 11 Sep 2015 11:05:52 -0400 Received: from bear.ext.ti.com ([192.94.94.41]:49053 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751795AbbIKPFt (ORCPT ); Fri, 11 Sep 2015 11:05:49 -0400 Date: Fri, 11 Sep 2015 10:05:20 -0500 From: Felipe Balbi To: Sudip Mukherjee CC: Felipe Balbi , David Cohen , Thomas Dahlmann , Greg Kroah-Hartman , , , Subject: Re: [PATCH] usb: gadget: amd5536udc: fix NULL pointer dereference Message-ID: <20150911150520.GE17326@saruman.tx.rr.com> Reply-To: References: <1441366943-9390-1-git-send-email-sudipm.mukherjee@gmail.com> <20150910180334.GA10760@psi-dev26.jf.intel.com> <20150911100256.GA15689@sudip-pc> <20150911132834.GB17326@saruman.tx.rr.com> <20150911142101.GA19150@sudip-pc> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="jKBxcB1XkHIR0Eqt" Content-Disposition: inline In-Reply-To: <20150911142101.GA19150@sudip-pc> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --jKBxcB1XkHIR0Eqt Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 11, 2015 at 07:51:01PM +0530, Sudip Mukherjee wrote: > On Fri, Sep 11, 2015 at 08:28:34AM -0500, Felipe Balbi wrote: > > Hi, > >=20 > > On Fri, Sep 11, 2015 at 03:32:56PM +0530, Sudip Mukherjee wrote: > > > On Thu, Sep 10, 2015 at 11:03:34AM -0700, David Cohen wrote: > > > no, the check is pointless. Most of these are. Just look at your probe() > > and you'll see that if dev->virt_addr is NULL (meaning ioremap_nocache() > > failed) you exit from probe() with error. The driver doesn't probe at > > all. So you can be sure that by remove, dev->regs is valid. > >=20 > > BTW, if probe fails, you have a TON of leaked resources!! You don't kfr= ee() > > dev, you don't pci_disable_device(), you don't release_mem_region(), you > > don't iounmap() virt_addr, you don't free_irq(). > >=20 > > Also the iounmap() call in remove is wrong. You ioremapped > > dev->virt_addr but iounmap() dev->regs. Are you SURE that's ok ? > >=20 > > Man, what a mess! You gotta fix that up. > :( > Yes, total mess. I am on it. > BTW, while I am fixing it can i also include a patch which will > rearrange the functions and remove the forward declarations? sure, just make sure fixes and cleanups are separate. First all fixes, then cleanups and reorderings. cheers --=20 balbi --jKBxcB1XkHIR0Eqt Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV8u2wAAoJEIaOsuA1yqREWVcQAK8Wdh0qJBNt2KpuuIGEOV7f 6uNNvVyDujBG/gXj7qz6MYXc9miF/najU8gtlJmqI5v/I0i23ZClLMBYgYGGBVNd nqJtlq6weOR3QVhRyUVb+37RaJ/AKXVs+rIVgCG1QmKb9p2x2Pj9IuwMj3UL8A4Z IbZc5BMMWPOgeIqzKQTVU2tryukWp+votRVvMz6yLSEgrXUg1CbgoJd+l9z3WVGy u6TFKPnUJkNpp6uPabH2eRSzT647bdJ4tVeD0cF2SKmSyHc5LRUBNz5+3O4ZkGZF lkCR4/Xn1u2fYqxAxPvfxQ9BKwvtqp/XOWGPP525cJvNQ7lGjL5xdT0uMPTKnFu9 aV2xxpC4hx3gflWTCAT1LMRvB+MvQaTgnypznnmDMNSfrvkRnfiA7PVR03AKQemZ DZ9AMsQUMu74vTwgT+4c90UXi10UwCV8v7BosaZu38rXxTbEi+GTZlD8EQL7k2bQ /ogrs4mi4kcQn6W5cPuenIoZYDPr5fEXgfBrl0fG2fyt9lWw87qKl5sYC0O6PAm6 5n4wgh0Lk8nct0nvGKdZd6Fn68vqaeA3/i81YAr2V2CdvfVmsp2Vkmf1C4DLkuqi QJKGRsvuCNBc9kyrrAThaSyWIbJujEC473u23vGbG7TursPGFN1+4gW2e0nvzutq wIDziUGxKo7vl3EjNUB3 =jB+7 -----END PGP SIGNATURE----- --jKBxcB1XkHIR0Eqt--