Messages in this thread | | | Date | Tue, 5 Feb 2013 09:14:08 +0800 | Subject | Re: [PATCH v2] i2c: busses/i2c-pxa.c: fix potential null pointer dereference error | From | Haojian Zhuang <> |
| |
On Tue, Feb 5, 2013 at 8:05 AM, Cong Ding <dinggnu@gmail.com> wrote: > If it goes to eclk through line 1107, the variable res would be NULL. It will > cause a null pointer dereference error if we call release_mem_region. > > Signed-off-by: Cong Ding <dinggnu@gmail.com> > --- > drivers/i2c/busses/i2c-pxa.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c > index 1034d93..00df535 100644 > --- a/drivers/i2c/busses/i2c-pxa.c > +++ b/drivers/i2c/busses/i2c-pxa.c > @@ -1211,7 +1211,8 @@ eremap: > eclk: > kfree(i2c); > emalloc: > - release_mem_region(res->start, resource_size(res)); > + if (res) > + release_mem_region(res->start, resource_size(res)); > return ret; > } > >
No. I don't agree on this. Your fix can't resolve all potential issues.
i2c = kzalloc(sizeof(struct pxa_i2c), GFP_KERNEL); if (!i2c) { ret = -ENOMEM; goto emalloc; }
ret = i2c_pxa_probe_dt(dev, i2c, &i2c_type); if (ret > 0) ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type); if (ret < 0) goto eclk;
res = platform_get_resource(dev, IORESOURCE_MEM, 0); irq = platform_get_irq(dev, 0); if (res == NULL || irq < 0) { ret = -ENODEV; goto eclk; }
if (!request_mem_region(res->start, resource_size(res), res->name)) { ret = -ENOMEM; goto eclk; }
We shouldn't jump to eclk for these error cases. Then we needn't to add checking on res.
Regards Haojian
| |