| Message ID | 20220530064807.34534-1-linmq006@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains | expand |
Hi Miaoqian, >of_get_child_by_name() returns a node pointer with refcount >incremented, we should use of_node_put() on it when not need anymore. >Add missing of_node_put() to avoid refcount leak. > >Fixes: 814cceebba9b ("PCI: mediatek-gen3: Add INTx support") >Signed-off-by: Miaoqian Lin <linmq006@gmail.com> >--- >changes in v2: >- move of_node_put(intc_node) right after irq_domain_add_linear to cover >normal path and error paths. > >v1 link: https://lore.kernel.org/all/20220526110246.53502-1-linmq006@gmail.com/ >--- > drivers/pci/controller/pcie-mediatek-gen3.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c >index 3e8d70bfabc6..19a01daad304 100644 >--- a/drivers/pci/controller/pcie-mediatek-gen3.c >+++ b/drivers/pci/controller/pcie-mediatek-gen3.c >@@ -598,6 +598,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie) > > pcie->intx_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX, > &intx_domain_ops, pcie); >+ of_node_put(intc_node); > if (!pcie->intx_domain) { > dev_err(dev, "failed to create INTx IRQ domain\n"); > return -ENODEV; > > Thanks for this v2 patch. I think we can do similar things like rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c. When I read the error path, I will go back and see if everything is released, so I think it is clearer to put of_node_put() in the error path and before return 0. How do you think? i.g., static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie) { struct device *dev = pcie->dev; struct device_node *intc_node, *node = dev->of_node; int ret; raw_spin_lock_init(&pcie->irq_lock); /* Setup INTx */ intc_node = of_get_child_by_name(node, "interrupt-controller"); if (!intc_node) { dev_err(dev, "missing interrupt-controller node\n"); return -ENODEV; } pcie->intx_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX, &intx_domain_ops, pcie); if (!pcie->intx_domain) { dev_err(dev, "failed to create INTx IRQ domain\n"); ret = -ENODEV; goto out_put_node; } /* Setup MSI */ mutex_init(&pcie->lock); pcie->msi_bottom_domain = irq_domain_add_linear(node, PCIE_MSI_IRQS_NUM, &mtk_msi_bottom_domain_ops, pcie); if (!pcie->msi_bottom_domain) { dev_err(dev, "failed to create MSI bottom domain\n"); ret = -ENODEV; goto err_msi_bottom_domain; } pcie->msi_domain = pci_msi_create_irq_domain(dev->fwnode, &mtk_msi_domain_info, pcie->msi_bottom_domain); if (!pcie->msi_domain) { dev_err(dev, "failed to create MSI domain\n"); ret = -ENODEV; goto err_msi_domain; } of_node_put(intc); return 0; err_msi_domain: irq_domain_remove(pcie->msi_bottom_domain); err_msi_bottom_domain: irq_domain_remove(pcie->intx_domain); out_put_node: of_node_put(intc); return ret; } thanks, Miles
diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c index 3e8d70bfabc6..19a01daad304 100644 --- a/drivers/pci/controller/pcie-mediatek-gen3.c +++ b/drivers/pci/controller/pcie-mediatek-gen3.c @@ -598,6 +598,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie) pcie->intx_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX, &intx_domain_ops, pcie); + of_node_put(intc_node); if (!pcie->intx_domain) { dev_err(dev, "failed to create INTx IRQ domain\n"); return -ENODEV;
of_get_child_by_name() returns a node pointer with refcount incremented, we should use of_node_put() on it when not need anymore. Add missing of_node_put() to avoid refcount leak. Fixes: 814cceebba9b ("PCI: mediatek-gen3: Add INTx support") Signed-off-by: Miaoqian Lin <linmq006@gmail.com> --- changes in v2: - move of_node_put(intc_node) right after irq_domain_add_linear to cover normal path and error paths. v1 link: https://lore.kernel.org/all/20220526110246.53502-1-linmq006@gmail.com/ --- drivers/pci/controller/pcie-mediatek-gen3.c | 1 + 1 file changed, 1 insertion(+)