* [PATCH] PCI: tegra: Add of_node_put() before return to fix reference leak
@ 2019-07-24 8:24 Nishka Dasgupta
2019-08-12 11:05 ` Lorenzo Pieralisi
0 siblings, 1 reply; 3+ messages in thread
From: Nishka Dasgupta @ 2019-07-24 8:24 UTC (permalink / raw)
To: thierry.reding, lorenzo.pieralisi, bhelgaas, jonathanh,
linux-tegra, linux-pci
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node() puts the previous node, but
in the case of a return from the middle of the loop, there is no put,
thus causing a reference leak.
Hence store these mid-loop return values in variable err and add a new
label err_node_put which puts the previous node and returns err. Change
six mid-loop return statements to point to this new label instead.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
Changes in v2:
- Edit subject line to better reflect changes and match other patches on
this driver.
- Edit commit message for readability and accuracy.
drivers/pci/controller/pci-tegra.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 9a917b2456f6..673a1725ef38 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2237,14 +2237,15 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
err = of_pci_get_devfn(port);
if (err < 0) {
dev_err(dev, "failed to parse address: %d\n", err);
- return err;
+ goto err_node_put;
}
index = PCI_SLOT(err);
if (index < 1 || index > soc->num_ports) {
dev_err(dev, "invalid port number: %d\n", index);
- return -EINVAL;
+ err = -EINVAL;
+ goto err_node_put;
}
index--;
@@ -2253,12 +2254,13 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
if (err < 0) {
dev_err(dev, "failed to parse # of lanes: %d\n",
err);
- return err;
+ goto err_node_put;
}
if (value > 16) {
dev_err(dev, "invalid # of lanes: %u\n", value);
- return -EINVAL;
+ err = -EINVAL;
+ goto err_node_put;
}
lanes |= value << (index << 3);
@@ -2272,13 +2274,15 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
lane += value;
rp = devm_kzalloc(dev, sizeof(*rp), GFP_KERNEL);
- if (!rp)
- return -ENOMEM;
+ if (!rp) {
+ err = -ENOMEM;
+ goto err_node_put;
+ }
err = of_address_to_resource(port, 0, &rp->regs);
if (err < 0) {
dev_err(dev, "failed to parse address: %d\n", err);
- return err;
+ goto err_node_put;
}
INIT_LIST_HEAD(&rp->list);
@@ -2330,6 +2334,10 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
return err;
return 0;
+
+err_node_put:
+ of_node_put(port);
+ return err;
}
/*
--
2.19.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] PCI: tegra: Add of_node_put() before return to fix reference leak
2019-07-24 8:24 [PATCH] PCI: tegra: Add of_node_put() before return to fix reference leak Nishka Dasgupta
@ 2019-08-12 11:05 ` Lorenzo Pieralisi
2019-08-13 4:38 ` Nishka Dasgupta
0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Pieralisi @ 2019-08-12 11:05 UTC (permalink / raw)
To: Nishka Dasgupta
Cc: thierry.reding, bhelgaas, jonathanh, linux-tegra, linux-pci
On Wed, Jul 24, 2019 at 01:54:12PM +0530, Nishka Dasgupta wrote:
> Each iteration of for_each_child_of_node() puts the previous node, but
> in the case of a return from the middle of the loop, there is no put,
> thus causing a reference leak.
>
> Hence store these mid-loop return values in variable err and add a new
> label err_node_put which puts the previous node and returns err. Change
> six mid-loop return statements to point to this new label instead.
>
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> Changes in v2:
If you are sending a v2 make it explicit in the patch $SUBJECT and send
the patch --in-reply-to=<message-ID-previous-version> otherwise *I* have
to fish out of mailing lists previous patches to understand what you are
doing.
> - Edit subject line to better reflect changes and match other patches on
> this driver.
> - Edit commit message for readability and accuracy.
>
> drivers/pci/controller/pci-tegra.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
Applied to pci/tegra, thanks.
Lorenzo
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 9a917b2456f6..673a1725ef38 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2237,14 +2237,15 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
> err = of_pci_get_devfn(port);
> if (err < 0) {
> dev_err(dev, "failed to parse address: %d\n", err);
> - return err;
> + goto err_node_put;
> }
>
> index = PCI_SLOT(err);
>
> if (index < 1 || index > soc->num_ports) {
> dev_err(dev, "invalid port number: %d\n", index);
> - return -EINVAL;
> + err = -EINVAL;
> + goto err_node_put;
> }
>
> index--;
> @@ -2253,12 +2254,13 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
> if (err < 0) {
> dev_err(dev, "failed to parse # of lanes: %d\n",
> err);
> - return err;
> + goto err_node_put;
> }
>
> if (value > 16) {
> dev_err(dev, "invalid # of lanes: %u\n", value);
> - return -EINVAL;
> + err = -EINVAL;
> + goto err_node_put;
> }
>
> lanes |= value << (index << 3);
> @@ -2272,13 +2274,15 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
> lane += value;
>
> rp = devm_kzalloc(dev, sizeof(*rp), GFP_KERNEL);
> - if (!rp)
> - return -ENOMEM;
> + if (!rp) {
> + err = -ENOMEM;
> + goto err_node_put;
> + }
>
> err = of_address_to_resource(port, 0, &rp->regs);
> if (err < 0) {
> dev_err(dev, "failed to parse address: %d\n", err);
> - return err;
> + goto err_node_put;
> }
>
> INIT_LIST_HEAD(&rp->list);
> @@ -2330,6 +2334,10 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
> return err;
>
> return 0;
> +
> +err_node_put:
> + of_node_put(port);
> + return err;
> }
>
> /*
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] PCI: tegra: Add of_node_put() before return to fix reference leak
2019-08-12 11:05 ` Lorenzo Pieralisi
@ 2019-08-13 4:38 ` Nishka Dasgupta
0 siblings, 0 replies; 3+ messages in thread
From: Nishka Dasgupta @ 2019-08-13 4:38 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: thierry.reding, bhelgaas, jonathanh, linux-tegra, linux-pci
On 12/08/19 4:35 PM, Lorenzo Pieralisi wrote:
> On Wed, Jul 24, 2019 at 01:54:12PM +0530, Nishka Dasgupta wrote:
>> Each iteration of for_each_child_of_node() puts the previous node, but
>> in the case of a return from the middle of the loop, there is no put,
>> thus causing a reference leak.
>>
>> Hence store these mid-loop return values in variable err and add a new
>> label err_node_put which puts the previous node and returns err. Change
>> six mid-loop return statements to point to this new label instead.
>>
>> Issue found with Coccinelle.
>>
>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>> ---
>> Changes in v2:
>
> If you are sending a v2 make it explicit in the patch $SUBJECT and send
> the patch --in-reply-to=<message-ID-previous-version> otherwise *I* have
> to fish out of mailing lists previous patches to understand what you are
> doing.
I am very sorry; it won't happen again.
Thanking you,
Nishka
>
>> - Edit subject line to better reflect changes and match other patches on
>> this driver.
>> - Edit commit message for readability and accuracy.
>>
>> drivers/pci/controller/pci-tegra.c | 22 +++++++++++++++-------
>> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> Applied to pci/tegra, thanks.
>
> Lorenzo
>
>> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
>> index 9a917b2456f6..673a1725ef38 100644
>> --- a/drivers/pci/controller/pci-tegra.c
>> +++ b/drivers/pci/controller/pci-tegra.c
>> @@ -2237,14 +2237,15 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>> err = of_pci_get_devfn(port);
>> if (err < 0) {
>> dev_err(dev, "failed to parse address: %d\n", err);
>> - return err;
>> + goto err_node_put;
>> }
>>
>> index = PCI_SLOT(err);
>>
>> if (index < 1 || index > soc->num_ports) {
>> dev_err(dev, "invalid port number: %d\n", index);
>> - return -EINVAL;
>> + err = -EINVAL;
>> + goto err_node_put;
>> }
>>
>> index--;
>> @@ -2253,12 +2254,13 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>> if (err < 0) {
>> dev_err(dev, "failed to parse # of lanes: %d\n",
>> err);
>> - return err;
>> + goto err_node_put;
>> }
>>
>> if (value > 16) {
>> dev_err(dev, "invalid # of lanes: %u\n", value);
>> - return -EINVAL;
>> + err = -EINVAL;
>> + goto err_node_put;
>> }
>>
>> lanes |= value << (index << 3);
>> @@ -2272,13 +2274,15 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>> lane += value;
>>
>> rp = devm_kzalloc(dev, sizeof(*rp), GFP_KERNEL);
>> - if (!rp)
>> - return -ENOMEM;
>> + if (!rp) {
>> + err = -ENOMEM;
>> + goto err_node_put;
>> + }
>>
>> err = of_address_to_resource(port, 0, &rp->regs);
>> if (err < 0) {
>> dev_err(dev, "failed to parse address: %d\n", err);
>> - return err;
>> + goto err_node_put;
>> }
>>
>> INIT_LIST_HEAD(&rp->list);
>> @@ -2330,6 +2334,10 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>> return err;
>>
>> return 0;
>> +
>> +err_node_put:
>> + of_node_put(port);
>> + return err;
>> }
>>
>> /*
>> --
>> 2.19.1
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-13 4:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-24 8:24 [PATCH] PCI: tegra: Add of_node_put() before return to fix reference leak Nishka Dasgupta
2019-08-12 11:05 ` Lorenzo Pieralisi
2019-08-13 4:38 ` Nishka Dasgupta
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).