dmaengine Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] dma: qcom: hidma_mgmt: Add of_node_put() before goto
@ 2019-07-24  8:16 Nishka Dasgupta
  2019-07-24 17:25 ` Sinan Kaya
  0 siblings, 1 reply; 3+ messages in thread
From: Nishka Dasgupta @ 2019-07-24  8:16 UTC (permalink / raw)
  To: okaya, agross, vkoul, dan.j.williams, linux-arm-kernel,
	linux-arm-msm, dmaengine
  Cc: Nishka Dasgupta

Each iteration of for_each_available_child_of_node puts the previous
node, but in the case of a goto from the middle of the loop, there is
no put, thus causing a memory leak. 
Hence add an of_node_put under the label that the gotos point to.
In order to avoid decrementing an already-decremented refcount, copy the
original contents of the label (including the return statement) to just
above the label, so that the code under the label is executed only when
a goto exit from the loop occurs.
Additionally, remove an unnecessary get/put pair from the loop, as the
loop itself already keeps track of refcount.
Issue found with Coccinelle.

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
Changes in v2:
- Move the put under the label instead of above each individual goto.
-Remove the get/put pair.

 drivers/dma/qcom/hidma_mgmt.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
index 3022d66e7a33..43f806c8b51e 100644
--- a/drivers/dma/qcom/hidma_mgmt.c
+++ b/drivers/dma/qcom/hidma_mgmt.c
@@ -388,7 +388,6 @@ static int __init hidma_mgmt_of_populate_channels(struct device_node *np)
 			ret = PTR_ERR(new_pdev);
 			goto out;
 		}
-		of_node_get(child);
 		new_pdev->dev.of_node = child;
 		of_dma_configure(&new_pdev->dev, child, true);
 		/*
@@ -396,9 +395,14 @@ static int __init hidma_mgmt_of_populate_channels(struct device_node *np)
 		 * platforms with or without MSI support.
 		 */
 		of_msi_configure(&new_pdev->dev, child);
-		of_node_put(child);
 	}
+
+	kfree(res);
+
+	return ret;
+
 out:
+	of_node_put(child);
 	kfree(res);
 
 	return ret;
-- 
2.19.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] dma: qcom: hidma_mgmt: Add of_node_put() before goto
  2019-07-24  8:16 [PATCH v2] dma: qcom: hidma_mgmt: Add of_node_put() before goto Nishka Dasgupta
@ 2019-07-24 17:25 ` Sinan Kaya
  2019-08-08 12:27   ` Vinod Koul
  0 siblings, 1 reply; 3+ messages in thread
From: Sinan Kaya @ 2019-07-24 17:25 UTC (permalink / raw)
  To: Nishka Dasgupta, agross, vkoul, dan.j.williams, linux-arm-kernel,
	linux-arm-msm, dmaengine

On 7/24/2019 4:16 AM, Nishka Dasgupta wrote:
> Each iteration of for_each_available_child_of_node puts the previous
> node, but in the case of a goto from the middle of the loop, there is
> no put, thus causing a memory leak. 
> Hence add an of_node_put under the label that the gotos point to.
> In order to avoid decrementing an already-decremented refcount, copy the
> original contents of the label (including the return statement) to just
> above the label, so that the code under the label is executed only when
> a goto exit from the loop occurs.
> Additionally, remove an unnecessary get/put pair from the loop, as the
> loop itself already keeps track of refcount.
> Issue found with Coccinelle.
> 
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>

nit: please post v3 with dmaengine:qcom:hidma_mgmt:....

Vinod doesn't like commit subjectss in this directory to have dma name
on it. You can keep my acked-by.

Acked-by: Sinan Kaya <okaya@kernel.org>



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] dma: qcom: hidma_mgmt: Add of_node_put() before goto
  2019-07-24 17:25 ` Sinan Kaya
@ 2019-08-08 12:27   ` Vinod Koul
  0 siblings, 0 replies; 3+ messages in thread
From: Vinod Koul @ 2019-08-08 12:27 UTC (permalink / raw)
  To: Sinan Kaya
  Cc: Nishka Dasgupta, agross, dan.j.williams, linux-arm-kernel,
	linux-arm-msm, dmaengine

On 24-07-19, 13:25, Sinan Kaya wrote:
> On 7/24/2019 4:16 AM, Nishka Dasgupta wrote:
> > Each iteration of for_each_available_child_of_node puts the previous
> > node, but in the case of a goto from the middle of the loop, there is
> > no put, thus causing a memory leak. 
> > Hence add an of_node_put under the label that the gotos point to.
> > In order to avoid decrementing an already-decremented refcount, copy the
> > original contents of the label (including the return statement) to just
> > above the label, so that the code under the label is executed only when
> > a goto exit from the loop occurs.
> > Additionally, remove an unnecessary get/put pair from the loop, as the
> > loop itself already keeps track of refcount.
> > Issue found with Coccinelle.
> > 
> > Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> 
> nit: please post v3 with dmaengine:qcom:hidma_mgmt:....
> 
> Vinod doesn't like commit subjectss in this directory to have dma name
> on it. You can keep my acked-by.

That's right but I am okay to hand edit while applying for drive by
contributors :) so applied with you ack

> Acked-by: Sinan Kaya <okaya@kernel.org>
> 

-- 
~Vinod

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-24  8:16 [PATCH v2] dma: qcom: hidma_mgmt: Add of_node_put() before goto Nishka Dasgupta
2019-07-24 17:25 ` Sinan Kaya
2019-08-08 12:27   ` Vinod Koul

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org dmaengine@archiver.kernel.org
	public-inbox-index dmaengine


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/ public-inbox