Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] usb: dwc3: st: Add of_node_put() before return in probe function
@ 2019-08-19  7:24 Nishka Dasgupta
  2019-08-19  7:24 ` [PATCH 2/2] usb: dwc3: st: Add of_dev_put() " Nishka Dasgupta
  2019-08-26  7:46 ` [PATCH 1/2] usb: dwc3: st: Add of_node_put() before return " Patrice CHOTARD
  0 siblings, 2 replies; 4+ messages in thread
From: Nishka Dasgupta @ 2019-08-19  7:24 UTC (permalink / raw)
  To: patrice.chotard, balbi, gregkh, linux-arm-kernel, linux-usb
  Cc: Nishka Dasgupta

The local variable child in the function st_dwc3_probe takes the return
value of of_get_child_by_name, which gets a node and does not put it. If
the function returns without releasing child, this could cause a memory
error. Hence put child as soon as there is no more use for it. Also
create a new label, err_node_put, just before label undo_softreset; so
that err_node_put puts child. In between initialisation of child and its
first put, modify all statements that go to undo_softreset to now go to
err_node_put instead, from where they can fall through to
undo_softreset.
Issue found with Coccinelle.

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
 drivers/usb/dwc3/dwc3-st.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index 16081383c401..6f52c749cd58 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -255,24 +255,25 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	if (!child) {
 		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
 		ret = -ENODEV;
-		goto undo_softreset;
+		goto err_node_put;
 	}
 
 	/* Allocate and initialize the core */
 	ret = of_platform_populate(node, NULL, NULL, dev);
 	if (ret) {
 		dev_err(dev, "failed to add dwc3 core\n");
-		goto undo_softreset;
+		goto err_node_put;
 	}
 
 	child_pdev = of_find_device_by_node(child);
 	if (!child_pdev) {
 		dev_err(dev, "failed to find dwc3 core device\n");
 		ret = -ENODEV;
-		goto undo_softreset;
+		goto err_node_put;
 	}
 
 	dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
+	of_node_put(child);
 
 	/*
 	 * Configure the USB port as device or host according to the static
@@ -292,6 +293,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dwc3_data);
 	return 0;
 
+err_node_put:
+	of_node_put(child);
 undo_softreset:
 	reset_control_assert(dwc3_data->rstc_rst);
 undo_powerdown:
-- 
2.19.1


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

* [PATCH 2/2] usb: dwc3: st: Add of_dev_put() in probe function
  2019-08-19  7:24 [PATCH 1/2] usb: dwc3: st: Add of_node_put() before return in probe function Nishka Dasgupta
@ 2019-08-19  7:24 ` " Nishka Dasgupta
  2019-08-26  7:47   ` Patrice CHOTARD
  2019-08-26  7:46 ` [PATCH 1/2] usb: dwc3: st: Add of_node_put() before return " Patrice CHOTARD
  1 sibling, 1 reply; 4+ messages in thread
From: Nishka Dasgupta @ 2019-08-19  7:24 UTC (permalink / raw)
  To: patrice.chotard, balbi, gregkh, linux-arm-kernel, linux-usb
  Cc: Nishka Dasgupta

In function st_dwc3_probe, variable child_pdev takes the value returned
by of_find_device_by_node, which gets a device pointer but does not put
it. If child_pdev is not put before the probe function returns, it may
cause a reference leak. Hence put child_pdev after its last usage.
Issue found with Coccinelle.

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
 drivers/usb/dwc3/dwc3-st.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index 6f52c749cd58..c682420f25ca 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -274,6 +274,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
 
 	dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
 	of_node_put(child);
+	of_dev_put(child_pdev);
 
 	/*
 	 * Configure the USB port as device or host according to the static
-- 
2.19.1


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

* Re: [PATCH 1/2] usb: dwc3: st: Add of_node_put() before return in probe function
  2019-08-19  7:24 [PATCH 1/2] usb: dwc3: st: Add of_node_put() before return in probe function Nishka Dasgupta
  2019-08-19  7:24 ` [PATCH 2/2] usb: dwc3: st: Add of_dev_put() " Nishka Dasgupta
@ 2019-08-26  7:46 ` " Patrice CHOTARD
  1 sibling, 0 replies; 4+ messages in thread
From: Patrice CHOTARD @ 2019-08-26  7:46 UTC (permalink / raw)
  To: Nishka Dasgupta, balbi, gregkh, linux-arm-kernel, linux-usb

Hi Nishka

On 8/19/19 9:24 AM, Nishka Dasgupta wrote:
> The local variable child in the function st_dwc3_probe takes the return
> value of of_get_child_by_name, which gets a node and does not put it. If
> the function returns without releasing child, this could cause a memory
> error. Hence put child as soon as there is no more use for it. Also
> create a new label, err_node_put, just before label undo_softreset; so
> that err_node_put puts child. In between initialisation of child and its
> first put, modify all statements that go to undo_softreset to now go to
> err_node_put instead, from where they can fall through to
> undo_softreset.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
>  drivers/usb/dwc3/dwc3-st.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index 16081383c401..6f52c749cd58 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -255,24 +255,25 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	if (!child) {
>  		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
>  		ret = -ENODEV;
> -		goto undo_softreset;
> +		goto err_node_put;
>  	}
>  
>  	/* Allocate and initialize the core */
>  	ret = of_platform_populate(node, NULL, NULL, dev);
>  	if (ret) {
>  		dev_err(dev, "failed to add dwc3 core\n");
> -		goto undo_softreset;
> +		goto err_node_put;
>  	}
>  
>  	child_pdev = of_find_device_by_node(child);
>  	if (!child_pdev) {
>  		dev_err(dev, "failed to find dwc3 core device\n");
>  		ret = -ENODEV;
> -		goto undo_softreset;
> +		goto err_node_put;
>  	}
>  
>  	dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
> +	of_node_put(child);
>  
>  	/*
>  	 * Configure the USB port as device or host according to the static
> @@ -292,6 +293,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, dwc3_data);
>  	return 0;
>  
> +err_node_put:
> +	of_node_put(child);
>  undo_softreset:
>  	reset_control_assert(dwc3_data->rstc_rst);
>  undo_powerdown:


Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Thanks

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

* Re: [PATCH 2/2] usb: dwc3: st: Add of_dev_put() in probe function
  2019-08-19  7:24 ` [PATCH 2/2] usb: dwc3: st: Add of_dev_put() " Nishka Dasgupta
@ 2019-08-26  7:47   ` Patrice CHOTARD
  0 siblings, 0 replies; 4+ messages in thread
From: Patrice CHOTARD @ 2019-08-26  7:47 UTC (permalink / raw)
  To: Nishka Dasgupta, balbi, gregkh, linux-arm-kernel, linux-usb


On 8/19/19 9:24 AM, Nishka Dasgupta wrote:
> In function st_dwc3_probe, variable child_pdev takes the value returned
> by of_find_device_by_node, which gets a device pointer but does not put
> it. If child_pdev is not put before the probe function returns, it may
> cause a reference leak. Hence put child_pdev after its last usage.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
>  drivers/usb/dwc3/dwc3-st.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index 6f52c749cd58..c682420f25ca 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -274,6 +274,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  
>  	dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
>  	of_node_put(child);
> +	of_dev_put(child_pdev);
>  
>  	/*
>  	 * Configure the USB port as device or host according to the static

Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Thanks

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19  7:24 [PATCH 1/2] usb: dwc3: st: Add of_node_put() before return in probe function Nishka Dasgupta
2019-08-19  7:24 ` [PATCH 2/2] usb: dwc3: st: Add of_dev_put() " Nishka Dasgupta
2019-08-26  7:47   ` Patrice CHOTARD
2019-08-26  7:46 ` [PATCH 1/2] usb: dwc3: st: Add of_node_put() before return " Patrice CHOTARD

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/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 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


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