| Message ID | 1400181871-10325-1-git-send-email-emilgoode@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, May 15, 2014 at 09:24:30PM +0200, Emil Goode wrote: > If we fail to allocate struct platform_device pdev we > dereference it after the goto label err. > > I have rearranged the error handling a bit to fix the issue > and also make it more clear. > > Signed-off-by: Emil Goode <emilgoode@gmail.com> > --- > arch/arm/mach-imx/devices/platform-ipu-core.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/mach-imx/devices/platform-ipu-core.c b/arch/arm/mach-imx/devices/platform-ipu-core.c > index fc4dd7c..14d61d9 100644 > --- a/arch/arm/mach-imx/devices/platform-ipu-core.c > +++ b/arch/arm/mach-imx/devices/platform-ipu-core.c > @@ -77,34 +77,38 @@ struct platform_device *__init imx_alloc_mx3_camera( > > pdev = platform_device_alloc("mx3-camera", 0); > if (!pdev) > - goto err; > + return ERR_PTR(ret); It's more readable to say "return ERR_PTR(-ENOMEM);". This patch is great but the subject should say "fix" and not "clean up" since it fixes a NULL dereference bug. The people who call their checkpatch.pl clean ups "fixes" are also doing it wrong but those are newbies and not worth educating. regards, dan carpenter
Hello Dan, Thanks for the review and sorry for the late reply. On Thu, May 15, 2014 at 11:32:13PM +0300, Dan Carpenter wrote: > On Thu, May 15, 2014 at 09:24:30PM +0200, Emil Goode wrote: > > If we fail to allocate struct platform_device pdev we > > dereference it after the goto label err. > > > > I have rearranged the error handling a bit to fix the issue > > and also make it more clear. > > > > Signed-off-by: Emil Goode <emilgoode@gmail.com> > > --- > > arch/arm/mach-imx/devices/platform-ipu-core.c | 22 +++++++++++++--------- > > 1 file changed, 13 insertions(+), 9 deletions(-) > > > > diff --git a/arch/arm/mach-imx/devices/platform-ipu-core.c b/arch/arm/mach-imx/devices/platform-ipu-core.c > > index fc4dd7c..14d61d9 100644 > > --- a/arch/arm/mach-imx/devices/platform-ipu-core.c > > +++ b/arch/arm/mach-imx/devices/platform-ipu-core.c > > @@ -77,34 +77,38 @@ struct platform_device *__init imx_alloc_mx3_camera( > > > > pdev = platform_device_alloc("mx3-camera", 0); > > if (!pdev) > > - goto err; > > + return ERR_PTR(ret); > > It's more readable to say "return ERR_PTR(-ENOMEM);". I think so to, will change this one but still use the ret variable at the bottom of the function. > > This patch is great but the subject should say "fix" and not "clean up" > since it fixes a NULL dereference bug. The people who call their > checkpatch.pl clean ups "fixes" are also doing it wrong but those are > newbies and not worth educating. I agree, it's a bug fix and not a clean up. Will resend tomorrow. Best regards, Emil Goode > > regards, > dan carpenter >
diff --git a/arch/arm/mach-imx/devices/platform-ipu-core.c b/arch/arm/mach-imx/devices/platform-ipu-core.c index fc4dd7c..14d61d9 100644 --- a/arch/arm/mach-imx/devices/platform-ipu-core.c +++ b/arch/arm/mach-imx/devices/platform-ipu-core.c @@ -77,34 +77,38 @@ struct platform_device *__init imx_alloc_mx3_camera( pdev = platform_device_alloc("mx3-camera", 0); if (!pdev) - goto err; + return ERR_PTR(ret); pdev->dev.dma_mask = kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL); if (!pdev->dev.dma_mask) - goto err; + goto put_pdev; *pdev->dev.dma_mask = DMA_BIT_MASK(32); pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res)); if (ret) - goto err; + goto free_dma_mask; if (pdata) { struct mx3_camera_pdata *copied_pdata; ret = platform_device_add_data(pdev, pdata, sizeof(*pdata)); - if (ret) { -err: - kfree(pdev->dev.dma_mask); - platform_device_put(pdev); - return ERR_PTR(-ENODEV); - } + if (ret) + goto free_dma_mask; + copied_pdata = dev_get_platdata(&pdev->dev); copied_pdata->dma_dev = &imx_ipu_coredev->dev; } return pdev; + +free_dma_mask: + kfree(pdev->dev.dma_mask); +put_pdev: + platform_device_put(pdev); + + return ERR_PTR(ret); } struct platform_device *__init imx_add_mx3_sdc_fb(
If we fail to allocate struct platform_device pdev we dereference it after the goto label err. I have rearranged the error handling a bit to fix the issue and also make it more clear. Signed-off-by: Emil Goode <emilgoode@gmail.com> --- arch/arm/mach-imx/devices/platform-ipu-core.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)