* [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains @ 2022-05-26 11:02 Miaoqian Lin 2022-05-26 18:44 ` Rob Herring 2022-05-27 8:45 ` Miles Chen 0 siblings, 2 replies; 8+ messages in thread From: Miaoqian Lin @ 2022-05-26 11:02 UTC (permalink / raw) To: Ryder Lee, Jianjun Wang, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Matthias Brugger, Marc Zyngier, linux-pci, linux-mediatek, linux-kernel, linux-arm-kernel Cc: linmq006 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> --- 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..da8e9db0abdf 100644 --- a/drivers/pci/controller/pcie-mediatek-gen3.c +++ b/drivers/pci/controller/pcie-mediatek-gen3.c @@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie) &intx_domain_ops, pcie); if (!pcie->intx_domain) { dev_err(dev, "failed to create INTx IRQ domain\n"); + of_node_put(intc_node); return -ENODEV; } -- 2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains 2022-05-26 11:02 [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains Miaoqian Lin @ 2022-05-26 18:44 ` Rob Herring 2022-05-27 8:45 ` Miles Chen 1 sibling, 0 replies; 8+ messages in thread From: Rob Herring @ 2022-05-26 18:44 UTC (permalink / raw) To: Miaoqian Lin Cc: Ryder Lee, Jianjun Wang, Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas, Matthias Brugger, Marc Zyngier, linux-pci, linux-mediatek, linux-kernel, linux-arm-kernel On Thu, May 26, 2022 at 03:02:46PM +0400, Miaoqian Lin wrote: > 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> > --- > drivers/pci/controller/pcie-mediatek-gen3.c | 1 + > 1 file changed, 1 insertion(+) Acked-by: Rob Herring <robh@kernel.org> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains 2022-05-26 11:02 [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains Miaoqian Lin 2022-05-26 18:44 ` Rob Herring @ 2022-05-27 8:45 ` Miles Chen 2022-05-28 9:19 ` Miaoqian Lin 1 sibling, 1 reply; 8+ messages in thread From: Miles Chen @ 2022-05-27 8:45 UTC (permalink / raw) To: linmq006 Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel, linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz, robh, ryder.lee Hi Miaoqian, >Signed-off-by: Miaoqian Lin <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..da8e9db0abdf 100644 >--- a/drivers/pci/controller/pcie-mediatek-gen3.c >+++ b/drivers/pci/controller/pcie-mediatek-gen3.c >@@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie) > &intx_domain_ops, pcie); > if (!pcie->intx_domain) { > dev_err(dev, "failed to create INTx IRQ domain\n"); >+ of_node_put(intc_node); > return -ENODEV; > } Thanks for doing this. I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like err_msi_domain and err_msi_bottom_domain and the normal path which also need of_node_put(intc_node). Maybe we can move the of_node_put(intc_node) to #54 below and cover all possible paths? cheers, Miles e.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); of_node_put(intc_node); if (!pcie->intx_domain) { dev_err(dev, "failed to create INTx IRQ domain\n"); return -ENODEV; } /* 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; } return 0; err_msi_domain: irq_domain_remove(pcie->msi_bottom_domain); err_msi_bottom_domain: irq_domain_remove(pcie->intx_domain); return ret; } > >-- >2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains 2022-05-27 8:45 ` Miles Chen @ 2022-05-28 9:19 ` Miaoqian Lin 2022-05-30 2:19 ` Miles Chen 0 siblings, 1 reply; 8+ messages in thread From: Miaoqian Lin @ 2022-05-28 9:19 UTC (permalink / raw) To: Miles Chen Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel, linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz, robh, ryder.lee Hi Miles, On 2022/5/27 16:45, Miles Chen wrote: > Hi Miaoqian, > >> Signed-off-by: Miaoqian Lin <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..da8e9db0abdf 100644 >> --- a/drivers/pci/controller/pcie-mediatek-gen3.c >> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c >> @@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie) >> &intx_domain_ops, pcie); >> if (!pcie->intx_domain) { >> dev_err(dev, "failed to create INTx IRQ domain\n"); >> + of_node_put(intc_node); >> return -ENODEV; >> } > Thanks for doing this. > > I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like > err_msi_domain and err_msi_bottom_domain and the normal path which also > need of_node_put(intc_node). Thanks for your reply, I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear(). __irq_domain_add() keeps &node->fwnode in the irq_domain structure. and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain. If the reference don't needed anymore after irq_domain_add_linear(), your suggestion looks fine, and I will submit v2. > Maybe we can move the of_node_put(intc_node) to #54 below and cover > all possible paths? > > > cheers, > Miles > > e.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); > of_node_put(intc_node); > if (!pcie->intx_domain) { > dev_err(dev, "failed to create INTx IRQ domain\n"); > return -ENODEV; > } > > /* 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; > } > > return 0; > > err_msi_domain: > irq_domain_remove(pcie->msi_bottom_domain); > err_msi_bottom_domain: > irq_domain_remove(pcie->intx_domain); > > return ret; > } >> -- >> 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains 2022-05-28 9:19 ` Miaoqian Lin @ 2022-05-30 2:19 ` Miles Chen 2022-05-30 6:54 ` Miaoqian Lin 0 siblings, 1 reply; 8+ messages in thread From: Miles Chen @ 2022-05-30 2:19 UTC (permalink / raw) To: linmq006 Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel, linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz, miles.chen, robh, ryder.lee Hi Miaoqian, >>> &intx_domain_ops, pcie); >>> if (!pcie->intx_domain) { >>> dev_err(dev, "failed to create INTx IRQ domain\n"); >>> + of_node_put(intc_node); >>> return -ENODEV; >>> } >> Thanks for doing this. >> >> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like >> err_msi_domain and err_msi_bottom_domain and the normal path which also >> need of_node_put(intc_node). > >Thanks for your reply, > >I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear(). > >__irq_domain_add() keeps &node->fwnode in the irq_domain structure. > >and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain. > >If the reference don't needed anymore after irq_domain_add_linear(), > >your suggestion looks fine, and I will submit v2. Thanks for your reply, I think we can do similar things like rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c Thanks, Miles _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains 2022-05-30 2:19 ` Miles Chen @ 2022-05-30 6:54 ` Miaoqian Lin 2022-05-30 7:35 ` Miles Chen 0 siblings, 1 reply; 8+ messages in thread From: Miaoqian Lin @ 2022-05-30 6:54 UTC (permalink / raw) To: Miles Chen Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel, linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz, robh, ryder.lee Hi, Miles On 2022/5/30 10:19, Miles Chen wrote: > Hi Miaoqian, > >>>> &intx_domain_ops, pcie); >>>> if (!pcie->intx_domain) { >>>> dev_err(dev, "failed to create INTx IRQ domain\n"); >>>> + of_node_put(intc_node); >>>> return -ENODEV; >>>> } >>> Thanks for doing this. >>> >>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like >>> err_msi_domain and err_msi_bottom_domain and the normal path which also >>> need of_node_put(intc_node). >> Thanks for your reply, >> >> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear(). >> >> __irq_domain_add() keeps &node->fwnode in the irq_domain structure. >> >> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain. >> >> If the reference don't needed anymore after irq_domain_add_linear(), >> >> your suggestion looks fine, and I will submit v2. > > Thanks for your reply, I think we can do similar things like > rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths. and calls of_node_put() before return 0 in normal path. I didn't see the same problem. > Thanks, > Miles _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains 2022-05-30 6:54 ` Miaoqian Lin @ 2022-05-30 7:35 ` Miles Chen 2022-05-31 14:01 ` Miaoqian Lin 0 siblings, 1 reply; 8+ messages in thread From: Miles Chen @ 2022-05-30 7:35 UTC (permalink / raw) To: linmq006 Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel, linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz, miles.chen, robh, ryder.lee Hi Miaoqian, >Hi, Miles > >On 2022/5/30 10:19, Miles Chen wrote: >> Hi Miaoqian, >> >>>>> &intx_domain_ops, pcie); >>>>> if (!pcie->intx_domain) { >>>>> dev_err(dev, "failed to create INTx IRQ domain\n"); >>>>> + of_node_put(intc_node); >>>>> return -ENODEV; >>>>> } >>>> Thanks for doing this. >>>> >>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like >>>> err_msi_domain and err_msi_bottom_domain and the normal path which also >>>> need of_node_put(intc_node). >>> Thanks for your reply, >>> >>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear(). >>> >>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure. >>> >>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain. >>> >>> If the reference don't needed anymore after irq_domain_add_linear(), >>> >>> your suggestion looks fine, and I will submit v2. >> >> Thanks for your reply, I think we can do similar things like >> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c > >I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths. > >and calls of_node_put() before return 0 in normal path. I didn't see the same problem. Sorry for the confusing. I meant that we can do the same thing - it calls of_node_put() by goto statement for error paths and calls of_node_put() before return 0 in normal path. :-) Thanks, Miles _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains 2022-05-30 7:35 ` Miles Chen @ 2022-05-31 14:01 ` Miaoqian Lin 0 siblings, 0 replies; 8+ messages in thread From: Miaoqian Lin @ 2022-05-31 14:01 UTC (permalink / raw) To: Miles Chen Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel, linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz, robh, ryder.lee Hi, Miles On 2022/5/30 15:35, Miles Chen wrote: > Hi Miaoqian, > >> Hi, Miles >> >> On 2022/5/30 10:19, Miles Chen wrote: >>> Hi Miaoqian, >>> >>>>>> &intx_domain_ops, pcie); >>>>>> if (!pcie->intx_domain) { >>>>>> dev_err(dev, "failed to create INTx IRQ domain\n"); >>>>>> + of_node_put(intc_node); >>>>>> return -ENODEV; >>>>>> } >>>>> Thanks for doing this. >>>>> >>>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like >>>>> err_msi_domain and err_msi_bottom_domain and the normal path which also >>>>> need of_node_put(intc_node). >>>> Thanks for your reply, >>>> >>>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear(). >>>> >>>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure. >>>> >>>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain. >>>> >>>> If the reference don't needed anymore after irq_domain_add_linear(), >>>> >>>> your suggestion looks fine, and I will submit v2. >>> Thanks for your reply, I think we can do similar things like >>> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c >> I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths. >> >> and calls of_node_put() before return 0 in normal path. I didn't see the same problem. > Sorry for the confusing. I meant that we can do the same thing - > it calls of_node_put() by goto statement for error paths > and calls of_node_put() before return 0 in normal path. :-) I'll sent a v2 for this: https://lore.kernel.org/all/20220530064807.34534-1-linmq006@gmail.com/ following your original suggestion. > Thanks, > Miles _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-05-31 14:03 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-26 11:02 [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains Miaoqian Lin 2022-05-26 18:44 ` Rob Herring 2022-05-27 8:45 ` Miles Chen 2022-05-28 9:19 ` Miaoqian Lin 2022-05-30 2:19 ` Miles Chen 2022-05-30 6:54 ` Miaoqian Lin 2022-05-30 7:35 ` Miles Chen 2022-05-31 14:01 ` Miaoqian Lin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).