[PATCH v2] cpufreq: imx6q: Fix the resource leak caused by incorrect error return

[PATCH v2] cpufreq: imx6q: Fix the resource leak caused by incorrect error return

[Jacky Bai]

Previous goto only handled the node reference, the opp table,
regulator & clk resource also need to be free before error return.

Fixes: ddb64c5db3c (cpufreq: imx6q: fix possible object reference leak).
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/cpufreq/imx6q-cpufreq.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 3e17560b1efe..1d4ecefaabc6 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -383,23 +383,22 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 		goto put_reg;
 	}
 
+	/* Because we have added the OPPs here, we must free them */
+	free_opp = true;
+
 	if (of_machine_is_compatible("fsl,imx6ul") ||
 	    of_machine_is_compatible("fsl,imx6ull")) {
 		ret = imx6ul_opp_check_speed_grading(cpu_dev);
 		if (ret) {
-			if (ret == -EPROBE_DEFER)
-				goto put_node;
-
-			dev_err(cpu_dev, "failed to read ocotp: %d\n",
-				ret);
-			goto put_node;
+			if (ret != -EPROBE_DEFER)
+				dev_err(cpu_dev, "failed to read ocotp: %d\n",
+					ret);
+			goto out_free_opp;
 		}
 	} else {
 		imx6q_opp_check_speed_grading(cpu_dev);
 	}
 
-	/* Because we have added the OPPs here, we must free them */
-	free_opp = true;
 	num = dev_pm_opp_get_opp_count(cpu_dev);
 	if (num < 0) {
 		ret = num;
-- 
2.21.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] cpufreq: imx6q: Fix the resource leak caused by incorrect error return

[Viresh Kumar]

On 05-05-19, 08:02, Jacky Bai wrote:
> Previous goto only handled the node reference, the opp table,
> regulator & clk resource also need to be free before error return.
> 
> Fixes: ddb64c5db3c (cpufreq: imx6q: fix possible object reference leak).

This should have been.

Fixes: ddb64c5db3cc ("cpufreq: imx6q: fix possible object reference leak")

Auto configure it with following in .gitconfig

[pretty]
	fixes = Fixes: %h (\"%s\")
	onelin = commit %h (\"%s\")

and then:

git log --pretty=fixes ddb64c5db3c

will generate it by itself.

> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  drivers/cpufreq/imx6q-cpufreq.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
> index 3e17560b1efe..1d4ecefaabc6 100644
> --- a/drivers/cpufreq/imx6q-cpufreq.c
> +++ b/drivers/cpufreq/imx6q-cpufreq.c
> @@ -383,23 +383,22 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
>  		goto put_reg;
>  	}
>  
> +	/* Because we have added the OPPs here, we must free them */
> +	free_opp = true;
> +
>  	if (of_machine_is_compatible("fsl,imx6ul") ||
>  	    of_machine_is_compatible("fsl,imx6ull")) {
>  		ret = imx6ul_opp_check_speed_grading(cpu_dev);
>  		if (ret) {
> -			if (ret == -EPROBE_DEFER)
> -				goto put_node;
> -
> -			dev_err(cpu_dev, "failed to read ocotp: %d\n",
> -				ret);
> -			goto put_node;
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(cpu_dev, "failed to read ocotp: %d\n",
> +					ret);
> +			goto out_free_opp;
>  		}
>  	} else {
>  		imx6q_opp_check_speed_grading(cpu_dev);
>  	}
>  
> -	/* Because we have added the OPPs here, we must free them */
> -	free_opp = true;
>  	num = dev_pm_opp_get_opp_count(cpu_dev);
>  	if (num < 0) {
>  		ret = num;

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

@Rafael: This needs to go in 5.2 itself, can you queue it directly ?

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] cpufreq: imx6q: Fix the resource leak caused by incorrect error return

[Uwe Kleine-König]

On Tue, May 07, 2019 at 11:23:27AM +0530, Viresh Kumar wrote:
> On 05-05-19, 08:02, Jacky Bai wrote:
> > Previous goto only handled the node reference, the opp table,
> > regulator & clk resource also need to be free before error return.
> > 
> > Fixes: ddb64c5db3c (cpufreq: imx6q: fix possible object reference leak).
> 
> This should have been.
> 
> Fixes: ddb64c5db3cc ("cpufreq: imx6q: fix possible object reference leak")
> 
> Auto configure it with following in .gitconfig
> 
> [pretty]
> 	fixes = Fixes: %h (\"%s\")
> 	onelin = commit %h (\"%s\")
> 
> and then:
> 
> git log --pretty=fixes ddb64c5db3c
> 
> will generate it by itself.

Just to add my color of the bikeshed, I have (among others):

[alias]
	oneq = show -s --pretty='format:%h (\"%s\")'

in my ~/.gitconfig and can do:

$ git oneq ddb64c5db3c
ddb64c5db3cc ("cpufreq: imx6q: fix possible object reference leak")

which is a bit shorter than Viresh's suggestion.

(Originally I had "one" without the quotes which I learned (IIRC) from
the git mailing list. Instead of deviating from this I added 'q' for
"quotes" to match the usual convention in kernel land.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] cpufreq: imx6q: Fix the resource leak caused by incorrect error return

[Viresh Kumar]

On 07-05-19, 08:07, Uwe Kleine-König wrote:
> Just to add my color of the bikeshed, I have (among others):
> 
> [alias]
> 	oneq = show -s --pretty='format:%h (\"%s\")'
> 
> in my ~/.gitconfig and can do:
> 
> $ git oneq ddb64c5db3c
> ddb64c5db3cc ("cpufreq: imx6q: fix possible object reference leak")
> 
> which is a bit shorter than Viresh's suggestion.
> 
> (Originally I had "one" without the quotes which I learned (IIRC) from
> the git mailing list. Instead of deviating from this I added 'q' for
> "quotes" to match the usual convention in kernel land.)

I didn't tell that I also use some bash style aliases :)

$ alias glf
alias glf='git log --pretty=fixes'

and so all I do is:

glf ddb64c5db3c

Thanks Uwe.

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel