* [PATCH 1/3] arch/powerpc: Add kmalloc NULL tests
@ 2009-08-06 20:04 Julia Lawall
2009-08-07 2:26 ` Kumar Gala
2009-08-07 6:34 ` Daniel K.
0 siblings, 2 replies; 5+ messages in thread
From: Julia Lawall @ 2009-08-06 20:04 UTC (permalink / raw)
To: benh, paulus, linuxppc-dev, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku•dk>
Check that the result of kmalloc/kzalloc is not NULL before dereferencing it.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression *x;
identifier f;
constant char *C;
@@
x = \(kmalloc\|kcalloc\|kzalloc\)(...);
... when != x == NULL
when != x != NULL
when != (x || ...)
(
kfree(x)
|
f(...,C,...,x,...)
|
*f(...,x,...)
|
*x->f
)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku•dk>
---
arch/powerpc/sysdev/fsl_rio.c | 18 ++++++++++++++----
1 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_rio.c b/arch/powerpc/sysdev/fsl_rio.c
index cbb3bed..598789c 100644
--- a/arch/powerpc/sysdev/fsl_rio.c
+++ b/arch/powerpc/sysdev/fsl_rio.c
@@ -1057,6 +1057,10 @@ int fsl_rio_setup(struct of_device *dev)
law_start, law_size);
ops = kmalloc(sizeof(struct rio_ops), GFP_KERNEL);
+ if (!ops) {
+ rc = -ENOMEM;
+ goto err_ops;
+ }
ops->lcread = fsl_local_config_read;
ops->lcwrite = fsl_local_config_write;
ops->cread = fsl_rio_config_read;
@@ -1064,6 +1068,10 @@ int fsl_rio_setup(struct of_device *dev)
ops->dsend = fsl_rio_doorbell_send;
port = kzalloc(sizeof(struct rio_mport), GFP_KERNEL);
+ if (!port) {
+ rc = -ENOMEM;
+ goto err_port;
+ }
port->id = 0;
port->index = 0;
@@ -1071,7 +1079,7 @@ int fsl_rio_setup(struct of_device *dev)
if (!priv) {
printk(KERN_ERR "Can't alloc memory for 'priv'\n");
rc = -ENOMEM;
- goto err;
+ goto err_priv;
}
INIT_LIST_HEAD(&port->dbells);
@@ -1169,13 +1177,15 @@ int fsl_rio_setup(struct of_device *dev)
return 0;
err:
- if (priv)
- iounmap(priv->regs_win);
- kfree(ops);
+ iounmap(priv->regs_win);
+err_priv:
kfree(priv);
+err_port:
kfree(port);
+err_ops:
+ kfree(ops);
return rc;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] arch/powerpc: Add kmalloc NULL tests
2009-08-06 20:04 [PATCH 1/3] arch/powerpc: Add kmalloc NULL tests Julia Lawall
@ 2009-08-07 2:26 ` Kumar Gala
2009-08-07 6:34 ` Daniel K.
1 sibling, 0 replies; 5+ messages in thread
From: Kumar Gala @ 2009-08-07 2:26 UTC (permalink / raw)
To: Julia Lawall; +Cc: kernel-janitors, paulus, linux-kernel, linuxppc-dev
On Aug 6, 2009, at 3:04 PM, Julia Lawall wrote:
> From: Julia Lawall <julia@diku•dk>
>
> Check that the result of kmalloc/kzalloc is not NULL before
> dereferencing it.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression *x;
> identifier f;
> constant char *C;
> @@
>
> x = \(kmalloc\|kcalloc\|kzalloc\)(...);
> ... when != x == NULL
> when != x != NULL
> when != (x || ...)
> (
> kfree(x)
> |
> f(...,C,...,x,...)
> |
> *f(...,x,...)
> |
> *x->f
> )
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku•dk>
>
> ---
> arch/powerpc/sysdev/fsl_rio.c | 18 ++++++++++++++----
> 1 files changed, 14 insertions(+), 4 deletions(-)
applied to next
- k
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] arch/powerpc: Add kmalloc NULL tests
2009-08-06 20:04 [PATCH 1/3] arch/powerpc: Add kmalloc NULL tests Julia Lawall
2009-08-07 2:26 ` Kumar Gala
@ 2009-08-07 6:34 ` Daniel K.
2009-08-07 6:51 ` Julia Lawall
2009-08-07 7:00 ` Julia Lawall
1 sibling, 2 replies; 5+ messages in thread
From: Daniel K. @ 2009-08-07 6:34 UTC (permalink / raw)
To: Julia Lawall; +Cc: kernel-janitors, paulus, linux-kernel, linuxppc-dev
Julia Lawall wrote:
> --- a/arch/powerpc/sysdev/fsl_rio.c
> +++ b/arch/powerpc/sysdev/fsl_rio.c
> @@ -1057,6 +1057,10 @@ int fsl_rio_setup(struct of_device *dev)
> law_start, law_size);
>
> ops = kmalloc(sizeof(struct rio_ops), GFP_KERNEL);
> + if (!ops) {
> + rc = -ENOMEM;
> + goto err_ops;
> + }
> ops->lcread = fsl_local_config_read;
> ops->lcwrite = fsl_local_config_write;
> ops->cread = fsl_rio_config_read;
> @@ -1064,6 +1068,10 @@ int fsl_rio_setup(struct of_device *dev)
> ops->dsend = fsl_rio_doorbell_send;
>
> port = kzalloc(sizeof(struct rio_mport), GFP_KERNEL);
> + if (!port) {
> + rc = -ENOMEM;
> + goto err_port;
> + }
> port->id = 0;
> port->index = 0;
>
> @@ -1071,7 +1079,7 @@ int fsl_rio_setup(struct of_device *dev)
> if (!priv) {
> printk(KERN_ERR "Can't alloc memory for 'priv'\n");
> rc = -ENOMEM;
> - goto err;
> + goto err_priv;
> }
>
> INIT_LIST_HEAD(&port->dbells);
> @@ -1169,13 +1177,15 @@ int fsl_rio_setup(struct of_device *dev)
>
> return 0;
> err:
> - if (priv)
> - iounmap(priv->regs_win);
> - kfree(ops);
> + iounmap(priv->regs_win);
> +err_priv:
> kfree(priv);
> +err_port:
> kfree(port);
> +err_ops:
> + kfree(ops);
> return rc;
There seems to be a goto-off-by-one error here.
If xxxx = kxalloc() fails, you goto err_xxxx, and do a kfree(xxxx) where xxxx is
already proven to be NULL.
Is there a reason for this that eludes me?
I'd expect that last hunk to look something like
@@ -1169,13 +1177,15 @@ int fsl_rio_setup(struct of_device *dev)
return 0;
err:
- if (priv)
- iounmap(priv->regs_win);
- kfree(ops);
+ iounmap(priv->regs_win);
kfree(priv);
+err_priv:
kfree(port);
+err_port:
+ kfree(ops);
+err_ops:
return rc;
}
Daniel K.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] arch/powerpc: Add kmalloc NULL tests
2009-08-07 6:34 ` Daniel K.
@ 2009-08-07 6:51 ` Julia Lawall
2009-08-07 7:00 ` Julia Lawall
1 sibling, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2009-08-07 6:51 UTC (permalink / raw)
To: Daniel K.; +Cc: kernel-janitors, paulus, linux-kernel, linuxppc-dev
On Fri, 7 Aug 2009, Daniel K. wrote:
> Julia Lawall wrote:
> > --- a/arch/powerpc/sysdev/fsl_rio.c
> > +++ b/arch/powerpc/sysdev/fsl_rio.c
> > @@ -1057,6 +1057,10 @@ int fsl_rio_setup(struct of_device *dev)
> > law_start, law_size);
> >
> > ops = kmalloc(sizeof(struct rio_ops), GFP_KERNEL);
> > + if (!ops) {
> > + rc = -ENOMEM;
> > + goto err_ops;
> > + }
> > ops->lcread = fsl_local_config_read;
> > ops->lcwrite = fsl_local_config_write;
> > ops->cread = fsl_rio_config_read;
> > @@ -1064,6 +1068,10 @@ int fsl_rio_setup(struct of_device *dev)
> > ops->dsend = fsl_rio_doorbell_send;
> >
> > port = kzalloc(sizeof(struct rio_mport), GFP_KERNEL);
> > + if (!port) {
> > + rc = -ENOMEM;
> > + goto err_port;
> > + }
> > port->id = 0;
> > port->index = 0;
> >
> > @@ -1071,7 +1079,7 @@ int fsl_rio_setup(struct of_device *dev)
> > if (!priv) {
> > printk(KERN_ERR "Can't alloc memory for 'priv'\n");
> > rc = -ENOMEM;
> > - goto err;
> > + goto err_priv;
> > }
> >
> > INIT_LIST_HEAD(&port->dbells);
> > @@ -1169,13 +1177,15 @@ int fsl_rio_setup(struct of_device *dev)
> >
> > return 0;
> > err:
> > - if (priv)
> > - iounmap(priv->regs_win);
> > - kfree(ops);
> > + iounmap(priv->regs_win);
> > +err_priv:
> > kfree(priv);
> > +err_port:
> > kfree(port);
> > +err_ops:
> > + kfree(ops);
> > return rc;
>
> There seems to be a goto-off-by-one error here.
>
> If xxxx = kxalloc() fails, you goto err_xxxx, and do a kfree(xxxx) where xxxx
> is
> already proven to be NULL.
>
> Is there a reason for this that eludes me?
No, I messed up... I will fix it.
julia
> I'd expect that last hunk to look something like
>
> @@ -1169,13 +1177,15 @@ int fsl_rio_setup(struct of_device *dev)
>
> return 0;
> err:
> - if (priv)
> - iounmap(priv->regs_win);
> - kfree(ops);
> + iounmap(priv->regs_win);
> kfree(priv);
> +err_priv:
> kfree(port);
> +err_port:
> + kfree(ops);
> +err_ops:
> return rc;
> }
>
>
> Daniel K.
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger•kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] arch/powerpc: Add kmalloc NULL tests
2009-08-07 6:34 ` Daniel K.
2009-08-07 6:51 ` Julia Lawall
@ 2009-08-07 7:00 ` Julia Lawall
1 sibling, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2009-08-07 7:00 UTC (permalink / raw)
To: Daniel K.; +Cc: kernel-janitors, paulus, linux-kernel, linuxppc-dev
From: Julia Lawall <julia@diku•dk>
Check that the result of kmalloc/kzalloc is not NULL before dereferencing it.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression *x;
identifier f;
constant char *C;
@@
x = \(kmalloc\|kcalloc\|kzalloc\)(...);
... when != x == NULL
when != x != NULL
when != (x || ...)
(
kfree(x)
|
f(...,C,...,x,...)
|
*f(...,x,...)
|
*x->f
)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku•dk>
---
arch/powerpc/sysdev/fsl_rio.c | 18 ++++++++++++++----
1 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_rio.c b/arch/powerpc/sysdev/fsl_rio.c
index cbb3bed..757a83f 100644
--- a/arch/powerpc/sysdev/fsl_rio.c
+++ b/arch/powerpc/sysdev/fsl_rio.c
@@ -1057,6 +1057,10 @@ int fsl_rio_setup(struct of_device *dev)
law_start, law_size);
ops = kmalloc(sizeof(struct rio_ops), GFP_KERNEL);
+ if (!ops) {
+ rc = -ENOMEM;
+ goto err_ops;
+ }
ops->lcread = fsl_local_config_read;
ops->lcwrite = fsl_local_config_write;
ops->cread = fsl_rio_config_read;
@@ -1064,6 +1068,10 @@ int fsl_rio_setup(struct of_device *dev)
ops->dsend = fsl_rio_doorbell_send;
port = kzalloc(sizeof(struct rio_mport), GFP_KERNEL);
+ if (!port) {
+ rc = -ENOMEM;
+ goto err_port;
+ }
port->id = 0;
port->index = 0;
@@ -1071,7 +1079,7 @@ int fsl_rio_setup(struct of_device *dev)
if (!priv) {
printk(KERN_ERR "Can't alloc memory for 'priv'\n");
rc = -ENOMEM;
- goto err;
+ goto err_priv;
}
INIT_LIST_HEAD(&port->dbells);
@@ -1169,11 +1177,13 @@ int fsl_rio_setup(struct of_device *dev)
return 0;
err:
- if (priv)
- iounmap(priv->regs_win);
- kfree(ops);
+ iounmap(priv->regs_win);
kfree(priv);
+err_priv:
kfree(port);
+err_port:
+ kfree(ops);
+err_ops:
return rc;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-08-07 7:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-06 20:04 [PATCH 1/3] arch/powerpc: Add kmalloc NULL tests Julia Lawall
2009-08-07 2:26 ` Kumar Gala
2009-08-07 6:34 ` Daniel K.
2009-08-07 6:51 ` Julia Lawall
2009-08-07 7:00 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox