On Thu, Aug 12, 2010 at 05:35:04PM +0300, Maxim Levitsky wrote:
> On Thu, 2010-08-12 at 09:46 +0200, Dan Carpenter wrote:
> > There were a couple issues here. If the allocation failed for "dev"
> > then it would lead to a NULL dereference. If request_irq() or
> > request_region() failed it would release the irq and the region even
> > though they were not successfully aquired.
> >
> > Signed-off-by: Dan Carpenter <error27@xxxxxxxxx>
>
> I don't think this is needed.
> I just alloc all the stuff, and if one of allocations fail, I free them
> all. {k}free on NULL pointer is perfectly legal.
>
> Same about IO and IRQ.
> IRQ0 and IO 0 isn't valid, and I do test that in error path.
>
>
Here is the original code:
Here is where we set "dev".
785 dev = kzalloc(sizeof(struct ene_device), GFP_KERNEL);
786
787 if (!input_dev || !ir_props || !dev)
788 goto error;
[snip]
Here is where we set the IO and IRQ:
800 dev->hw_io = pnp_port_start(pnp_dev, 0);
801 dev->irq = pnp_irq(pnp_dev, 0);
[snip]
Here is where the request_region() and request_irq() are.
806 if (!request_region(dev->hw_io, ENE_MAX_IO, ENE_DRIVER_NAME))
807 goto error;
808
809 if (request_irq(dev->irq, ene_isr,
810 IRQF_SHARED, ENE_DRIVER_NAME, (void *)dev))
811 goto error;
[snip]
Here is the error label:
897 error:
898 if (dev->irq)
^^^^^^^^
Oops! The allocation of dev failed and this is a NULL
dereference.
899 free_irq(dev->irq, dev);
Oops! Request region failed and dev->irq is non-zero but
request_irq() hasn't been called.
900 if (dev->hw_io)
901 release_region(dev->hw_io, ENE_MAX_IO);
Oops! dev->hw_io is non-zero but request_region() failed and so
we just released someone else's region.
Hehe. :P
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
