Messages in this thread | | | Date | Thu, 20 Nov 2014 21:06:54 -0600 | From | Suravee Suthikulpanit <> | Subject | Re: [PATCH] irqdomain: Fix NULL pointer dererence in irq_domain_free_irqs_parent |
| |
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@amd.com 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
| |