Skip to contentSkip to conversation

of: kobj: fix unreasonable pr_warn() in safe_name()

PATCH In review [email protected] 2 participants 2 messages Feb 16, 2020 – Feb 18, 2020 Device tree

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 of: kobj: fix unreasonable pr_warn() in safe_name()

From: chenqiwu <[email protected]>

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.

---
 drivers/of/kobj.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/of/kobj.c +6 -3
index c72eef9..b32cf55 100644
@@ -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;
 }
 
Signature · 1 line
1.9.1
#1 lore raw
Quoted 5 lines · On 2/15/20 9:11 PM, [email protected] wrote:

From: chenqiwu <[email protected]>

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

Quoted 8 lines

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

Quoted 29 lines
---
 drivers/of/kobj.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/of/kobj.c +6 -3
index c72eef9..b32cf55 100644
@@ -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;
 }