Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1224244 > unrolled thread

[PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-14 17:20 +0200
    Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Felipe Balbi <balbi@ti.com> - 2015-09-18 20:50 +0200
      Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-19 06:00 +0200
        Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-20 10:20 +0200
          Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Felipe Balbi <balbi@ti.com> - 2015-09-20 18:20 +0200
            Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-09-21 14:50 +0200
              Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Felipe Balbi <balbi@ti.com> - 2015-09-21 16:50 +0200
        Re: [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks Felipe Balbi <balbi@ti.com> - 2015-09-20 18:20 +0200

#1224244 — [PATCH 00/16] usb: gadget: amd5536udc: fix memory leaks

This amd5536udc was a complete mess. The major problems that i could
find are:

1) if udc_pci_probe() fails in any stage then it just calls the
udc_pci_remove() to handle error. And udc_pci_remove() works with
struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
incase of error we are handling the error by dereferencing a NULL
pointer.

2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
will be set only if probe is success. So that means if probe fails then
probe will call udc_pci_remove() for error handling and udc_pci_remove()
will inturn halts the kernel by calling BUG().

And apart from these numerous memory leaks and not releasing of
resources. Here comes a rewrite of few of the functions in an
attempt to fix these.

regards
sudip

Sudip Mukherjee (16):
  usb: gadget: amd5536udc: introduce free_dma_pools
  usb: gadget: amd5536udc: rewrite init_dma_pools
  usb: gadget: amd5536udc: rewrite udc_pci_probe
  usb: gadget: amd5536udc: use WARN_ON
  usb: gadget: amd5536udc: use free_dma_pools
  usb: gadget: amd5536udc: remove unnecessary conditions
  usb: gadget: amd5536udc: unmap virt_addr
  usb: gadget: amd5536udc: remove forward declaration of udc_probe
  usb: gadget: amd5536udc: remove forward declaration of udc_remote_wakeup
  usb: gadget: amd5536udc: remove forward declaration of udc_create_dma_chain
  usb: gadget: amd5536udc: remove forward declaration of udc_free_dma_chain
  usb: gadget: amd5536udc: remove forward declaration of udc_pci_*
  usb: gadget: amd5536udc: remove forward declaration of udc_basic_init
  usb: gadget: amd5536udc: NULL comparison
  usb: gadget: amd5536udc: remove multiple blank lines
  usb: gadget: amd5536udc: match alignment

 drivers/usb/gadget/udc/amd5536udc.c | 797 ++++++++++++++++++------------------
 1 file changed, 390 insertions(+), 407 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1228228

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> This amd5536udc was a complete mess. The major problems that i could
> find are:
> 
> 1) if udc_pci_probe() fails in any stage then it just calls the
> udc_pci_remove() to handle error. And udc_pci_remove() works with
> struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
> pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
> incase of error we are handling the error by dereferencing a NULL
> pointer.
> 
> 2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
> will be set only if probe is success. So that means if probe fails then
> probe will call udc_pci_remove() for error handling and udc_pci_remove()
> will inturn halts the kernel by calling BUG().
> 
> And apart from these numerous memory leaks and not releasing of
> resources. Here comes a rewrite of few of the functions in an
> attempt to fix these.

run checkpatch.pl and try again

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1228379

On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > This amd5536udc was a complete mess. The major problems that i could
> > find are:
> > 
> > 1) if udc_pci_probe() fails in any stage then it just calls the
> > udc_pci_remove() to handle error. And udc_pci_remove() works with
> > struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
> > pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
> > incase of error we are handling the error by dereferencing a NULL
> > pointer.
> > 
> > 2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
> > will be set only if probe is success. So that means if probe fails then
> > probe will call udc_pci_remove() for error handling and udc_pci_remove()
> > will inturn halts the kernel by calling BUG().
> > 
> > And apart from these numerous memory leaks and not releasing of
> > resources. Here comes a rewrite of few of the functions in an
> > attempt to fix these.
> 
> run checkpatch.pl and try again
I know checkpatch gives warning on some of my patches but as the warning
was not related to the part I have modified so I have not done any thing
with them as they will become unrelated changes than what is mentioned
in the commit log.
Anyways, I will fix up all the warnings and send v2. But do you want me
to also fix the checkpatch warnings in those patch where functions are
rearranged? Because in those patches functions were just moved and there
was no change in the body of the function.

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1228843

On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > This amd5536udc was a complete mess. The major problems that i could
> > > find are:
> > > 
<snip>
> > 
> > run checkpatch.pl and try again
<snip>
> Anyways, I will fix up all the warnings and send v2.

I guess v2 is not required any more. The main thing that this series was
trying to do has already been done by:
6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1228898

[Multipart message — attachments visible in raw view] — view raw

On Sun, Sep 20, 2015 at 01:42:42PM +0530, Sudip Mukherjee wrote:
> On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> > On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > > This amd5536udc was a complete mess. The major problems that i could
> > > > find are:
> > > > 
> <snip>
> > > 
> > > run checkpatch.pl and try again
> <snip>
> > Anyways, I will fix up all the warnings and send v2.
> 
> I guess v2 is not required any more. The main thing that this series was
> trying to do has already been done by:
> 6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")

all right, see if there's anything missing, please.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1229263

On Sun, Sep 20, 2015 at 11:17:36AM -0500, Felipe Balbi wrote:
> On Sun, Sep 20, 2015 at 01:42:42PM +0530, Sudip Mukherjee wrote:
> > On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> > > On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > > > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > > > This amd5536udc was a complete mess. The major problems that i could
> > > > > find are:
> > > > > 
> > <snip>
> > > > 
> > > > run checkpatch.pl and try again
> > <snip>
> > > Anyways, I will fix up all the warnings and send v2.
> > 
> > I guess v2 is not required any more. The main thing that this series was
> > trying to do has already been done by:
> > 6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")
> 
> all right, see if there's anything missing, please.
I think something still needs to be done there. I will send a v2 for
your review.

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229442

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 21, 2015 at 06:18:04PM +0530, Sudip Mukherjee wrote:
> On Sun, Sep 20, 2015 at 11:17:36AM -0500, Felipe Balbi wrote:
> > On Sun, Sep 20, 2015 at 01:42:42PM +0530, Sudip Mukherjee wrote:
> > > On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> > > > On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > > > > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > > > > This amd5536udc was a complete mess. The major problems that i could
> > > > > > find are:
> > > > > > 
> > > <snip>
> > > > > 
> > > > > run checkpatch.pl and try again
> > > <snip>
> > > > Anyways, I will fix up all the warnings and send v2.
> > > 
> > > I guess v2 is not required any more. The main thing that this series was
> > > trying to do has already been done by:
> > > 6527cc27761a ("usb: gadget: amd5536udc: fix error handling in udc_pci_probe()")
> > 
> > all right, see if there's anything missing, please.
> I think something still needs to be done there. I will send a v2 for
> your review.

cool, thanks

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1228899

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Sat, Sep 19, 2015 at 09:24:38AM +0530, Sudip Mukherjee wrote:
> On Fri, Sep 18, 2015 at 01:39:54PM -0500, Felipe Balbi wrote:
> > On Mon, Sep 14, 2015 at 08:42:47PM +0530, Sudip Mukherjee wrote:
> > > This amd5536udc was a complete mess. The major problems that i could
> > > find are:
> > > 
> > > 1) if udc_pci_probe() fails in any stage then it just calls the
> > > udc_pci_remove() to handle error. And udc_pci_remove() works with
> > > struct udc *dev which we get from pci_get_drvdata(pdev). But we do the
> > > pci_set_drvdata(pdev, dev) almost at the end of probe. So basically
> > > incase of error we are handling the error by dereferencing a NULL
> > > pointer.
> > > 
> > > 2) udc_pci_remove() does a BUG_ON(dev->driver != NULL) and dev->driver
> > > will be set only if probe is success. So that means if probe fails then
> > > probe will call udc_pci_remove() for error handling and udc_pci_remove()
> > > will inturn halts the kernel by calling BUG().
> > > 
> > > And apart from these numerous memory leaks and not releasing of
> > > resources. Here comes a rewrite of few of the functions in an
> > > attempt to fix these.
> > 
> > run checkpatch.pl and try again
> I know checkpatch gives warning on some of my patches but as the warning
> was not related to the part I have modified so I have not done any thing
> with them as they will become unrelated changes than what is mentioned
> in the commit log.
> Anyways, I will fix up all the warnings and send v2. But do you want me
> to also fix the checkpatch warnings in those patch where functions are
> rearranged? Because in those patches functions were just moved and there
> was no change in the body of the function.

sure, just add a note "while at that, also fix checkpatch warnings"

-- 
balbi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web