iommu/intel: Fix potential memory leak in intel_setup_irq_remapping
Subsystems (2)
PATCH 1/1 iommu/intel: Fix potential memory leak in intel_setup_irq_remapping
Call irq_domain_free_fwnode to free 'fn' if dmar_enable_qi returns failure, same as when irq_domain_create_hierarchy returns failure.
--- drivers/iommu/intel/irq_remapping.c | 1 + 1 file changed, 1 insertion(+)
drivers/iommu/intel/irq_remapping.c +1 -0
@@ -594,6 +594,7 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu)dmar_disable_qi (iommu);if (dmar_enable_qi (iommu)) { +irq_domain_free_fwnode (fn);pr_err ("Failed to enable queued invalidation\n" ); goto out_free_bitmap; }
Signature · 6 lines
2.31.1 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
Lu Baolu
maintainerQuoted 4 lines · Guoqing Jiang
Call
irq_domain_free_fwnodeto free 'fn' ifdmar_enable_qireturns failure, same as whenirq_domain_create_hierarchyreturns failure.Signed-off-by Guoqing Jiang <[email protected]>
Add a "Fixes" tag if you think this change needs backport.
Quoted 10 lines
--- drivers/iommu/intel/irq_remapping.c | 1 + 1 file changed, 1 insertion(+)drivers/iommu/intel/irq_remapping.c +0
-0
drivers/iommu/intel/irq_remapping.c static int intel_setup_irq_remapping(struct intel_iommu *iommu) · line 600
Quoted 7 lines
drivers/iommu/intel/irq_remapping.c +1
-0@@ -594,6 +594,7 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu)dmar_disable_qi (iommu);if (dmar_enable_qi (iommu)) { +irq_domain_free_fwnode (fn);pr_err ("Failed to enable queued invalidation\n" ); goto out_free_bitmap; }
The rewinding paths in intel_setup_irq_remapping() need more enhancement. How about below changes?
drivers/iommu/intel/irq_remapping.c +0 -1
@@ -569,9 +569,8 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu) fn, &intel_ir_domain_ops, iommu);if (!iommu->ir_domain) { -irq_domain_free_fwnode (fn); pr_err("IR%d: failed to allocate irqdomain\n",
iommu->seq_id);
- goto out_free_bitmap;
+ goto out_free_fwnode;
}
iommu->ir_msi_domain =
arch_create_remap_msi_irq_domain(iommu->ir_domain,@@ -595,7 +594,7 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu)
if (dmar_enable_qi(iommu)) {
pr_err("Failed to enable queued invalidation\n");
- goto out_free_bitmap;
+ goto out_free_ir_domain;
}
}@@ -619,6 +618,14 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu)
return 0;
+out_free_ir_domain:
+ if (iommu->ir_msi_domain)
+ irq_domain_remove(iommu->ir_msi_domain);
+ irq_domain_remove(iommu->ir_domain);
+ iommu->ir_msi_domain = NULL;
+ iommu->ir_domain = NULL;
+out_free_fwnode:
+ irq_domain_free_fwnode(fn);
out_free_bitmap:
bitmap_free(bitmap); out_free_pages:
Best regards,
baolu
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Quoted 7 lines · Lu Baolu
Quoted 4 lines · Guoqing Jiang
Call
irq_domain_free_fwnodeto free 'fn' ifdmar_enable_qireturns failure, same as whenirq_domain_create_hierarchyreturns failure.Signed-off-by Guoqing Jiang <[email protected]>Add a "Fixes" tag if you think this change needs backport.
I guess the tag might be:
Since before that commit, irq_domain_free_fwnode is called unconditionally after create ir_domain, please correct me if my understanding is not correct.
Quoted 61 lines
Quoted 11 lines
---
drivers/iommu/intel/irq_remapping.c| 1 +1 file changed, 1 insertion(+)drivers/iommu/intel/irq_remapping.c +0
-0drivers/iommu/intel/irq_remapping.c static int intel_setup_irq_remapping(struct · line 600
Quoted 7 lines
drivers/iommu/intel/irq_remapping.c +1
-0@@ -594,6 +594,7 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu) dmar_disable_qi(iommu); if (dmar_enable_qi(iommu)) { + irq_domain_free_fwnode(fn); pr_err("Failed to enable queued invalidation\n"); goto out_free_bitmap; }The rewinding paths in
intel_setup_irq_remapping()need more enhancement. How about below changes?drivers/iommu/intel/irq_remapping.c +1
-2@@ -569,9 +569,8 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu) fn, &intel_ir_domain_ops, iommu); if (!iommu->ir_domain) { - irq_domain_free_fwnode(fn); pr_err("IR%d: failed to allocate irqdomain\n", iommu->seq_id); - goto out_free_bitmap; + goto out_free_fwnode; }
iommu->ir_msi_domain= arch_create_remap_msi_irq_domain(iommu->ir_domain,drivers/iommu/intel/irq_remapping.c +9
-1@@ -595,7 +594,7 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu) if (dmar_enable_qi(iommu)) { pr_err("Failed to enable queued invalidation\n"); - goto out_free_bitmap; + goto out_free_ir_domain; } }@@ -619,6 +618,14 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu) return 0; +out_free_ir_domain: + if (iommu->ir_msi_domain) + irq_domain_remove(iommu->ir_msi_domain); + irq_domain_remove(iommu->ir_domain); + iommu->ir_msi_domain = NULL; + iommu->ir_domain = NULL; +out_free_fwnode: + irq_domain_free_fwnode(fn);
Quoted 3 lines
out_free_bitmap:
bitmap_free(bitmap);
out_free_pages:
Thanks for the suggestion, will send new version with your suggested-by.
Thanks,
Guoqing
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu