iommu/amd: fix a small leak in irq_remapping_alloc()
Subsystems (2)
PATCH 1/1 iommu/amd: fix a small leak in irq_remapping_alloc()
There is a memory leak here that was detected by Smatch:
drivers/iommu/amd_iommu.c:4261 irq_remapping_alloc()
warn: possible memory leak of 'data'
It happens if you hit the "if (!irq_data || !cfg) {" on the first iteration through the loop. The original code was a bit weird. For example, it treated the first allocation as a special case for some reason. Anyway I cleaned it up a bit.
---
Please review this carefully. I haven't tested it.
drivers/iommu/amd_iommu.c +9 -13
@@ -4208,11 +4208,6 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq,if (ret <0 )return ret; - ret = -ENOMEM ; - data =kzalloc (sizeof (*data),GFP_KERNEL ); -if (!data) - goto out_free_parent; -if (info->type =X86_IRQ_ALLOC_TYPE_IOAPIC ) {if (get_irq_table (devid, true)) index = info->ioapic_pin;
@@ -4223,7 +4218,6 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq, }if (index <0 ) {pr_warn ("Failed to allocate IRTE\n" ); -kfree (data); goto out_free_parent; }
@@ -4232,14 +4226,16 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq, cfg =irqd_cfg (irq_data);if (!irq_data || !cfg) { ret = -EINVAL ; - goto out_free_data; + goto out_unwind_loop; } -if (i >0 ) { - data =kzalloc (sizeof (*data),GFP_KERNEL ); -if (!data) - goto out_free_data; + + data =kzalloc (sizeof (*data),GFP_KERNEL ); +if (!data) { + ret = -ENOMEM ; + goto out_unwind_loop; } + irq_data->hwirq = (devid <<16 ) + i; irq_data->chip_data = data; irq_data->chip = &amd_ir_chip;
@@ -4248,8 +4244,8 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq, }return 0 ; -out_free_data: -for (i--; i >=0 ; i--) { +out_unwind_loop: +while (--i >=0 ) { irq_data =irq_domain_get_irq_data (domain, virq + i);if (irq_data)kfree (irq_data->chip_data);
Quoted 10 lines · Dan Carpenter
There is a memory leak here that was detected by Smatch:
drivers/iommu/amd_iommu.c:4261irq_remapping_alloc()
warn: possible memory leak of 'data'It happens if you hit the "if (!irq_data || !cfg) {" on the first iteration through the loop. The original code was a bit weird. For example, it treated the first allocation as a special case for some reason. Anyway I cleaned it up a bit.
Quoted 4 lines
Fixes ecf87b38d902 ('iommu/amd: Enhance AMD IR driver to suppport hierarchy irqdomain')Signed-off-by Dan Carpenter <[email protected]>---
Please review this carefully. I haven't tested it.
Hi Dan,
Thanks for fixing this leakage, it also makes code clearer.
The strange code is wreckage of original code, should be cleaned up:)
Quoted 64 lines
drivers/iommu/amd_iommu.c +9
-14@@ -4208,11 +4208,6 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq,if (ret <0 )return ret; - ret = -ENOMEM ; - data =kzalloc (sizeof (*data),GFP_KERNEL ); -if (!data) - goto out_free_parent; -if (info->type =X86_IRQ_ALLOC_TYPE_IOAPIC ) {if (get_irq_table (devid, true)) index = info->ioapic_pin;@@ -4223,7 +4218,6 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq, }if (index <0 ) {pr_warn ("Failed to allocate IRTE\n" ); -kfree (data); goto out_free_parent; }@@ -4232,14 +4226,16 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq, cfg =irqd_cfg (irq_data);if (!irq_data || !cfg) { ret = -EINVAL ; - goto out_free_data; + goto out_unwind_loop; } -if (i >0 ) { - data =kzalloc (sizeof (*data),GFP_KERNEL ); -if (!data) - goto out_free_data; + + data =kzalloc (sizeof (*data),GFP_KERNEL ); +if (!data) { + ret = -ENOMEM ; + goto out_unwind_loop; } + irq_data->hwirq = (devid <<16 ) + i; irq_data->chip_data = data; irq_data->chip = &amd_ir_chip;@@ -4248,8 +4244,8 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq, }return 0 ; -out_free_data: -for (i--; i >=0 ; i--) { +out_unwind_loop: +while (--i >=0 ) { irq_data =irq_domain_get_irq_data (domain, virq + i);if (irq_data)kfree (irq_data->chip_data); --To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/