| Message ID | 1401831034-30255-1-git-send-email-rickard_strandqvist@spectrumdigital.se |
|---|---|
| State | Changes Requested |
| Headers | show |
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
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
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; }
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(-)