[PATCH] LoongArch: Avoid in-place string operation on FDT content

Yao Zi posted 1 patch 3 months ago
arch/loongarch/kernel/env.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
[PATCH] LoongArch: Avoid in-place string operation on FDT content
Posted by Yao Zi 3 months ago
In init_cpu_fullname, a constant pointer to model property is retrieved.
It's later modified by the strsep function, which is illegal and
corrupts kernel's FDT copy. This is shown by dmesg,

	OF: fdt: not creating '/sys/firmware/fdt': CRC check failed

Create a mutable copy of the model property and do in-place operations
on it instead. loongson_sysconf.cpuname lives across the kernel
lifetime, thus manually releasing isn't necessary.

Also move of_node_put() for the root node after the usage of its
property, since of_node_put() decreases the reference counter thus usage
after the call is unsafe.

Fixes: 44a01f1f726a ("LoongArch: Parsing CPU-related information from DTS")
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/loongarch/kernel/env.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
index 27144de5c5fe..fba141472ab9 100644
--- a/arch/loongarch/kernel/env.c
+++ b/arch/loongarch/kernel/env.c
@@ -40,15 +40,18 @@ void __init init_environ(void)
 static int __init init_cpu_fullname(void)
 {
 	struct device_node *root;
+	const char *model;
 	int cpu, ret;
-	char *model;
+	char *tmp;
 
 	/* Parsing cpuname from DTS model property */
 	root = of_find_node_by_path("/");
-	ret = of_property_read_string(root, "model", (const char **)&model);
+	ret = of_property_read_string(root, "model", &model);
+	if (ret == 0) {
+		tmp = kstrdup(model, GFP_KERNEL);
+		loongson_sysconf.cpuname = strsep(&tmp, " ");
+	}
 	of_node_put(root);
-	if (ret == 0)
-		loongson_sysconf.cpuname = strsep(&model, " ");
 
 	if (loongson_sysconf.cpuname && !strncmp(loongson_sysconf.cpuname, "Loongson", 8)) {
 		for (cpu = 0; cpu < NR_CPUS; cpu++)
-- 
2.49.0
Re: [PATCH] LoongArch: Avoid in-place string operation on FDT content
Posted by Jiaxun Yang 3 months ago

在2025年7月5日周六 下午1:57,Yao Zi写道:
> In init_cpu_fullname, a constant pointer to model property is retrieved.
> It's later modified by the strsep function, which is illegal and
> corrupts kernel's FDT copy. This is shown by dmesg,
>
> 	OF: fdt: not creating '/sys/firmware/fdt': CRC check failed
>
> Create a mutable copy of the model property and do in-place operations
> on it instead. loongson_sysconf.cpuname lives across the kernel
> lifetime, thus manually releasing isn't necessary.
>
> Also move of_node_put() for the root node after the usage of its
> property, since of_node_put() decreases the reference counter thus usage
> after the call is unsafe.
>
> Fixes: 44a01f1f726a ("LoongArch: Parsing CPU-related information from DTS")

Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>

> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/loongarch/kernel/env.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
> index 27144de5c5fe..fba141472ab9 100644
> --- a/arch/loongarch/kernel/env.c
> +++ b/arch/loongarch/kernel/env.c
> @@ -40,15 +40,18 @@ void __init init_environ(void)
>  static int __init init_cpu_fullname(void)
>  {
>  	struct device_node *root;
> +	const char *model;
>  	int cpu, ret;
> -	char *model;
> +	char *tmp;
> 
>  	/* Parsing cpuname from DTS model property */
>  	root = of_find_node_by_path("/");
> -	ret = of_property_read_string(root, "model", (const char **)&model);
> +	ret = of_property_read_string(root, "model", &model);
> +	if (ret == 0) {
> +		tmp = kstrdup(model, GFP_KERNEL);
> +		loongson_sysconf.cpuname = strsep(&tmp, " ");
> +	}
>  	of_node_put(root);
> -	if (ret == 0)
> -		loongson_sysconf.cpuname = strsep(&model, " ");
> 
>  	if (loongson_sysconf.cpuname && !strncmp(loongson_sysconf.cpuname, 
> "Loongson", 8)) {
>  		for (cpu = 0; cpu < NR_CPUS; cpu++)
> -- 
> 2.49.0

-- 
- Jiaxun
Re: [PATCH] LoongArch: Avoid in-place string operation on FDT content
Posted by Huacai Chen 3 months ago
Applied, thanks.

Huacai

On Sat, Jul 5, 2025 at 10:06 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
>
>
> 在2025年7月5日周六 下午1:57,Yao Zi写道:
> > In init_cpu_fullname, a constant pointer to model property is retrieved.
> > It's later modified by the strsep function, which is illegal and
> > corrupts kernel's FDT copy. This is shown by dmesg,
> >
> >       OF: fdt: not creating '/sys/firmware/fdt': CRC check failed
> >
> > Create a mutable copy of the model property and do in-place operations
> > on it instead. loongson_sysconf.cpuname lives across the kernel
> > lifetime, thus manually releasing isn't necessary.
> >
> > Also move of_node_put() for the root node after the usage of its
> > property, since of_node_put() decreases the reference counter thus usage
> > after the call is unsafe.
> >
> > Fixes: 44a01f1f726a ("LoongArch: Parsing CPU-related information from DTS")
>
> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  arch/loongarch/kernel/env.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
> > index 27144de5c5fe..fba141472ab9 100644
> > --- a/arch/loongarch/kernel/env.c
> > +++ b/arch/loongarch/kernel/env.c
> > @@ -40,15 +40,18 @@ void __init init_environ(void)
> >  static int __init init_cpu_fullname(void)
> >  {
> >       struct device_node *root;
> > +     const char *model;
> >       int cpu, ret;
> > -     char *model;
> > +     char *tmp;
> >
> >       /* Parsing cpuname from DTS model property */
> >       root = of_find_node_by_path("/");
> > -     ret = of_property_read_string(root, "model", (const char **)&model);
> > +     ret = of_property_read_string(root, "model", &model);
> > +     if (ret == 0) {
> > +             tmp = kstrdup(model, GFP_KERNEL);
> > +             loongson_sysconf.cpuname = strsep(&tmp, " ");
> > +     }
> >       of_node_put(root);
> > -     if (ret == 0)
> > -             loongson_sysconf.cpuname = strsep(&model, " ");
> >
> >       if (loongson_sysconf.cpuname && !strncmp(loongson_sysconf.cpuname,
> > "Loongson", 8)) {
> >               for (cpu = 0; cpu < NR_CPUS; cpu++)
> > --
> > 2.49.0
>
> --
> - Jiaxun