[PATCH] of: kobj: fix unreasonable pr_warn() in safe_name()

[PATCH] of: kobj: fix unreasonable pr_warn() in safe_name()

[qiwuchen55]

From: chenqiwu <chenqiwu@xiaomi.com>

safe_name() tries to find a non-existing name of kernfs_node
by sysfs_get_dirent() no more than 16 times.

There are three possible results when while loop breaks:
1. i = 0, name = orig_name, name is safe to use.
2. i > 0 && i < 16, name != orig_name, name is safe to use.
3. i == 16, name != orig_name, name is unsafe to use.

However, the original code consider 2nd result as unsafe with
a unnecessary warning message by pr_warn(). This patch can fix
the problem.

Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
---
 drivers/of/kobj.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c
index c72eef9..b32cf55 100644
--- a/drivers/of/kobj.c
+++ b/drivers/of/kobj.c
@@ -52,11 +52,14 @@ static const char *safe_name(struct kobject *kobj, const char *orig_name)
 	}
 
 	if (name == orig_name) {
-		name = kstrdup(orig_name, GFP_KERNEL);
-	} else {
+		name = kstrdup_const(orig_name, GFP_KERNEL);
+		return name;
+	}
+
+	if (i == 16)
 		pr_warn("Duplicate name in %s, renamed to \"%s\"\n",
 			kobject_name(kobj), name);
-	}
+
 	return name;
 }
 
-- 
1.9.1

Re: [PATCH] of: kobj: fix unreasonable pr_warn() in safe_name()

[Frank Rowand]

On 2/15/20 9:11 PM, qiwuchen55@gmail.com wrote:
> From: chenqiwu <chenqiwu@xiaomi.com>
> 
> safe_name() tries to find a non-existing name of kernfs_node
> by sysfs_get_dirent() no more than 16 times.
> 
> There are three possible results when while loop breaks:
> 1. i = 0, name = orig_name, name is safe to use.
> 2. i > 0 && i < 16, name != orig_name, name is safe to use.
> 3. i == 16, name != orig_name, name is unsafe to use.
> 
> However, the original code consider 2nd result as unsafe with
> a unnecessary warning message by pr_warn(). This patch can fix
> the problem.

No, the warning for case 2 is appropriate.

The result of the rename is that the contents of /proc/device-tree
are not equivalent to the devicetree.  It is valuable to know this
has occurred.

The only way I know of for the rename to occur is if a node contains
both a property and a node with the same name.  This should not occur
if the name conventions for properties and nodes are followed.

If anyone is aware of a scenario where variable i can be greater
than 1, please let me know.

-Frank

> 
> Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
> ---
>  drivers/of/kobj.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c
> index c72eef9..b32cf55 100644
> --- a/drivers/of/kobj.c
> +++ b/drivers/of/kobj.c
> @@ -52,11 +52,14 @@ static const char *safe_name(struct kobject *kobj, const char *orig_name)
>  	}
>  
>  	if (name == orig_name) {
> -		name = kstrdup(orig_name, GFP_KERNEL);
> -	} else {
> +		name = kstrdup_const(orig_name, GFP_KERNEL);
> +		return name;
> +	}
> +
> +	if (i == 16)
>  		pr_warn("Duplicate name in %s, renamed to \"%s\"\n",
>  			kobject_name(kobj), name);
> -	}
> +
>  	return name;
>  }
>  
>