lkml.org 
[lkml]   [2022]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
Date
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

\
 
 \ /
  Last update: 2022-05-31 20:16    [W:0.050 / U:0.356 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site