Re: [PATCH] irqdomain: Fix NULL pointer dererence in irq_domain_free_irqs_parent

[Suravee Suthikulpanit]

On 11/20/2014 08:49 PM, Jiang Liu wrote:
> On 2014/11/21 10:08, Suravee Suthikulpanit wrote:
> > On 11/20/2014 07:32 PM, Thomas Gleixner wrote:
> > > On Thu, 20 Nov 2014, suravee.suthikulpanit@xxxxxxx wrote:
> > > > This patch checks if the parent domain is NULL before recursively
> > > > freeing
> > > > irqs in the parent domains.
> > >
> > > Which is nonsense, because if the thing has not been allocated in the
> > > first place, then it cannot explode in the free path magically, except
> > > there is a missing check in the allocation path error handling.
> > >
> > > And that's obviously not the case simply because this originates from:
> > > > [<fffffe0000449278>] pci_disable_msix+0x40/0x50
> >
> > Thomas,
> >
> > In this case, I have the following irq domain hierarchy:
> >
> > [GIC] -- [GICv2m] -- [MSI]
> >
> > which recursively calling the freeing function:
> >
> > In GIC domain, it currently defines the struct irq_domain_ops.free() with :
> >    --> irq_domain_free_irqs_top()
> >      |--> irq_domain_free_irqs_common()
> >        |--> irq_domain_free_irq_parent()
> >          |--> irq_domain_free_irqs_recursive()
> >
> > and there is no check before passing the NULL domain->parent into the
> > irq_domain_free_irqs_recursive(), which causes the error.
> >
> > Since the GIC is the top most domain, it does not have parent domain.
> > So, I'm not sure what is missing from the allocation path error
> > handling, as you mentioned.
> Hi Thomas,
> 	We have had a discussion about this issue in another thread.
> Originally irq_domain_free_irqs_common() is designed to be used by
> irqdomains with parent. But there are desires to reuse it to support
> irqdomains without parent too for code reduction.
> So I suggest to change irq_domain_free_irqs_common() instead of
> irq_domain_free_irqs_parent() because caller of
> irq_domain_free_irqs_parent() should guarantee current domain do have
> a parent.
> I'm preparing a patch for this:)
> Regards!
> Gerry

Thanks Gerry and Thomas.

Suravee

> > Thanks,
> >
> > Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/