lkml.org 
[lkml]   [2009]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/3] arch/powerpc: Add kmalloc NULL tests
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.

\
 
 \ /
  Last update: 2009-08-07 08:41    [W:0.043 / U:22.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site