diff mbox series

[7/7] cpufreq: ppc_cbe: fix possible object reference leak

Message ID 1554082674-2049-8-git-send-email-wen.yang99@zte.com.cn (mailing list archive)
State Accepted
Delegated to: viresh kumar
Headers show
Series [1/7] cpufreq: ap806: fix possible object reference leak | expand

Commit Message

Wen Yang April 1, 2019, 1:37 a.m. UTC
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 <wen.yang99@zte.com.cn>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/cpufreq/ppc_cbe_cpufreq.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Markus Elfring April 2, 2019, 10:43 a.m. UTC | #1
> 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
Julia Lawall April 2, 2019, 10:50 a.m. UTC | #2
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
Markus Elfring April 2, 2019, 12:50 p.m. UTC | #3
> @@ -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
Julia Lawall April 2, 2019, 1:15 p.m. UTC | #4
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
diff mbox series

Patch

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;
 	}