ARM: imx: bug fix and convertion to platform_device_register_full()
cover ARM: imx: bug fix and convertion to platform_device_register_full()
This small series fixes a potential NULL pointer dereference bug and also converts the imx camera allocation and initialization functions to use platform_device_register_full().
The change the first patch makes is removed by the second patch, so the purpose of the first one is purely for easy backporting and git history.
Emil Goode (2):
ARM: imx: fix error handling in ipu device registration
ARM: imx: convert camera init to use platform_device_register_full()
arch/arm/mach-imx/devices/platform-ipu-core.c | 43 +++++++++---------------- arch/arm/mach-imx/mach-mx31_3ds.c | 5 ++- arch/arm/mach-imx/mach-mx31moboard.c | 6 ++-- arch/arm/mach-imx/mach-mx35_3ds.c | 5 ++- arch/arm/mach-imx/mach-pcm037.c | 5 ++- 5 files changed, 23 insertions(+), 41 deletions(-)
Signature · 1 line
1.7.10.4
PATCH v4 1/2 ARM: imx: fix error handling in ipu device registration
If we fail to allocate struct platform_device pdev we dereference it after the goto label err.
This bug was found using coccinelle.
---
v4: Simplified version that just fixes the bug.
Also updated the changelog.
v3: Made subject line more specific.
v2: Changed to return -ENOMEM instead of ret where possible and
updated the subject line.
arch/arm/mach-imx/devices/platform-ipu-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-imx/devices/platform-ipu-core.c +1 -1
@@ -77,7 +77,7 @@ struct platform_device *__init imx_alloc_mx3_camera( pdev =platform_device_alloc ("mx3-camera" ,0 );if (!pdev) - goto err; +return ERR_PTR (-ENOMEM ); pdev->dev.dma_mask =kmalloc (sizeof (*pdev->dev.dma_mask),GFP_KERNEL );if (!pdev->dev.dma_mask)
Signature · 1 line
1.7.10.4
Re: [PATCH 1/2 v4] ARM: imx: fix error handling in ipu device registration
Quoted 6 lines · Emil Goode
If we fail to allocate struct
platform_devicepdev we dereference it after the goto label err.This bug was found using coccinelle.
Signed-off-by Emil Goode <[email protected]>
Shawn, is it already to late for 3.15?
Thanks
Uwe
Signature · 6 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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
Re: [PATCH 1/2 v4] ARM: imx: fix error handling in ipu device registration
Quoted 10 lines · Uwe Kleine-König
Quoted 6 lines · Emil Goode
If we fail to allocate struct
platform_devicepdev we dereference it after the goto label err.This bug was found using coccinelle.
Signed-off-by Emil Goode <[email protected]>Acked-by Uwe Kleine-König <[email protected]>Shawn, is it already to late for 3.15?
Let me try to see.
Shawn
--
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
PATCH 2/2 ARM: imx: convert camera init to use platform_device_register_full()
In reply to Emil GoodeThis converts the imx camera allocation and initialization functions to use platform_device_register_full() thus simplifying the code.
---
Only build tested, unfortunately I currently don't have the hardware.
arch/arm/mach-imx/devices/platform-ipu-core.c | 43 +++++++++---------------- arch/arm/mach-imx/mach-mx31_3ds.c | 5 ++- arch/arm/mach-imx/mach-mx31moboard.c | 6 ++-- arch/arm/mach-imx/mach-mx35_3ds.c | 5 ++- arch/arm/mach-imx/mach-pcm037.c | 5 ++- 5 files changed, 23 insertions(+), 41 deletions(-)
arch/arm/mach-imx/devices/platform-ipu-core.c +15 -28
@@ -69,42 +69,29 @@ struct platform_device *__init imx_alloc_mx3_camera( .flags = IORESOURCE_MEM, }, }; -int ret = -ENOMEM ; -struct platform_device *pdev; +struct platform_device_info pdevinfo; +struct mx3_camera_pdata tmppdata;if (IS_ERR_OR_NULL (imx_ipu_coredev))return ERR_PTR (-ENODEV ); - pdev =platform_device_alloc ("mx3-camera" ,0 ); -if (!pdev) -return ERR_PTR (-ENOMEM ); - - pdev->dev.dma_mask =kmalloc (sizeof (*pdev->dev.dma_mask),GFP_KERNEL ); -if (!pdev->dev.dma_mask) - goto err; - - *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; +memset (&pdevinfo,0 ,sizeof (pdevinfo)); + pdevinfo.name ="mx3-camera" ; + pdevinfo.id =0 ; + pdevinfo.res = res; + pdevinfo.num_res =ARRAY_SIZE (res); + pdevinfo.dma_mask =DMA_BIT_MASK (32 );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 ); - } - copied_pdata =dev_get_platdata (&pdev->dev); - copied_pdata->dma_dev = &imx_ipu_coredev->dev; + tmppdata = *pdata; + tmppdata.dma_dev = &imx_ipu_coredev->dev; + + pdata = &tmppdata; + pdevinfo.data = pdata; + pdevinfo.size_data =sizeof (*pdata); } -return pdev; +return platform_device_register_full (&pdevinfo); } struct platform_device *__init imx_add_mx3_sdc_fb(
arch/arm/mach-imx/mach-mx31_3ds.c +2 -3
@@ -199,10 +199,9 @@ static int __init mx31_3ds_init_camera(void)if (!(dma &DMA_MEMORY_MAP )) goto err; - ret =platform_device_add (pdev); -if (ret) +return 0 ; err: -platform_device_put (pdev); +platform_device_put (pdev);return ret; }
arch/arm/mach-imx/mach-mx31moboard.c +2 -4
@@ -492,13 +492,11 @@ static int __init mx31moboard_init_cam(void)if (!(dma &DMA_MEMORY_MAP )) goto err; - ret =platform_device_add (pdev); -if (ret) +return 0 ; err: -platform_device_put (pdev); +platform_device_put (pdev);return ret; - }static void mx31moboard_poweroff (void )
arch/arm/mach-imx/mach-mx35_3ds.c +2 -3
@@ -282,10 +282,9 @@ static int __init imx35_3ds_init_camera(void)if (!(dma &DMA_MEMORY_MAP )) goto err; - ret =platform_device_add (pdev); -if (ret) +return 0 ; err: -platform_device_put (pdev); +platform_device_put (pdev);return ret; }
arch/arm/mach-imx/mach-pcm037.c +2 -3
@@ -425,10 +425,9 @@ static int __init pcm037_init_camera(void)if (!(dma &DMA_MEMORY_MAP )) goto err; - ret =platform_device_add (pdev); -if (ret) +return 0 ; err: -platform_device_put (pdev); +platform_device_put (pdev);return ret; }
Signature · 1 line
1.7.10.4
Re: [PATCH 2/2] ARM: imx: convert camera init to use platform_device_register_full()
Hello Emil,
thanks for your effort.
arch/arm/mach-imx/devices/platform-ipu-core.c struct platform_device *__init imx_alloc_mx3_camera(
Quoted 19 lines · Emil Goode
This converts the imx camera allocation and initialization functions to use
platform_device_register_full()thus simplifying the code.Signed-off-by Emil Goode <[email protected]>---
Only build tested, unfortunately I currently don't have the hardware.arch/arm/mach-imx/devices/platform-ipu-core.c | 43 +++++++++---------------- arch/arm/mach-imx/mach-mx31_3ds.c | 5 ++- arch/arm/mach-imx/mach-mx31moboard.c | 6 ++-- arch/arm/mach-imx/mach-mx35_3ds.c | 5 ++- arch/arm/mach-imx/mach-pcm037.c | 5 ++- 5 files changed, 23 insertions(+), 41 deletions(-)arch/arm/mach-imx/devices/platform-ipu-core.c +0
-0@@ -69,42 +69,29 @@ struct platform_device *__init imx_alloc_mx3_camera(
IMHO you should better rename the function because now it doesn't only allocate the device, but also registers it.
Also I doubt that it's OK to call dma_declare_coherent_memory after the device is added. In this case maybe extend platform_device_register_full? And also add a warning to dma_declare_coherent_memory if it is called for an already added device? (Added a few people to Cc: that might be able to comment this. I don't even know if there is a reliable way to check if a device is already added.)
Best regards
Uwe
Signature · 6 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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
Re: [PATCH 2/2] ARM: imx: convert camera init to use platform_device_register_full()
Hello Uwe,
Thank you for the review and sorry for the late response.
Quoted 19 lines · Uwe Kleine-König
Hello Emil,
thanks for your effort.
Quoted 14 lines · Emil Goode
This converts the imx camera allocation and initialization functions to use
platform_device_register_full()thus simplifying the code.Signed-off-by Emil Goode <[email protected]>---
Only build tested, unfortunately I currently don't have the hardware.arch/arm/mach-imx/devices/platform-ipu-core.c | 43 +++++++++---------------- arch/arm/mach-imx/mach-mx31_3ds.c | 5 ++- arch/arm/mach-imx/mach-mx31moboard.c | 6 ++-- arch/arm/mach-imx/mach-mx35_3ds.c | 5 ++- arch/arm/mach-imx/mach-pcm037.c | 5 ++- 5 files changed, 23 insertions(+), 41 deletions(-)
Quoted 7 lines
arch/arm/mach-imx/devices/platform-ipu-core.c struct platform_device *__init imx_alloc_mx3_camera(
Quoted 5 lines
arch/arm/mach-imx/devices/platform-ipu-core.c +0
-0@@ -69,42 +69,29 @@ struct platform_device *__init imx_alloc_mx3_camera(IMHO you should better rename the function because now it doesn't only allocate the device, but also registers it.
Agreed.
Quoted 8 lines
Also I doubt that it's OK to call
dma_declare_coherent_memoryafter the device is added. In this case maybe extendplatform_device_register_full? And also add a warning todma_declare_coherent_memoryif it is called for an already added device? (Added a few people to Cc: that might be able to comment this. I don't even know if there is a reliable way to check if a device is already added.)
Yes I also had doubts about that. However, apart from this patch there are three other places in arch/arm/mach-imx/ where dma_declare_coherent_memory() is called after calling platform_device_register_full().
Perhaps this is enough reason to extend platform_device_register_full().
It would be great to get some more feedback on this and also if there is a reliable way to check if a device is already added.
Best regards,
Emil Goode
Re: [PATCH 2/2] ARM: imx: convert camera init to use platform_device_register_full()
In reply to Uwe Kleine-KönigHello Uwe,
Quoted 27 lines · Uwe Kleine-König
Hello Emil,
thanks for your effort.
arch/arm/mach-imx/devices/platform-ipu-core.c struct platform_device *__init imx_alloc_mx3_camera(
Quoted 19 lines · Emil Goode
This converts the imx camera allocation and initialization functions to use
platform_device_register_full()thus simplifying the code.Signed-off-by Emil Goode <[email protected]>---
Only build tested, unfortunately I currently don't have the hardware.arch/arm/mach-imx/devices/platform-ipu-core.c | 43 +++++++++---------------- arch/arm/mach-imx/mach-mx31_3ds.c | 5 ++- arch/arm/mach-imx/mach-mx31moboard.c | 6 ++-- arch/arm/mach-imx/mach-mx35_3ds.c | 5 ++- arch/arm/mach-imx/mach-pcm037.c | 5 ++- 5 files changed, 23 insertions(+), 41 deletions(-)arch/arm/mach-imx/devices/platform-ipu-core.c +0
-0@@ -69,42 +69,29 @@ struct platform_device *__init imx_alloc_mx3_camera(IMHO you should better rename the function because now it doesn't only allocate the device, but also registers it.
Quoted 7 lines
Also I doubt that it's OK to call
dma_declare_coherent_memoryafter the device is added. In this case maybe extendplatform_device_register_full? And also add a warning todma_declare_coherent_memoryif it is called for an already added device? (Added a few people to Cc: that might be able to comment this. I don't even know if there is a reliable way to check if a device is already added.)
According to the documentation dma_declare_coherent_memory() is only used in unlikely corner cases, so I concluded that it is probably better not to include it in platform_device_register_full().
Best regards,
Emil Goode