* [PATCH 8/8] sound/soc: introduce missing kfree
@ 2009-09-11 16:23 ` Julia Lawall
0 siblings, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2009-09-11 16:23 UTC (permalink / raw)
To: Takashi Iwai, Jaroslav Kysela, Mark Brown, Liam Girdwood,
alsa-devel, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
Error handling code following a kzalloc should free the allocated data.
Error handling code following an ioremap should iounmap the allocated data.
The semantic match that finds the first problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)
// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@
x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
when != if (...) { <+...x...+> }
(
x->f1 = E
|
(x->f1 == NULL || ...)
|
f(...,x->f1,...)
)
...>
(
return \(0\|<+...x...+>\|ptr\);
|
return@p2 ...;
)
@script:python@
p1 << r.p1;
p2 << r.p2;
@@
print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
sound/soc/fsl/mpc5200_dma.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
index 9ff62e3..41ffd57 100644
--- a/sound/soc/fsl/mpc5200_dma.c
+++ b/sound/soc/fsl/mpc5200_dma.c
@@ -469,8 +469,11 @@ int mpc5200_audio_dma_create(struct of_device *op)
/* Get the PSC ID */
prop = of_get_property(op->node, "cell-index", &size);
- if (!prop || size < sizeof *prop)
+ if (!prop || size < sizeof *prop) {
+ iounmap(regs);
+ kfree(psc_dma);
return -ENODEV;
+ }
spin_lock_init(&psc_dma->lock);
mutex_init(&psc_dma->mutex);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 8/8] sound/soc: introduce missing kfree
@ 2009-09-11 16:23 ` Julia Lawall
0 siblings, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2009-09-11 16:23 UTC (permalink / raw)
To: Takashi Iwai, Jaroslav Kysela, Mark Brown, Liam Girdwood,
alsa-devel, lin
From: Julia Lawall <julia@diku.dk>
Error handling code following a kzalloc should free the allocated data.
Error handling code following an ioremap should iounmap the allocated data.
The semantic match that finds the first problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)
// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@
x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x = NULL) S
<... when != x
when != if (...) { <+...x...+> }
(
x->f1 = E
|
(x->f1 = NULL || ...)
|
f(...,x->f1,...)
)
...>
(
return \(0\|<+...x...+>\|ptr\);
|
return@p2 ...;
)
@script:python@
p1 << r.p1;
p2 << r.p2;
@@
print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
sound/soc/fsl/mpc5200_dma.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
index 9ff62e3..41ffd57 100644
--- a/sound/soc/fsl/mpc5200_dma.c
+++ b/sound/soc/fsl/mpc5200_dma.c
@@ -469,8 +469,11 @@ int mpc5200_audio_dma_create(struct of_device *op)
/* Get the PSC ID */
prop = of_get_property(op->node, "cell-index", &size);
- if (!prop || size < sizeof *prop)
+ if (!prop || size < sizeof *prop) {
+ iounmap(regs);
+ kfree(psc_dma);
return -ENODEV;
+ }
spin_lock_init(&psc_dma->lock);
mutex_init(&psc_dma->mutex);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 8/8] sound/soc: introduce missing kfree
@ 2009-09-11 16:23 ` Julia Lawall
0 siblings, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2009-09-11 16:23 UTC (permalink / raw)
To: Takashi Iwai, Jaroslav Kysela, Mark Brown, Liam Girdwood,
alsa-devel, lin
From: Julia Lawall <julia@diku.dk>
Error handling code following a kzalloc should free the allocated data.
Error handling code following an ioremap should iounmap the allocated data.
The semantic match that finds the first problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)
// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@
x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
when != if (...) { <+...x...+> }
(
x->f1 = E
|
(x->f1 == NULL || ...)
|
f(...,x->f1,...)
)
...>
(
return \(0\|<+...x...+>\|ptr\);
|
return@p2 ...;
)
@script:python@
p1 << r.p1;
p2 << r.p2;
@@
print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
sound/soc/fsl/mpc5200_dma.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
index 9ff62e3..41ffd57 100644
--- a/sound/soc/fsl/mpc5200_dma.c
+++ b/sound/soc/fsl/mpc5200_dma.c
@@ -469,8 +469,11 @@ int mpc5200_audio_dma_create(struct of_device *op)
/* Get the PSC ID */
prop = of_get_property(op->node, "cell-index", &size);
- if (!prop || size < sizeof *prop)
+ if (!prop || size < sizeof *prop) {
+ iounmap(regs);
+ kfree(psc_dma);
return -ENODEV;
+ }
spin_lock_init(&psc_dma->lock);
mutex_init(&psc_dma->mutex);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 8/8] sound/soc: introduce missing kfree
2009-09-11 16:23 ` Julia Lawall
(?)
@ 2009-09-12 11:50 ` Mark Brown
-1 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2009-09-12 11:50 UTC (permalink / raw)
To: Julia Lawall
Cc: Takashi Iwai, Jaroslav Kysela, Liam Girdwood, alsa-devel,
linux-kernel, kernel-janitors
On Fri, Sep 11, 2009 at 06:23:04PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> Error handling code following a kzalloc should free the allocated data.
> Error handling code following an ioremap should iounmap the allocated data.
This doesn't seem to be a complete fix for the issue. There are several
other places where the function can fail but which don't do this cleanup
- this may indicate a problem with the automatic analysis if it fails to
spot the problem in the fixed code?
In general for this sort of issue I'd suggest considering adding the
error handling using the goto unwind block style since that tends to be
a bit more robust against forgotten error handling than adding unwind
code at each error site. It's also a bit more managable when the code
is doing a series of allocations like this is.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 8/8] sound/soc: introduce missing kfree
@ 2009-09-12 11:50 ` Mark Brown
0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2009-09-12 11:50 UTC (permalink / raw)
To: Julia Lawall
Cc: alsa-devel, Takashi Iwai, kernel-janitors, linux-kernel, Liam Girdwood
On Fri, Sep 11, 2009 at 06:23:04PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> Error handling code following a kzalloc should free the allocated data.
> Error handling code following an ioremap should iounmap the allocated data.
This doesn't seem to be a complete fix for the issue. There are several
other places where the function can fail but which don't do this cleanup
- this may indicate a problem with the automatic analysis if it fails to
spot the problem in the fixed code?
In general for this sort of issue I'd suggest considering adding the
error handling using the goto unwind block style since that tends to be
a bit more robust against forgotten error handling than adding unwind
code at each error site. It's also a bit more managable when the code
is doing a series of allocations like this is.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 8/8] sound/soc: introduce missing kfree
@ 2009-09-12 11:50 ` Mark Brown
0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2009-09-12 11:50 UTC (permalink / raw)
To: Julia Lawall
Cc: alsa-devel, Takashi Iwai, kernel-janitors, linux-kernel, Liam Girdwood
On Fri, Sep 11, 2009 at 06:23:04PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> Error handling code following a kzalloc should free the allocated data.
> Error handling code following an ioremap should iounmap the allocated data.
This doesn't seem to be a complete fix for the issue. There are several
other places where the function can fail but which don't do this cleanup
- this may indicate a problem with the automatic analysis if it fails to
spot the problem in the fixed code?
In general for this sort of issue I'd suggest considering adding the
error handling using the goto unwind block style since that tends to be
a bit more robust against forgotten error handling than adding unwind
code at each error site. It's also a bit more managable when the code
is doing a series of allocations like this is.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 8/8] sound/soc: introduce missing kfree
2009-09-12 11:50 ` Mark Brown
@ 2009-09-12 11:58 ` Julia Lawall
-1 siblings, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2009-09-12 11:58 UTC (permalink / raw)
To: Mark Brown
Cc: Takashi Iwai, Jaroslav Kysela, Liam Girdwood, alsa-devel,
linux-kernel, kernel-janitors
On Sat, 12 Sep 2009, Mark Brown wrote:
> On Fri, Sep 11, 2009 at 06:23:04PM +0200, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
>
> > Error handling code following a kzalloc should free the allocated data.
> > Error handling code following an ioremap should iounmap the allocated data.
>
> This doesn't seem to be a complete fix for the issue. There are several
> other places where the function can fail but which don't do this cleanup
> - this may indicate a problem with the automatic analysis if it fails to
> spot the problem in the fixed code?
>
> In general for this sort of issue I'd suggest considering adding the
> error handling using the goto unwind block style since that tends to be
> a bit more robust against forgotten error handling than adding unwind
> code at each error site. It's also a bit more managable when the code
> is doing a series of allocations like this is.
OK, thanks for the suggestions. I see that there is a return near the
end:
if (rc) {
free_irq(psc_dma->irq, psc_dma);
free_irq(psc_dma->capture.irq,
&psc_dma->capture);
free_irq(psc_dma->playback.irq,
&psc_dma->playback);
return -ENODEV;
}
that I missed. I will send a new patch, using the suggested goto style.
julia
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 8/8] sound/soc: introduce missing kfree
@ 2009-09-12 11:58 ` Julia Lawall
0 siblings, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2009-09-12 11:58 UTC (permalink / raw)
To: Mark Brown
Cc: Takashi Iwai, Jaroslav Kysela, Liam Girdwood, alsa-devel,
linux-kernel, kernel-janitors
On Sat, 12 Sep 2009, Mark Brown wrote:
> On Fri, Sep 11, 2009 at 06:23:04PM +0200, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
>
> > Error handling code following a kzalloc should free the allocated data.
> > Error handling code following an ioremap should iounmap the allocated data.
>
> This doesn't seem to be a complete fix for the issue. There are several
> other places where the function can fail but which don't do this cleanup
> - this may indicate a problem with the automatic analysis if it fails to
> spot the problem in the fixed code?
>
> In general for this sort of issue I'd suggest considering adding the
> error handling using the goto unwind block style since that tends to be
> a bit more robust against forgotten error handling than adding unwind
> code at each error site. It's also a bit more managable when the code
> is doing a series of allocations like this is.
OK, thanks for the suggestions. I see that there is a return near the
end:
if (rc) {
free_irq(psc_dma->irq, psc_dma);
free_irq(psc_dma->capture.irq,
&psc_dma->capture);
free_irq(psc_dma->playback.irq,
&psc_dma->playback);
return -ENODEV;
}
that I missed. I will send a new patch, using the suggested goto style.
julia
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-09-12 11:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-11 16:23 [PATCH 8/8] sound/soc: introduce missing kfree Julia Lawall
2009-09-11 16:23 ` Julia Lawall
2009-09-11 16:23 ` Julia Lawall
2009-09-12 11:50 ` Mark Brown
2009-09-12 11:50 ` Mark Brown
2009-09-12 11:50 ` Mark Brown
2009-09-12 11:58 ` Julia Lawall
2009-09-12 11:58 ` Julia Lawall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.