Messages in this thread | | | From | "Varadarajan, Charulatha" <> | Date | Wed, 13 Oct 2010 16:25:08 +0530 | Subject | RE: [PATCHv2 1/1] omap: Ptr "isr_reg" tracked as NULL was dereferenced |
| |
> -----Original Message----- > From: Evgeny Kuznetsov [mailto:EXT-Eugeny.Kuznetsov@nokia.com] > Sent: Wednesday, October 13, 2010 3:35 PM > To: tony@atomide.com > Cc: linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; Balbi, Felipe; linux@arm.linux.org.uk; > khilman@deeprootsystems.com; akpm@linux-foundation.org; Varadarajan, > Charulatha; tero.kristo@nokia.com; ext-eugeny.kuznetsov@nokia.com > Subject: [PATCHv2 1/1] omap: Ptr "isr_reg" tracked as NULL was > dereferenced > > From: Evgeny Kuznetsov <ext-eugeny.kuznetsov@nokia.com> > > Value of "isr_reg" pointer is depend on configuration and GPIO method. > Potentially it may have NULL value and it is dereferenced later > in code. If pointer is NULL there is some kernel issue.
Can you elaborate?
> Warning and exit from function are added in this case. > Also compilation check is added for correct architecture > configuration. > > Signed-off-by: Evgeny Kuznetsov <EXT-Eugeny.Kuznetsov@nokia.com> > --- > arch/arm/plat-omap/gpio.c | 18 ++++++++++++++++++ > 1 files changed, 18 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c > index c05c653..d04913c 100644 > --- a/arch/arm/plat-omap/gpio.c > +++ b/arch/arm/plat-omap/gpio.c > @@ -1318,6 +1318,23 @@ static void gpio_irq_handler(unsigned int irq, > struct irq_desc *desc) > if (bank->method == METHOD_GPIO_44XX) > isr_reg = bank->base + OMAP4_GPIO_IRQSTATUS0; > #endif > + > +#if !defined(CONFIG_ARCH_OMAP1) && \ > + !defined(CONFIG_ARCH_OMAP15XX) && \ > + !defined(CONFIG_ARCH_OMAP16XX) && \ > + !defined(CONFIG_ARCH_OMAP730) && \ > + !defined(CONFIG_ARCH_OMAP850) && \ > + !defined(CONFIG_ARCH_OMAP2) && \ > + !defined(CONFIG_ARCH_OMAP3) && \ > + !defined(CONFIG_ARCH_OMAP4) > + > +#error "Incorrect arch configuration"
This is not required. If the architecture is not one of the above mentioned, gpio_irq_handler() will not be used/called at all.
Also all the possible gpio methods for a given OMAP architecture are already considered with "#ifdef"s and (bank->method) checks in gpio_irq_handler().
> + > +#endif > + > + if (WARN_ON(!isr_reg)) > + goto exit;
For the above mentioned reason, this isr_reg would be non-NULL. Have you observed this error anytime?
Also, the omap-gpio code has similar code spread all over and has to be anyway cleaned-up. Is there any reason why gpio_irq_handler() alone is addressed in this patch?
> + > while(1) { > u32 isr_saved, level_mask = 0; > u32 enabled; > @@ -1377,6 +1394,7 @@ static void gpio_irq_handler(unsigned int irq, > struct irq_desc *desc) > configured, we must unmask the bank interrupt only after > handler(s) are executed in order to avoid spurious bank > interrupt */ > +exit: > if (!unmasked) > desc->chip->unmask(irq); > > -- > 1.6.3.3
| |