[PATCH] genirq/irqdomain: Don't try to free an interrupt that has no mapping

[PATCH] genirq/irqdomain: Don't try to free an interrupt that has no mapping

[Marc Zyngier]

When an interrupt allocation fails for N interrupts, it is pretty
common for the error handling code to free the same number of interrupts,
no matter how many interrupts have actually been allocated.

This may result in the domain freeing code to be unexpectedly called
for interrupts that have no mapping in that domain. Things end pretty
badly.

Instead, add some checks to irq_domain_free_irqs_hierarchy() to make
sure that we don't follow the hierarchy if no mapping exists for
a given interrupt.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 kernel/irq/irqdomain.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index cf8b374b892d..4d8f5a661c2a 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1370,8 +1370,14 @@ static void irq_domain_free_irqs_hierarchy(struct irq_domain *domain,
 					   unsigned int irq_base,
 					   unsigned int nr_irqs)
 {
-	if (domain->ops->free)
-		domain->ops->free(domain, irq_base, nr_irqs);
+	int i;
+
+	if (!domain->ops->free)
+		return;
+
+	for (i = 0; i < nr_irqs; i++)
+		if (irq_domain_get_irq_data(domain, irq_base + i))
+			domain->ops->free(domain, irq_base + i, 1);
 }
 
 int irq_domain_alloc_irqs_hierarchy(struct irq_domain *domain,
-- 
2.29.2

Re: [PATCH] genirq/irqdomain: Don't try to free an interrupt that has no mapping

[Marc Zyngier]

On 2020-11-29 13:55, Marc Zyngier wrote:
> When an interrupt allocation fails for N interrupts, it is pretty
> common for the error handling code to free the same number of 
> interrupts,
> no matter how many interrupts have actually been allocated.
> 
> This may result in the domain freeing code to be unexpectedly called
> for interrupts that have no mapping in that domain. Things end pretty
> badly.
> 
> Instead, add some checks to irq_domain_free_irqs_hierarchy() to make
> sure that we don't follow the hierarchy if no mapping exists for
> a given interrupt.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

A for a good measure:

Fixes: 6a6544e520abe ("genirq/irqdomain: Remove auto-recursive hierarchy 
support")

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

[tip: irq/core] genirq/irqdomain: Don't try to free an interrupt that has no mapping

[tip-bot2 for Marc Zyngier]

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     4615fbc3788ddc8e7c6d697714ad35a53729aa2c
Gitweb:        https://git.kernel.org/tip/4615fbc3788ddc8e7c6d697714ad35a53729aa2c
Author:        Marc Zyngier <maz@kernel.org>
AuthorDate:    Sun, 29 Nov 2020 13:55:51 
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 30 Nov 2020 14:50:21 +01:00

genirq/irqdomain: Don't try to free an interrupt that has no mapping

When an interrupt allocation fails for N interrupts, it is pretty
common for the error handling code to free the same number of interrupts,
no matter how many interrupts have actually been allocated.

This may result in the domain freeing code to be unexpectedly called
for interrupts that have no mapping in that domain. Things end pretty
badly.

Instead, add some checks to irq_domain_free_irqs_hierarchy() to make sure
that thiss does not follow the hierarchy if no mapping exists for a given
interrupt.

Fixes: 6a6544e520abe ("genirq/irqdomain: Remove auto-recursive hierarchy support")
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20201129135551.396777-1-maz@kernel.org

---
 kernel/irq/irqdomain.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 3d7463f..30a7887 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1381,8 +1381,15 @@ static void irq_domain_free_irqs_hierarchy(struct irq_domain *domain,
 					   unsigned int irq_base,
 					   unsigned int nr_irqs)
 {
-	if (domain->ops->free)
-		domain->ops->free(domain, irq_base, nr_irqs);
+	unsigned int i;
+
+	if (!domain->ops->free)
+		return;
+
+	for (i = 0; i < nr_irqs; i++) {
+		if (irq_domain_get_irq_data(domain, irq_base + i))
+			domain->ops->free(domain, irq_base + i, 1);
+	}
 }
 
 int irq_domain_alloc_irqs_hierarchy(struct irq_domain *domain,