lkml.org 
[lkml]   [2010]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRE: [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



\
 
 \ /
  Last update: 2010-10-13 12:59    [W:0.060 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site