[PATCH] i2c: busses: i2c-pxa.c: Fix for possible null pointer dereferenc

[PATCH] i2c: busses: i2c-pxa.c: Fix for possible null pointer dereferenc

[Rickard Strandqvist]

Fix for possible null pointer dereferenc, and there is a risk
for memory leak in when something unexpected happens and the
function returns.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
 drivers/i2c/busses/i2c-pxa.c |   19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index bbe6dfb..948a3c7 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1142,10 +1142,8 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	int ret, irq;
 
 	i2c = kzalloc(sizeof(struct pxa_i2c), GFP_KERNEL);
-	if (!i2c) {
-		ret = -ENOMEM;
-		goto emalloc;
-	}
+	if (!i2c)
+		return -ENOMEM;
 
 	/* Default adapter num to device id; i2c_pxa_probe_dt can override. */
 	i2c->adap.nr = dev->id;
@@ -1154,11 +1152,16 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	if (ret > 0)
 		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
 	if (ret < 0)
-		goto eclk;
+		goto emalloc;
 
 	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+	if (res == NULL) {
+		ret = -ENODEV;
+		goto emalloc;
+	}
+
 	irq = platform_get_irq(dev, 0);
-	if (res == NULL || irq < 0) {
+	if (irq < 0) {
 		ret = -ENODEV;
 		goto eclk;
 	}
@@ -1267,9 +1270,9 @@ ereqirq:
 eremap:
 	clk_put(i2c->clk);
 eclk:
-	kfree(i2c);
-emalloc:
 	release_mem_region(res->start, resource_size(res));
+emalloc:
+	kfree(i2c);
 	return ret;
 }
 
-- 
1.7.10.4

Re: [PATCH] i2c: busses: i2c-pxa.c: Fix for possible null pointer dereferenc

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

On Tue, Jun 03, 2014 at 11:30:34PM +0200, Rickard Strandqvist wrote:
> Fix for possible null pointer dereferenc, and there is a risk
> for memory leak in when something unexpected happens and the
> function returns.
> 
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
>  drivers/i2c/busses/i2c-pxa.c |   19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index bbe6dfb..948a3c7 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -1142,10 +1142,8 @@ static int i2c_pxa_probe(struct platform_device *dev)
>  	int ret, irq;
>  
>  	i2c = kzalloc(sizeof(struct pxa_i2c), GFP_KERNEL);
> -	if (!i2c) {
> -		ret = -ENOMEM;
> -		goto emalloc;
> -	}
> +	if (!i2c)
> +		return -ENOMEM;
>  
>  	/* Default adapter num to device id; i2c_pxa_probe_dt can override. */
>  	i2c->adap.nr = dev->id;
> @@ -1154,11 +1152,16 @@ static int i2c_pxa_probe(struct platform_device *dev)
>  	if (ret > 0)
>  		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
>  	if (ret < 0)
> -		goto eclk;
> +		goto emalloc;
>  
>  	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
> +	if (res == NULL) {
> +		ret = -ENODEV;
> +		goto emalloc;
> +	}
> +
>  	irq = platform_get_irq(dev, 0);
> -	if (res == NULL || irq < 0) {
> +	if (irq < 0) {
>  		ret = -ENODEV;
>  		goto eclk;

Nope, those should be emalloc as well since the region was not requested
here. May I ask you to just convert this driver to managed devices (devm_*
functions)? They are made to exactly avoid this hazzle and cleanup the
driver.

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: busses: i2c-pxa.c: Fix for possible null pointer dereferenc

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 442 bytes --]

Hi,

please *always* reply to the list.

> So how do you want me to change it, or maybe it's not even meant for
> me to do that?

If you want, I'd be happy if you do it. Tests on real hardware would be
needed, though. Do you have one? Maybe somebody here can help you?

An example is commit 9b2b98a3b4de. More details, including a list of
available functions, can be found here: Documentation/driver-model/devres.txt

Good luck!

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]