[PATCH AUTOSEL 4.9 09/13] mips: lantiq: falcon: Fix refcount leak bug in sysctrl

[PATCH AUTOSEL 4.9 09/13] mips: lantiq: falcon: Fix refcount leak bug in sysctrl

[Sasha Levin]

From: Liang He <windhl@126.com>

[ Upstream commit 72a2af539fff975caadd9a4db3f99963569bd9c9 ]

In ltq_soc_init(), of_find_compatible_node() will return a node pointer
with refcount incremented. We should use of_node_put() when it is not
used anymore.

Signed-off-by: Liang He <windhl@126.com>
Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/mips/lantiq/falcon/sysctrl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/mips/lantiq/falcon/sysctrl.c b/arch/mips/lantiq/falcon/sysctrl.c
index 714d92659489..665739bd4190 100644
--- a/arch/mips/lantiq/falcon/sysctrl.c
+++ b/arch/mips/lantiq/falcon/sysctrl.c
@@ -210,6 +210,12 @@ void __init ltq_soc_init(void)
 			of_address_to_resource(np_sysgpe, 0, &res_sys[2]))
 		panic("Failed to get core resources");
 
+	of_node_put(np_status);
+	of_node_put(np_ebu);
+	of_node_put(np_sys1);
+	of_node_put(np_syseth);
+	of_node_put(np_sysgpe);
+
 	if ((request_mem_region(res_status.start, resource_size(&res_status),
 				res_status.name) < 0) ||
 		(request_mem_region(res_ebu.start, resource_size(&res_ebu),
-- 
2.35.1

[PATCH AUTOSEL 4.9 10/13] mips: lantiq: xway: Fix refcount leak bug in sysctrl

[Sasha Levin]

From: Liang He <windhl@126.com>

[ Upstream commit 76695592711ef1e215cc24ed3e1cd857d7fc3098 ]

In ltq_soc_init(), of_find_compatible_node() will return a node
pointer with refcount incremented. We should use of_node_put() when
it is not used anymore.

Signed-off-by: Liang He <windhl@126.com>
Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/mips/lantiq/xway/sysctrl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/mips/lantiq/xway/sysctrl.c b/arch/mips/lantiq/xway/sysctrl.c
index dd7c36a193e3..6891456a7603 100644
--- a/arch/mips/lantiq/xway/sysctrl.c
+++ b/arch/mips/lantiq/xway/sysctrl.c
@@ -457,6 +457,10 @@ void __init ltq_soc_init(void)
 			of_address_to_resource(np_ebu, 0, &res_ebu))
 		panic("Failed to get core resources");
 
+	of_node_put(np_pmu);
+	of_node_put(np_cgu);
+	of_node_put(np_ebu);
+
 	if (!request_mem_region(res_pmu.start, resource_size(&res_pmu),
 				res_pmu.name) ||
 		!request_mem_region(res_cgu.start, resource_size(&res_cgu),
-- 
2.35.1

[PATCH AUTOSEL 4.9 11/13] mips/pic32/pic32mzda: Fix refcount leak bugs

[Sasha Levin]

From: Liang He <windhl@126.com>

[ Upstream commit eb9e9bc4fa5fb489c92ec588b3fb35f042ba6d86 ]

of_find_matching_node(), of_find_compatible_node() and
of_find_node_by_path() will return node pointers with refcout
incremented. We should call of_node_put() when they are not
used anymore.

Signed-off-by: Liang He <windhl@126.com>
Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/mips/pic32/pic32mzda/init.c | 7 ++++++-
 arch/mips/pic32/pic32mzda/time.c | 3 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/mips/pic32/pic32mzda/init.c b/arch/mips/pic32/pic32mzda/init.c
index 406c6c5cec29..f8985d4573e6 100644
--- a/arch/mips/pic32/pic32mzda/init.c
+++ b/arch/mips/pic32/pic32mzda/init.c
@@ -131,13 +131,18 @@ static int __init pic32_of_prepare_platform_data(struct of_dev_auxdata *lookup)
 		np = of_find_compatible_node(NULL, NULL, lookup->compatible);
 		if (np) {
 			lookup->name = (char *)np->name;
-			if (lookup->phys_addr)
+			if (lookup->phys_addr) {
+				of_node_put(np);
 				continue;
+			}
 			if (!of_address_to_resource(np, 0, &res))
 				lookup->phys_addr = res.start;
+			of_node_put(np);
 		}
 	}
 
+	of_node_put(root);
+
 	return 0;
 }
 
diff --git a/arch/mips/pic32/pic32mzda/time.c b/arch/mips/pic32/pic32mzda/time.c
index 62a0a78b6c64..bfafe241c1b5 100644
--- a/arch/mips/pic32/pic32mzda/time.c
+++ b/arch/mips/pic32/pic32mzda/time.c
@@ -40,6 +40,9 @@ static unsigned int pic32_xlate_core_timer_irq(void)
 		goto default_map;
 
 	irq = irq_of_parse_and_map(node, 0);
+
+	of_node_put(node);
+
 	if (!irq)
 		goto default_map;
 
-- 
2.35.1

Re: [PATCH AUTOSEL 4.9 11/13] mips/pic32/pic32mzda: Fix refcount leak bugs

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]

Hi!

> From: Liang He <windhl@126.com>
> 
> [ Upstream commit eb9e9bc4fa5fb489c92ec588b3fb35f042ba6d86 ]
> 
> of_find_matching_node(), of_find_compatible_node() and
> of_find_node_by_path() will return node pointers with refcout
> incremented. We should call of_node_put() when they are not
> used anymore.

It looks like this may introduces an use-after-free bug:

> +++ b/arch/mips/pic32/pic32mzda/init.c
> @@ -131,13 +131,18 @@ static int __init pic32_of_prepare_platform_data(struct of_dev_auxdata *lookup)
>  		np = of_find_compatible_node(NULL, NULL, lookup->compatible);
>  		if (np) {
>  			lookup->name = (char *)np->name;
> -			if (lookup->phys_addr)
> +			if (lookup->phys_addr) {
> +				of_node_put(np);
>  				continue;
> +			}
>  			if (!of_address_to_resource(np, 0, &res))
>  				lookup->phys_addr = res.start;
> +			of_node_put(np);
>  		}
>  	}

lookup->name now contains pointer taken from np->name, but we did
put() on the np. What guarantees np->name is not freed?

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re:Re: [PATCH AUTOSEL 4.9 11/13] mips/pic32/pic32mzda: Fix refcount leak bugs

[Liang He]

At 2022-06-29 21:06:17, "Pavel Machek" <pavel@denx.de> wrote:
>Hi!
>
>> From: Liang He <windhl@126.com>
>> 
>> [ Upstream commit eb9e9bc4fa5fb489c92ec588b3fb35f042ba6d86 ]
>> 
>> of_find_matching_node(), of_find_compatible_node() and
>> of_find_node_by_path() will return node pointers with refcout
>> incremented. We should call of_node_put() when they are not
>> used anymore.
>
>It looks like this may introduces an use-after-free bug:
>
>> +++ b/arch/mips/pic32/pic32mzda/init.c
>> @@ -131,13 +131,18 @@ static int __init pic32_of_prepare_platform_data(struct of_dev_auxdata *lookup)
>>  		np = of_find_compatible_node(NULL, NULL, lookup->compatible);
>>  		if (np) {
>>  			lookup->name = (char *)np->name;
>> -			if (lookup->phys_addr)
>> +			if (lookup->phys_addr) {
>> +				of_node_put(np);
>>  				continue;
>> +			}
>>  			if (!of_address_to_resource(np, 0, &res))
>>  				lookup->phys_addr = res.start;
>> +			of_node_put(np);
>>  		}
>>  	}
>
>lookup->name now contains pointer taken from np->name, but we did
>put() on the np. What guarantees np->name is not freed?
>
>Best regards,
>								Pavel

Hi, Pavel.

Thanks for you to review this patched code.

In fact, the |PUT| on 'np' will not lead to the |FREE|.
First, before calling of_find_compatible_node(), the target object's refcount must be >= 1, as the object is alive.
Then, after calling of_find_compatible_node(), its refcount must be >=2.
So, after calling of_node_put(np), its refcount must be still >=1.

In fact, these |PUT|s are just used to keep refcount balance for the |GET| in of_find_compatible_node().

If there is anything wrong, please correct me.

Thans very much to review my patch code.

Liang

>-- 
>DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[PATCH AUTOSEL 4.9 12/13] mips: lantiq: Add missing of_node_put() in irq.c

[Sasha Levin]

From: Liang He <windhl@126.com>

[ Upstream commit 3748d2185ac4c2c6f80989672253aad909ecaf95 ]

In icu_of_init(), of_find_compatible_node() will return a node
pointer with refcount incremented. We should use of_node_put()
when it is not used anymore.

Signed-off-by: Liang He <windhl@126.com>
Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/mips/lantiq/irq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/lantiq/irq.c b/arch/mips/lantiq/irq.c
index e64f678ca12c..e29dc58271b2 100644
--- a/arch/mips/lantiq/irq.c
+++ b/arch/mips/lantiq/irq.c
@@ -460,6 +460,7 @@ int __init icu_of_init(struct device_node *node, struct device_node *parent)
 		if (!ltq_eiu_membase)
 			panic("Failed to remap eiu memory");
 	}
+	of_node_put(eiu_node);
 
 	return 0;
 }
-- 
2.35.1

[PATCH AUTOSEL 4.9 09/13] mips: lantiq: falcon: Fix refcount leak bug in sysctrl

[Sasha Levin]

From: Liang He <windhl@126.com>

[ Upstream commit 72a2af539fff975caadd9a4db3f99963569bd9c9 ]

In ltq_soc_init(), of_find_compatible_node() will return a node pointer
with refcount incremented. We should use of_node_put() when it is not
used anymore.

Signed-off-by: Liang He <windhl@126.com>
Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/mips/lantiq/falcon/sysctrl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/mips/lantiq/falcon/sysctrl.c b/arch/mips/lantiq/falcon/sysctrl.c
index 714d926594897..665739bd41900 100644
--- a/arch/mips/lantiq/falcon/sysctrl.c
+++ b/arch/mips/lantiq/falcon/sysctrl.c
@@ -210,6 +210,12 @@ void __init ltq_soc_init(void)
 			of_address_to_resource(np_sysgpe, 0, &res_sys[2]))
 		panic("Failed to get core resources");
 
+	of_node_put(np_status);
+	of_node_put(np_ebu);
+	of_node_put(np_sys1);
+	of_node_put(np_syseth);
+	of_node_put(np_sysgpe);
+
 	if ((request_mem_region(res_status.start, resource_size(&res_status),
 				res_status.name) < 0) ||
 		(request_mem_region(res_ebu.start, resource_size(&res_ebu),
-- 
2.35.1