| Message ID | 20190724081609.9724-1-nishkadg.linux@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
| Series | [v2] dma: qcom: hidma_mgmt: Add of_node_put() before goto | expand |
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>
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> >
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;
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(-)