| Message ID | 20220704065501.275677-1-windhl@126.com |
|---|---|
| State | New |
| Headers | show |
| Series | pci: controller: brcmstb: Move of_node_put() out of 'if' in brcm_pcie_probe | expand |
On Mon, Jul 04, 2022 at 02:55:01PM +0800, Liang He wrote: > Commit 3a87cb8f6a ("Fix refcount leak in brcm_pcie_probe()") adds > of_node_put() for of_parse_phandle() in fail path but not adds it > correctly in normal path. We should move the second of_node_put() > out of the 'if(pci_msi_enabled() && msi_np == pcie->np)'. > > Fixes: 3a87cb8f6a ("Fix refcount leak in brcm_pcie_probe()") > Co-authored-by: Miaoqian Lin <linmq006@gmail.com> > Signed-off-by: Liang He <windhl@126.com> > --- > Patched file has been compiled test in next branch. > > drivers/pci/controller/pcie-brcmstb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index 48a7148376d4..80e19d053e9f 100755 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -1413,8 +1413,8 @@ static int brcm_pcie_probe(struct platform_device *pdev) > of_node_put(msi_np); > goto fail; > } > - of_node_put(msi_np); > } > + of_node_put(msi_np); Can we just move the of_parse_phandle() and related checking into brcm_pcie_enable_msi()? It can return success without doing anything if !pci_msi_enabled() or msi_np != pcie_np. If you don't want to do that, please just send a revised version of 3a87cb8f6a72 ("PCI: brcmstb: Fix refcount leak in brcm_pcie_probe()"). That's not upstream yet, and I don't want to clutter the git history with a fix of a fix. > bridge->ops = pcie->type == BCM7425 ? &brcm_pcie_ops32 : &brcm_pcie_ops; > bridge->sysdata = pcie; > -- > 2.25.1 >
At 2022-07-15 00:47:26, "Bjorn Helgaas" <helgaas@kernel.org> wrote: >On Mon, Jul 04, 2022 at 02:55:01PM +0800, Liang He wrote: >> Commit 3a87cb8f6a ("Fix refcount leak in brcm_pcie_probe()") adds >> of_node_put() for of_parse_phandle() in fail path but not adds it >> correctly in normal path. We should move the second of_node_put() >> out of the 'if(pci_msi_enabled() && msi_np == pcie->np)'. >> >> Fixes: 3a87cb8f6a ("Fix refcount leak in brcm_pcie_probe()") >> Co-authored-by: Miaoqian Lin <linmq006@gmail.com> >> Signed-off-by: Liang He <windhl@126.com> >> --- >> Patched file has been compiled test in next branch. >> >> drivers/pci/controller/pcie-brcmstb.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c >> index 48a7148376d4..80e19d053e9f 100755 >> --- a/drivers/pci/controller/pcie-brcmstb.c >> +++ b/drivers/pci/controller/pcie-brcmstb.c >> @@ -1413,8 +1413,8 @@ static int brcm_pcie_probe(struct platform_device *pdev) >> of_node_put(msi_np); >> goto fail; >> } >> - of_node_put(msi_np); >> } >> + of_node_put(msi_np); > >Can we just move the of_parse_phandle() and related checking into >brcm_pcie_enable_msi()? It can return success without doing anything >if !pci_msi_enabled() or msi_np != pcie_np. > >If you don't want to do that, please just send a revised version of >3a87cb8f6a72 ("PCI: brcmstb: Fix refcount leak in brcm_pcie_probe()"). >That's not upstream yet, and I don't want to clutter the git history >with a fix of a fix. > Thanks, Bjorn, We will make a better version or a revised version soon. Liang >> bridge->ops = pcie->type == BCM7425 ? &brcm_pcie_ops32 : &brcm_pcie_ops; >> bridge->sysdata = pcie; >> -- >> 2.25.1 >>
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index 48a7148376d4..80e19d053e9f 100755 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -1413,8 +1413,8 @@ static int brcm_pcie_probe(struct platform_device *pdev) of_node_put(msi_np); goto fail; } - of_node_put(msi_np); } + of_node_put(msi_np); bridge->ops = pcie->type == BCM7425 ? &brcm_pcie_ops32 : &brcm_pcie_ops; bridge->sysdata = pcie;
Commit 3a87cb8f6a ("Fix refcount leak in brcm_pcie_probe()") adds of_node_put() for of_parse_phandle() in fail path but not adds it correctly in normal path. We should move the second of_node_put() out of the 'if(pci_msi_enabled() && msi_np == pcie->np)'. Fixes: 3a87cb8f6a ("Fix refcount leak in brcm_pcie_probe()") Co-authored-by: Miaoqian Lin <linmq006@gmail.com> Signed-off-by: Liang He <windhl@126.com> --- Patched file has been compiled test in next branch. drivers/pci/controller/pcie-brcmstb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)