On Tue, Jul 21, 2020 at 02:26:09PM -0600, Jon Derrick wrote:
> Change 711419e504eb ("irqdomain: Add the missing assignment of
> domain->fwnode for named fwnode") unintentionally caused a dangling
> pointer page fault issue on firmware nodes that were freed after IRQ
> domain allocation. Change e3beca48a45b fixed that dangling pointer issue
> by only freeing the firmware node after an IRQ domain allocation
> failure. That fix no longer frees the firmware node immediately, but
> leaves the firmware node allocated after the domain is removed.
>
> We need to keep the firmware node through irq_domain_remove, but should
> free it afterwards. This patch saves the handle and adds the freeing of
> firmware node after domain removal where appropriate.
>
> Fixes: e3beca48a45b ("irqdomain/treewide: Keep firmware node unconditionally
> allocated")
> Reviewed-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> Signed-off-by: Jon Derrick <jonathan.derr...@intel.com>
> Cc: sta...@vger.kernel.org
Acked-by: Bjorn Helgaas <bhelg...@google.com> # drivers/pci
> ---
> arch/mips/pci/pci-xtalk-bridge.c | 3 +++
> arch/x86/kernel/apic/io_apic.c | 5 +++++
> drivers/iommu/intel/irq_remapping.c | 8 ++++++++
> drivers/mfd/ioc3.c | 6 ++++++
> drivers/pci/controller/vmd.c | 3 +++
> 5 files changed, 25 insertions(+)
>
> diff --git a/arch/mips/pci/pci-xtalk-bridge.c
> b/arch/mips/pci/pci-xtalk-bridge.c
> index 5958217..9b3cc77 100644
> --- a/arch/mips/pci/pci-xtalk-bridge.c
> +++ b/arch/mips/pci/pci-xtalk-bridge.c
> @@ -728,6 +728,7 @@ static int bridge_probe(struct platform_device *pdev)
> pci_free_resource_list(&host->windows);
> err_remove_domain:
> irq_domain_remove(domain);
> + irq_domain_free_fwnode(fn);
> return err;
> }
>
> @@ -735,8 +736,10 @@ static int bridge_remove(struct platform_device *pdev)
> {
> struct pci_bus *bus = platform_get_drvdata(pdev);
> struct bridge_controller *bc = BRIDGE_CONTROLLER(bus);
> + struct fwnode_handle *fn = bc->domain->fwnode;
>
> irq_domain_remove(bc->domain);
> + irq_domain_free_fwnode(fn);
> pci_lock_rescan_remove();
> pci_stop_root_bus(bus);
> pci_remove_root_bus(bus);
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 81ffcfb..21325a4a 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2335,8 +2335,13 @@ static int mp_irqdomain_create(int ioapic)
>
> static void ioapic_destroy_irqdomain(int idx)
> {
> + struct ioapic_domain_cfg *cfg = &ioapics[idx].irqdomain_cfg;
> + struct fwnode_handle *fn = ioapics[idx].irqdomain->fwnode;
> +
> if (ioapics[idx].irqdomain) {
> irq_domain_remove(ioapics[idx].irqdomain);
> + if (!cfg->dev)
> + irq_domain_free_fwnode(fn);
> ioapics[idx].irqdomain = NULL;
> }
> }
> diff --git a/drivers/iommu/intel/irq_remapping.c
> b/drivers/iommu/intel/irq_remapping.c
> index 9564d23..aa096b3 100644
> --- a/drivers/iommu/intel/irq_remapping.c
> +++ b/drivers/iommu/intel/irq_remapping.c
> @@ -628,13 +628,21 @@ static int intel_setup_irq_remapping(struct intel_iommu
> *iommu)
>
> static void intel_teardown_irq_remapping(struct intel_iommu *iommu)
> {
> + struct fwnode_handle *fn;
> +
> if (iommu && iommu->ir_table) {
> if (iommu->ir_msi_domain) {
> + fn = iommu->ir_msi_domain->fwnode;
> +
> irq_domain_remove(iommu->ir_msi_domain);
> + irq_domain_free_fwnode(fn);
> iommu->ir_msi_domain = NULL;
> }
> if (iommu->ir_domain) {
> + fn = iommu->ir_domain->fwnode;
> +
> irq_domain_remove(iommu->ir_domain);
> + irq_domain_free_fwnode(fn);
> iommu->ir_domain = NULL;
> }
> free_pages((unsigned long)iommu->ir_table->base,
> diff --git a/drivers/mfd/ioc3.c b/drivers/mfd/ioc3.c
> index 74cee7c..d939ccc 100644
> --- a/drivers/mfd/ioc3.c
> +++ b/drivers/mfd/ioc3.c
> @@ -616,7 +616,10 @@ static int ioc3_mfd_probe(struct pci_dev *pdev,
> /* Remove all already added MFD devices */
> mfd_remove_devices(&ipd->pdev->dev);
> if (ipd->domain) {
> + struct fwnode_handle *fn = ipd->domain->fwnode;
> +
> irq_domain_remove(ipd->domain);
> + irq_domain_free_fwnode(fn);
> free_irq(ipd->domain_irq, (void *)ipd);
> }
> pci_iounmap(pdev, regs);
> @@ -643,7 +646,10 @@ static void ioc3_mfd_remove(struct pci_dev *pdev)
> /* Release resources */
> mfd_remove_devices(&ipd->pdev->dev);
> if (ipd->domain) {
> + struct fwnode_handle *fn = ipd->domain->fwnode;
> +
> irq_domain_remove(ipd->domain);
> + irq_domain_free_fwnode(fn);
> free_irq(ipd->domain_irq, (void *)ipd);
> }
> pci_iounmap(pdev, ipd->regs);
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index f078114..91eb769 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -560,6 +560,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd,
> unsigned long features)
> if (!vmd->bus) {
> pci_free_resource_list(&resources);
> irq_domain_remove(vmd->irq_domain);
> + irq_domain_free_fwnode(fn);
> return -ENODEV;
> }
>
> @@ -673,6 +674,7 @@ static void vmd_cleanup_srcu(struct vmd_dev *vmd)
> static void vmd_remove(struct pci_dev *dev)
> {
> struct vmd_dev *vmd = pci_get_drvdata(dev);
> + struct fwnode_handle *fn = vmd->irq_domain->fwnode;
>
> sysfs_remove_link(&vmd->dev->dev.kobj, "domain");
> pci_stop_root_bus(vmd->bus);
> @@ -680,6 +682,7 @@ static void vmd_remove(struct pci_dev *dev)
> vmd_cleanup_srcu(vmd);
> vmd_detach_resources(vmd);
> irq_domain_remove(vmd->irq_domain);
> + irq_domain_free_fwnode(fn);
> }
>
> #ifdef CONFIG_PM_SLEEP
> --
> 1.8.3.1
>
