Re: [PATCH v1] irqchip: irq-mips-gic:- Handle return NULL error from ioremap_nocache

From: Matt Redfearn
Date: Tue Jan 17 2017 - 05:18:09 EST




On 17/01/17 10:10, Arvind Yadav wrote:
Hi Matt,

Please Acknowledge this.

Regards
Arvind Yadav

Hi Arvind,

Acked-by: Matt Redfearn <matt.redfearn@xxxxxxxxxx>



On Monday 09 January 2017 02:30 PM, Marc Zyngier wrote:
On 09/01/17 08:08, Arvind Yadav wrote:
Here, If ioremap_nocache will fail. It will return NULL.
Kernel can run into a NULL-pointer dereference.
This error check will avoid NULL pointer dereference.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@xxxxxxxxx>
---
drivers/irqchip/irq-mips-gic.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index c01c09e..eeea2e8 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -979,6 +979,8 @@ static void __init __gic_init(unsigned long gic_base_addr,
__gic_base_addr = gic_base_addr;
gic_base = ioremap_nocache(gic_base_addr, gic_addrspace_size);
+ if (!gic_base)
+ panic("Failed to map GIC memory");
So you're replacing a panic due to dereferencing a NULL pointer with
another panic -- not much progress here. I appreciate that the message
is a bit more explicit, but is there something slightly less drastic we
could do? Like returning an error code and see if the kernel otherwise
recovers (possibly with reduced functionality)?

Marc, there's really not much that can be done without the GIC initializing sucessfully. This should not happen if platform code / device tree is set up correctly, so an explicit panic if that is not the case is helpful.

Thanks,
Matt


gicconfig = gic_read(GIC_REG(SHARED, GIC_SH_CONFIG));
gic_shared_intrs = (gicconfig & GIC_SH_CONFIG_NUMINTRS_MSK) >>

Thanks,

M.