2019-03-22 03:08:21

by Wen Yang

[permalink] [raw]
Subject: [PATCH 1/5] powerpc/sparse: fix possible object reference leak

The call to of_find_node_by_path returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.

Detected by coccinelle with the following warnings:
./arch/powerpc/platforms/pseries/pseries_energy.c:101:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 46, but without a corresponding object release within this function.
./arch/powerpc/platforms/pseries/pseries_energy.c:172:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 111, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/powerpc/platforms/pseries/pseries_energy.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
index 6ed2212..e3913e4 100644
--- a/arch/powerpc/platforms/pseries/pseries_energy.c
+++ b/arch/powerpc/platforms/pseries/pseries_energy.c
@@ -69,7 +69,7 @@ static u32 cpu_to_drc_index(int cpu)

of_read_drc_info_cell(&info, &value, &drc);
if (strncmp(drc.drc_type, "CPU", 3))
- goto err;
+ goto err_of_node_put;

if (thread_index < drc.last_drc_index)
break;
@@ -131,7 +131,7 @@ static int drc_index_to_cpu(u32 drc_index)

of_read_drc_info_cell(&info, &value, &drc);
if (strncmp(drc.drc_type, "CPU", 3))
- goto err;
+ goto err_of_node_put;

if (drc_index > drc.last_drc_index) {
cpu += drc.num_sequential_elems;
--
2.9.5



2019-03-22 03:08:21

by Wen Yang

[permalink] [raw]
Subject: [PATCH 4/5] powerpc/embedded6xx: fix possible object reference leak

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:
./arch/powerpc/platforms/embedded6xx/mvme5100.c:89:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 80, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/powerpc/platforms/embedded6xx/mvme5100.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/embedded6xx/mvme5100.c b/arch/powerpc/platforms/embedded6xx/mvme5100.c
index 273dfa3..660654f4 100644
--- a/arch/powerpc/platforms/embedded6xx/mvme5100.c
+++ b/arch/powerpc/platforms/embedded6xx/mvme5100.c
@@ -86,6 +86,7 @@ static void __init mvme5100_pic_init(void)
cirq = irq_of_parse_and_map(cp, 0);
if (!cirq) {
pr_warn("mvme5100_pic_init: no cascade interrupt?\n");
+ of_node_put(cp);
return;
}

--
2.9.5


2019-03-22 03:08:26

by Wen Yang

[permalink] [raw]
Subject: [PATCH 5/5] powerpc/8xx: fix possible object reference leak

The call to of_find_compatible_node returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
irq_domain_add_linear also calls of_node_get to increase refcount,
so irq_domain will not be affected when it is released.

Detected by coccinelle with the following warnings:
./arch/powerpc/platforms/8xx/pic.c:158:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 136, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Vitaly Bordug <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/powerpc/platforms/8xx/pic.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/8xx/pic.c b/arch/powerpc/platforms/8xx/pic.c
index 8d5a25d..13d880b 100644
--- a/arch/powerpc/platforms/8xx/pic.c
+++ b/arch/powerpc/platforms/8xx/pic.c
@@ -155,6 +155,7 @@ int mpc8xx_pic_init(void)
ret = -ENOMEM;
goto out;
}
+ of_node_put(np);
return 0;

out:
--
2.9.5


2019-03-22 03:09:07

by Wen Yang

[permalink] [raw]
Subject: [PATCH 2/5] powerpc/83xx: fix possible object reference leak

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:
./arch/powerpc/platforms/83xx/km83xx.c:68:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 59, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Scott Wood <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/powerpc/platforms/83xx/km83xx.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/83xx/km83xx.c b/arch/powerpc/platforms/83xx/km83xx.c
index d8642a4..11eea7c 100644
--- a/arch/powerpc/platforms/83xx/km83xx.c
+++ b/arch/powerpc/platforms/83xx/km83xx.c
@@ -65,6 +65,7 @@ static void quirk_mpc8360e_qe_enet10(void)
ret = of_address_to_resource(np_par, 0, &res);
if (ret) {
pr_warn("%s couldn;t map par_io registers\n", __func__);
+ of_node_put(np_par);
return;
}

--
2.9.5


2019-03-22 03:09:41

by Wen Yang

[permalink] [raw]
Subject: [PATCH 3/5] powerpc/powernv: fix possible object reference leak

The call to of_find_node_by_path returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.

Detected by coccinelle with the following warnings:
./arch/powerpc/platforms/powernv/opal.c:741:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 733, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Mahesh Salgaonkar <[email protected]>
Cc: Haren Myneni <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/powerpc/platforms/powernv/opal.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 2b0eca1..d7736a5 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -738,6 +738,7 @@ static void opal_export_attrs(void)
kobj = kobject_create_and_add("exports", opal_kobj);
if (!kobj) {
pr_warn("kobject_create_and_add() of exports failed\n");
+ of_node_put(np);
return;
}

--
2.9.5


2019-03-22 07:12:42

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 5/5] powerpc/8xx: fix possible object reference leak



On 03/22/2019 03:05 AM, 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.
> irq_domain_add_linear also calls of_node_get to increase refcount,
> so irq_domain will not be affected when it is released.


Should you have a:

Fixes: a8db8cf0d894 ("irq_domain: Replace irq_alloc_host() with
revmap-specific initializers")

If not, it means your change is in contradiction with commit
b1725c9319aa ("[POWERPC] arch/powerpc/sysdev: Add missing of_node_put")

>
> Detected by coccinelle with the following warnings:
> ./arch/powerpc/platforms/8xx/pic.c:158:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 136, but without a corresponding object release within this function.
>
> Signed-off-by: Wen Yang <[email protected]>
> Cc: Vitaly Bordug <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/powerpc/platforms/8xx/pic.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/8xx/pic.c b/arch/powerpc/platforms/8xx/pic.c
> index 8d5a25d..13d880b 100644
> --- a/arch/powerpc/platforms/8xx/pic.c
> +++ b/arch/powerpc/platforms/8xx/pic.c
> @@ -155,6 +155,7 @@ int mpc8xx_pic_init(void)
> ret = -ENOMEM;
> goto out;
> }
> + of_node_put(np);
> return 0;
>
> out:
>

I guess it would be better as follows:

--- a/arch/powerpc/platforms/8xx/pic.c
+++ b/arch/powerpc/platforms/8xx/pic.c
@@ -153,9 +153,7 @@ int mpc8xx_pic_init(void)
if (mpc8xx_pic_host == NULL) {
printk(KERN_ERR "MPC8xx PIC: failed to allocate irq
host!\n");
ret = -ENOMEM;
- goto out;
}
- return 0;

out:
of_node_put(np);



Christophe

2019-04-05 08:51:39

by Markus Elfring

[permalink] [raw]
Subject: Re: [1/5] powerpc/sparse: fix possible object reference leak

> @@ -131,7 +131,7 @@ static int drc_index_to_cpu(u32 drc_index)
>
> of_read_drc_info_cell(&info, &value, &drc);
> if (strncmp(drc.drc_type, "CPU", 3))
> - goto err;
> + goto err_of_node_put;
>
> if (drc_index > drc.last_drc_index) {

Can the jump label “put_node” be nicer here?

Regards,
Markus

2022-03-27 20:53:26

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH 2/5] powerpc/83xx: fix possible object reference leak



Le 22/03/2019 à 04:05, Wen Yang a écrit :
> 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:
> ./arch/powerpc/platforms/83xx/km83xx.c:68:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 59, but without a corresponding object release within this function.

This was fixed by
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=88654d5b4476

>
> Signed-off-by: Wen Yang <[email protected]>
> Cc: Scott Wood <[email protected]>
> Cc: Kumar Gala <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/powerpc/platforms/83xx/km83xx.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/83xx/km83xx.c b/arch/powerpc/platforms/83xx/km83xx.c
> index d8642a4..11eea7c 100644
> --- a/arch/powerpc/platforms/83xx/km83xx.c
> +++ b/arch/powerpc/platforms/83xx/km83xx.c
> @@ -65,6 +65,7 @@ static void quirk_mpc8360e_qe_enet10(void)
> ret = of_address_to_resource(np_par, 0, &res);
> if (ret) {
> pr_warn("%s couldn;t map par_io registers\n", __func__);
> + of_node_put(np_par);
> return;
> }
>