2010-08-24 14:39:34

by Julia Lawall

Subject: [PATCH 4/5] drivers/char/agp: Eliminate memory leak

From: Julia Lawall <[email protected]>

alloc_pci_dev allocates some memory, so that memory should be freed before
leaving the function in an error case.

A simplified version of the semantic match that finds this problem is:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E;
identifier f1;
iterator I;
@@

x = alloc_pci_dev(...);
<... when != x
when != true (x == NULL || ...)
when != if (...) { <+...x...+> }
when != I (...) { <+...x...+> }
(
x == NULL
|
x == E
|
x->f1
)
...>
* return ...;
// </smpl>

Signed-off-by: Julia Lawall <[email protected]>

---
drivers/char/agp/parisc-agp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c
index 1c12921..e2372b7 100644
--- a/drivers/char/agp/parisc-agp.c
+++ b/drivers/char/agp/parisc-agp.c
@@ -357,9 +357,10 @@ parisc_agp_setup(void __iomem *ioc_hpa, void __iomem *lba_hpa)
fake_bridge_dev->device = PCI_DEVICE_ID_HP_PCIX_LBA;
bridge->dev = fake_bridge_dev;

- error = agp_add_bridge(bridge);
+ return agp_add_bridge(bridge);

fail:
+ kfree(fake_bridge_dev);
return error;
}

2010-08-24 17:17:18

by Dan Carpenter

Subject: Re: [PATCH 4/5] drivers/char/agp: Eliminate memory leak

On Tue, Aug 24, 2010 at 04:39:29PM +0200, Julia Lawall wrote:
>
> - error = agp_add_bridge(bridge);
> + return agp_add_bridge(bridge);
>

error = agp_add_bridge(bridge);
if (error)
goto fail;
return 0;

> fail:
> + kfree(fake_bridge_dev);
> return error;
> }
>

regards,
dan carpenter

2010-08-24 19:46:29

by Julia Lawall

Subject: Re: [PATCH 4/5] drivers/char/agp: Eliminate memory leak

On Tue, 24 Aug 2010, Dan Carpenter wrote:

> On Tue, Aug 24, 2010 at 04:39:29PM +0200, Julia Lawall wrote:
> >
> > - error = agp_add_bridge(bridge);
> > + return agp_add_bridge(bridge);
> >
>
> error = agp_add_bridge(bridge);
> if (error)
> goto fail;
> return 0;

Good point. I will send another patch.

julia

> > fail:
> > + kfree(fake_bridge_dev);
> > return error;
> > }
> >
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2010-08-24 20:20:49

by Julia Lawall

Subject: Re: [PATCH 4/5] drivers/char/agp: Eliminate memory leak

From: Julia Lawall <[email protected]>

alloc_pci_dev allocates some memory, so that memory should be freed before
leaving the function in an error case.

A simplified version of the semantic match that finds this problem is:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E;
identifier f1;
iterator I;
@@

x = alloc_pci_dev(...);
<... when != x
when != true (x == NULL || ...)
when != if (...) { <+...x...+> }
when != I (...) { <+...x...+> }
(
x == NULL
|
x == E
|
x->f1
)
...>
* return ...;
// </smpl>

Signed-off-by: Julia Lawall <[email protected]>

---
drivers/char/agp/parisc-agp.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c
index 1c12921..17e380f 100644
--- a/drivers/char/agp/parisc-agp.c
+++ b/drivers/char/agp/parisc-agp.c
@@ -358,8 +358,12 @@ parisc_agp_setup(void __iomem *ioc_hpa, void __iomem *lba_hpa)
bridge->dev = fake_bridge_dev;

error = agp_add_bridge(bridge);
+ if (error)
+ goto fail;
+ return 0;

fail:
+ kfree(fake_bridge_dev);
return error;
}