The call to of_get_cpu_node/of_find_compatible_node/of_parse_phandle...
returns a node pointer with refcount incremented thus it must be
explicitly decremented after the last usage.
This patch series fix those possible object reference leaks in cpufreq.
Wen Yang (7):
cpufreq: ap806: fix possible object reference leak
cpufreq: imx6q: fix possible object reference leak
cpufreq: kirkwood: fix possible object reference leak
cpufreq: maple: fix possible object reference leak
cpufreq/pasemi: fix possible object reference leak
cpufreq: pmac32: fix possible object reference leak
cpufreq: ppc_cbe: fix possible object reference leak
drivers/cpufreq/armada-8k-cpufreq.c | 1 +
drivers/cpufreq/imx6q-cpufreq.c | 4 ++--
drivers/cpufreq/kirkwood-cpufreq.c | 19 +++++++++++--------
drivers/cpufreq/maple-cpufreq.c | 2 +-
drivers/cpufreq/pasemi-cpufreq.c | 1 +
drivers/cpufreq/pmac32-cpufreq.c | 2 ++
drivers/cpufreq/ppc_cbe_cpufreq.c | 1 +
7 files changed, 19 insertions(+), 11 deletions(-)
--
2.9.5
The call to of_node_get returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
./drivers/cpufreq/imx6q-cpufreq.c:391:4-10: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 348, but without a corresponding object release within this function.
./drivers/cpufreq/imx6q-cpufreq.c:395:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 348, but without a corresponding object release within this function.
Signed-off-by: Wen Yang <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: NXP Linux Team <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/cpufreq/imx6q-cpufreq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index a4ff09f..3e17560 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -388,11 +388,11 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
ret = imx6ul_opp_check_speed_grading(cpu_dev);
if (ret) {
if (ret == -EPROBE_DEFER)
- return ret;
+ goto put_node;
dev_err(cpu_dev, "failed to read ocotp: %d\n",
ret);
- return ret;
+ goto put_node;
}
} else {
imx6q_opp_check_speed_grading(cpu_dev);
--
2.9.5
The call to of_cpu_device_node_get returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
./drivers/cpufreq/maple-cpufreq.c:213:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 177, but without a corresponding object release within this function.
Signed-off-by: Wen Yang <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Viresh Kumar <[email protected]>
[email protected]
[email protected]
---
drivers/cpufreq/maple-cpufreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/maple-cpufreq.c b/drivers/cpufreq/maple-cpufreq.c
index d9df893..a05f134 100644
--- a/drivers/cpufreq/maple-cpufreq.c
+++ b/drivers/cpufreq/maple-cpufreq.c
@@ -210,7 +210,7 @@ static int __init maple_cpufreq_init(void)
*/
valp = of_get_property(cpunode, "clock-frequency", NULL);
if (!valp)
- return -ENODEV;
+ goto bail_noprops;
max_freq = (*valp)/1000;
maple_cpu_freqs[0].frequency = max_freq;
maple_cpu_freqs[1].frequency = max_freq/2;
--
2.9.5
The call to of_find_node_by_name returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
./drivers/cpufreq/pmac32-cpufreq.c:557:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 552, but without a corresponding object release within this function.
./drivers/cpufreq/pmac32-cpufreq.c:569:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 552, but without a corresponding object release within this function.
./drivers/cpufreq/pmac32-cpufreq.c:598:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 587, but without a corresponding object release within this function.
Signed-off-by: Wen Yang <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/cpufreq/pmac32-cpufreq.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/cpufreq/pmac32-cpufreq.c b/drivers/cpufreq/pmac32-cpufreq.c
index 52f0d91..9b4ce2e 100644
--- a/drivers/cpufreq/pmac32-cpufreq.c
+++ b/drivers/cpufreq/pmac32-cpufreq.c
@@ -552,6 +552,7 @@ static int pmac_cpufreq_init_7447A(struct device_node *cpunode)
volt_gpio_np = of_find_node_by_name(NULL, "cpu-vcore-select");
if (volt_gpio_np)
voltage_gpio = read_gpio(volt_gpio_np);
+ of_node_put(volt_gpio_np);
if (!voltage_gpio){
pr_err("missing cpu-vcore-select gpio\n");
return 1;
@@ -588,6 +589,7 @@ static int pmac_cpufreq_init_750FX(struct device_node *cpunode)
if (volt_gpio_np)
voltage_gpio = read_gpio(volt_gpio_np);
+ of_node_put(volt_gpio_np);
pvr = mfspr(SPRN_PVR);
has_cpu_l2lve = !((pvr & 0xf00) == 0x100);
--
2.9.5
The call to of_get_cpu_node returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
./drivers/cpufreq/ppc_cbe_cpufreq.c:89:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 76, but without a corresponding object release within this function.
./drivers/cpufreq/ppc_cbe_cpufreq.c:89:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 76, but without a corresponding object release within this function.
Signed-off-by: Wen Yang <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/cpufreq/ppc_cbe_cpufreq.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.c b/drivers/cpufreq/ppc_cbe_cpufreq.c
index 41a0f0b..8414c3a 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq.c
+++ b/drivers/cpufreq/ppc_cbe_cpufreq.c
@@ -86,6 +86,7 @@ static int cbe_cpufreq_cpu_init(struct cpufreq_policy *policy)
if (!cbe_get_cpu_pmd_regs(policy->cpu) ||
!cbe_get_cpu_mic_tm_regs(policy->cpu)) {
pr_info("invalid CBE regs pointers for cpufreq\n");
+ of_node_put(cpu);
return -EINVAL;
}
--
2.9.5
The call to of_get_cpu_node returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
./drivers/cpufreq/pasemi-cpufreq.c:212:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 147, but without a corresponding object release within this function.
./drivers/cpufreq/pasemi-cpufreq.c:220:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 147, but without a corresponding object release within this function.
Signed-off-by: Wen Yang <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/cpufreq/pasemi-cpufreq.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
index 75dfbd2..c7710c1 100644
--- a/drivers/cpufreq/pasemi-cpufreq.c
+++ b/drivers/cpufreq/pasemi-cpufreq.c
@@ -146,6 +146,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
cpu = of_get_cpu_node(policy->cpu, NULL);
+ of_node_put(cpu);
if (!cpu)
goto out;
--
2.9.5
The call to of_find_compatible_node returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
./drivers/cpufreq/armada-8k-cpufreq.c:187:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 130, but without a corresponding object release within this function.
./drivers/cpufreq/armada-8k-cpufreq.c:191:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 130, but without a corresponding object release within this function.
Signed-off-by: Wen Yang <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Sebastian Hesselbarth <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/cpufreq/armada-8k-cpufreq.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/armada-8k-cpufreq.c b/drivers/cpufreq/armada-8k-cpufreq.c
index b3f4bd6..988ebc3 100644
--- a/drivers/cpufreq/armada-8k-cpufreq.c
+++ b/drivers/cpufreq/armada-8k-cpufreq.c
@@ -132,6 +132,7 @@ static int __init armada_8k_cpufreq_init(void)
of_node_put(node);
return -ENODEV;
}
+ of_node_put(node);
nb_cpus = num_possible_cpus();
freq_tables = kcalloc(nb_cpus, sizeof(*freq_tables), GFP_KERNEL);
--
2.9.5
The call to of_get_child_by_name returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
Detected by coccinelle with the following warnings:
./drivers/cpufreq/kirkwood-cpufreq.c:127:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 118, but without a corresponding object release within this function.
./drivers/cpufreq/kirkwood-cpufreq.c:133:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 118, but without a corresponding object release within this function.
and also do some cleanup:
- of_node_put(np);
- np = NULL;
...
of_node_put(np);
Signed-off-by: Wen Yang <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/cpufreq/kirkwood-cpufreq.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/cpufreq/kirkwood-cpufreq.c b/drivers/cpufreq/kirkwood-cpufreq.c
index c2dd43f..8d63a6dc 100644
--- a/drivers/cpufreq/kirkwood-cpufreq.c
+++ b/drivers/cpufreq/kirkwood-cpufreq.c
@@ -124,13 +124,14 @@ static int kirkwood_cpufreq_probe(struct platform_device *pdev)
priv.cpu_clk = of_clk_get_by_name(np, "cpu_clk");
if (IS_ERR(priv.cpu_clk)) {
dev_err(priv.dev, "Unable to get cpuclk\n");
- return PTR_ERR(priv.cpu_clk);
+ err = PTR_ERR(priv.cpu_clk);
+ goto out_node;
}
err = clk_prepare_enable(priv.cpu_clk);
if (err) {
dev_err(priv.dev, "Unable to prepare cpuclk\n");
- return err;
+ goto out_node;
}
kirkwood_freq_table[0].frequency = clk_get_rate(priv.cpu_clk) / 1000;
@@ -161,20 +162,22 @@ static int kirkwood_cpufreq_probe(struct platform_device *pdev)
goto out_ddr;
}
- of_node_put(np);
- np = NULL;
-
err = cpufreq_register_driver(&kirkwood_cpufreq_driver);
- if (!err)
- return 0;
+ if (err) {
+ dev_err(priv.dev, "Failed to register cpufreq driver\n");
+ goto out_powersave;
+ }
- dev_err(priv.dev, "Failed to register cpufreq driver\n");
+ of_node_put(np);
+ return 0;
+out_powersave:
clk_disable_unprepare(priv.powersave_clk);
out_ddr:
clk_disable_unprepare(priv.ddr_clk);
out_cpu:
clk_disable_unprepare(priv.cpu_clk);
+out_node:
of_node_put(np);
return err;
--
2.9.5
> The call to of_get_cpu_node returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
I would prefer a wording like the following.
A reference counter was incremented for a CPU node by a call of
the function “of_get_cpu_node”.
Thus decrement it after the last usage.
> Detected by coccinelle with the following warnings:
I wonder about the shown duplicate notification.
Can a single message be sufficient for the code search result
in this source file?
Regards,
Markus
On Tue, 2 Apr 2019, Markus Elfring wrote:
> > The call to of_get_cpu_node returns a node pointer with refcount
> > incremented thus it must be explicitly decremented after the last
> > usage.
>
> I would prefer a wording like the following.
>
> A reference counter was incremented for a CPU node by a call of
> the function “of_get_cpu_node”.
> Thus decrement it after the last usage.
The original log message seems perfectly clear.
>
>
> > Detected by coccinelle with the following warnings:
>
> I wonder about the shown duplicate notification.
> Can a single message be sufficient for the code search result
> in this source file?
Since you have removed the context, I have no idea what you are talking
about.
julia
> @@ -86,6 +86,7 @@ static int cbe_cpufreq_cpu_init(struct cpufreq_policy *policy)
> if (!cbe_get_cpu_pmd_regs(policy->cpu) ||
> !cbe_get_cpu_mic_tm_regs(policy->cpu)) {
> pr_info("invalid CBE regs pointers for cpufreq\n");
> + of_node_put(cpu);
> return -EINVAL;
> }
I have taken another look at the implementation of this function.
I find that the second statement “return -EINVAL” would need related
source code adjustments.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpufreq/ppc_cbe_cpufreq.c?id=05d08e2995cbe6efdb993482ee0d38a77040861a#n96
How do you think about to complete the exception handling here?
Regards,
Markus
On Tue, 2 Apr 2019, Markus Elfring wrote:
> > @@ -86,6 +86,7 @@ static int cbe_cpufreq_cpu_init(struct cpufreq_policy *policy)
> > if (!cbe_get_cpu_pmd_regs(policy->cpu) ||
> > !cbe_get_cpu_mic_tm_regs(policy->cpu)) {
> > pr_info("invalid CBE regs pointers for cpufreq\n");
> > + of_node_put(cpu);
> > return -EINVAL;
> > }
>
> I have taken another look at the implementation of this function.
> I find that the second statement “return -EINVAL” would need related
> source code adjustments.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpufreq/ppc_cbe_cpufreq.c?id=05d08e2995cbe6efdb993482ee0d38a77040861a#n96
>
> How do you think about to complete the exception handling here?
There is an of_node_put two lines above.
julia
> @@ -132,6 +132,7 @@ static int __init armada_8k_cpufreq_init(void)
> of_node_put(node);
> return -ENODEV;
> }
> + of_node_put(node);
>
> nb_cpus = num_possible_cpus();
> freq_tables = kcalloc(nb_cpus, sizeof(*freq_tables), GFP_KERNEL);
Would another null pointer check be safer for this memory allocation?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpufreq/armada-8k-cpufreq.c?id=05d08e2995cbe6efdb993482ee0d38a77040861a#n137
Regards,
Markus
> @@ -132,6 +132,7 @@ static int __init armada_8k_cpufreq_init(void)
> of_node_put(node);
> return -ENODEV;
> }
> + of_node_put(node);
>
> nb_cpus = num_possible_cpus();
> freq_tables = kcalloc(nb_cpus, sizeof(*freq_tables), GFP_KERNEL);
Would another null pointer check be safer for this memory allocation?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpufreq/armada-8k-cpufreq.c?id=05d08e2995cbe6efdb993482ee0d38a77040861a#n137
Regards,
Markus
> and also do some cleanup:
Would it be safer to integrate this source code adjustment
by another update step?
Regards,
Markus
> @@ -210,7 +210,7 @@ static int __init maple_cpufreq_init(void)
> */
> valp = of_get_property(cpunode, "clock-frequency", NULL);
> if (!valp)
> - return -ENODEV;
> + goto bail_noprops;
> max_freq = (*valp)/1000;
> maple_cpu_freqs[0].frequency = max_freq;
> maple_cpu_freqs[1].frequency = max_freq/2;
* How do you think about use the jump label “put_node” instead?
* Would you like to reduce a bit of duplicate source code at the end
of this function?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpufreq/maple-cpufreq.c?id=bf97b82f37c6d90e16de001d0659644c57fa490d#n232
Regards,
Markus
> @@ -146,6 +146,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
>
> cpu = of_get_cpu_node(policy->cpu, NULL);
>
> + of_node_put(cpu);
> if (!cpu)
> goto out;
Can the statement “return -ENODEV” be nicer as exception handling
in the if branch of this source code place?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpufreq/pasemi-cpufreq.c?id=bf97b82f37c6d90e16de001d0659644c57fa490d#n137
Regards,
Markus
On Wed, Apr 03, 2019 at 04:23:54PM +0200, Markus Elfring wrote:
> > @@ -146,6 +146,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> >
> > cpu = of_get_cpu_node(policy->cpu, NULL);
> >
> > + of_node_put(cpu);
> > if (!cpu)
> > goto out;
>
> Can the statement “return -ENODEV” be nicer as exception handling
> in the if branch of this source code place?
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpufreq/pasemi-cpufreq.c?id=bf97b82f37c6d90e16de001d0659644c57fa490d#n137
>
Why am I only receiving only one side of this conversation?
I don't know why you're responding to... It's not required to fix/change
unrelated style choices. If people want, they can just focus on their
own thing.
regards,
dan carpenter
On 01-04-19, 09:37, Wen Yang wrote:
> The call to of_find_compatible_node returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
>
> Detected by coccinelle with the following warnings:
> ./drivers/cpufreq/armada-8k-cpufreq.c:187:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 130, but without a corresponding object release within this function.
> ./drivers/cpufreq/armada-8k-cpufreq.c:191:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 130, but without a corresponding object release within this function.
>
> Signed-off-by: Wen Yang <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Gregory Clement <[email protected]>
> Cc: Sebastian Hesselbarth <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Viresh Kumar <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/cpufreq/armada-8k-cpufreq.c | 1 +
> 1 file changed, 1 insertion(+)
I haven't received the cover-letter and patch 4/7 (because the tags in that
commit were incorrect). I have applied all the 7 patches to my cpufreq tree
though.
Thanks.
--
viresh