Message-ID: <20241217183711.2525863-1-robh@kernel.org>
Date: Tue, 17 Dec 2024 12:37:09 -0600
From: "Rob Herring (Arm)" <robh@...nel.org>
To: Petr Mladek <pmladek@...e.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Jonathan Corbet <corbet@....net>,
	Saravana Kannan <saravanak@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc: Zijun Hu <quic_zijuhu@...cinc.com>,
	linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: [PATCH] of: Add printf '%pOFm' for generating modalias

The callers for of_modalias() generally need the module alias as part of
some larger string. That results in some error prone manipulation of the
buffer prepend/append the module alias string. In fact,
of_device_uevent_modalias() has several issues. First, it's off by one
too few characters in utilization of the full buffer. Second, the error
paths leave OF_MODALIAS with a truncated value when in the end nothing
should be added to the buffer. It is also fragile because it needs
internal details of struct kobj_uevent_env. add_uevent_var() really
wants to write the env variable and value in one shot which would need
either a temporary buffer for value or a format specifier.

Fix these issues by adding a new printf format specifier, "%pOFm". With
the format specifier in place, simplify all the callers of
of_modalias(). of_modalias() can also be simplified with vsprintf()
being the only caller as it avoids the error conditions.

Cc: Zijun Hu <quic_zijuhu@...cinc.com>
Signed-off-by: Rob Herring (Arm) <robh@...nel.org>
---
 Documentation/core-api/printk-formats.rst |  1 +
 drivers/of/device.c                       | 25 ++--------------
 drivers/of/module.c                       | 35 +++++------------------
 drivers/of/unittest.c                     |  2 ++
 include/linux/of.h                        |  8 +++---
 lib/vsprintf.c                            |  7 +++--
 6 files changed, 22 insertions(+), 56 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index ecccc0473da9..d72fe3d8c427 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -496,6 +496,7 @@ equivalent to %pOFf.
 	- F - device node flags
 	- c - major compatible string
 	- C - full compatible string
+	- m - module alias string
 
 The separator when using multiple arguments is ':'
 
diff --git a/drivers/of/device.c b/drivers/of/device.c
index edf3be197265..ae8c47d5db8e 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -199,14 +199,9 @@ ssize_t of_device_modalias(struct device *dev, char *str, ssize_t len)
 	if (!dev || !dev->of_node || dev->of_node_reused)
 		return -ENODEV;
 
-	sl = of_modalias(dev->of_node, str, len - 2);
-	if (sl < 0)
-		return sl;
-	if (sl > len - 2)
+	sl = snprintf(str, len, "%pOFm\n", dev->of_node);
+	if (sl >= len)
 		return -ENOMEM;
-
-	str[sl++] = '\n';
-	str[sl] = 0;
 	return sl;
 }
 EXPORT_SYMBOL_GPL(of_device_modalias);
@@ -256,24 +251,10 @@ EXPORT_SYMBOL_GPL(of_device_uevent);
 
 int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env *env)
 {
-	int sl;
-
 	if ((!dev) || (!dev->of_node) || dev->of_node_reused)
 		return -ENODEV;
 
-	/* Devicetree modalias is tricky, we add it in 2 steps */
-	if (add_uevent_var(env, "MODALIAS="))
-		return -ENOMEM;
-
-	sl = of_modalias(dev->of_node, &env->buf[env->buflen-1],
-			 sizeof(env->buf) - env->buflen);
-	if (sl < 0)
-		return sl;
-	if (sl >= (sizeof(env->buf) - env->buflen))
-		return -ENOMEM;
-	env->buflen += sl;
-
-	return 0;
+	return add_uevent_var(env, "MODALIAS=%pOFm", dev->of_node);
 }
 EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
 
diff --git a/drivers/of/module.c b/drivers/of/module.c
index 1e735fc130ad..80879d2abea8 100644
--- a/drivers/of/module.c
+++ b/drivers/of/module.c
@@ -8,21 +8,14 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 
-ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len)
+/* Do not use directly, use %pOFm format specifier instead */
+size_t of_modalias(const struct device_node *np, char *str, size_t len)
 {
 	const char *compat;
 	char *c;
 	struct property *p;
-	ssize_t csize;
-	ssize_t tsize;
-
-	/*
-	 * Prevent a kernel oops in vsnprintf() -- it only allows passing a
-	 * NULL ptr when the length is also 0. Also filter out the negative
-	 * lengths...
-	 */
-	if ((len > 0 && !str) || len < 0)
-		return -EINVAL;
+	size_t csize;
+	size_t tsize;
 
 	/* Name & Type */
 	/* %p eats all alphanum characters, so %c must be used here */
@@ -53,29 +46,15 @@ ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len)
 
 int of_request_module(const struct device_node *np)
 {
-	char *str;
-	ssize_t size;
-	int ret;
+	char *str __free(kfree);
 
 	if (!np)
 		return -ENODEV;
 
-	size = of_modalias(np, NULL, 0);
-	if (size < 0)
-		return size;
-
-	/* Reserve an additional byte for the trailing '\0' */
-	size++;
-
-	str = kmalloc(size, GFP_KERNEL);
+	str = kasprintf(GFP_KERNEL, "%pOFm", np);
 	if (!str)
 		return -ENOMEM;
 
-	of_modalias(np, str, size);
-	str[size - 1] = '\0';
-	ret = request_module(str);
-	kfree(str);
-
-	return ret;
+	return request_module(str);
 }
 EXPORT_SYMBOL_GPL(of_request_module);
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index daf9a2dddd7e..93921399f02d 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -342,6 +342,8 @@ static void __init of_unittest_printf(void)
 	of_unittest_printf_one(np, "%pOFc", "test-sub-device");
 	of_unittest_printf_one(np, "%pOFC",
 			"\"test-sub-device\",\"test-compat2\",\"test-compat3\"");
+	of_unittest_printf_one(np, "%pOFm",
+			"of:NdevT(null)Ctest-sub-deviceCtest-compat2Ctest-compat3");
 }
 
 struct node_hash {
diff --git a/include/linux/of.h b/include/linux/of.h
index f921786cb8ac..9fe7d17ce7e2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -382,7 +382,7 @@ extern int of_count_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name);
 
 /* module functions */
-extern ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len);
+extern size_t of_modalias(const struct device_node *np, char *str, size_t len);
 extern int of_request_module(const struct device_node *np);
 
 /* phandle iterator functions */
@@ -762,10 +762,10 @@ static inline int of_count_phandle_with_args(const struct device_node *np,
 	return -ENOSYS;
 }
 
-static inline ssize_t of_modalias(const struct device_node *np, char *str,
-				  ssize_t len)
+static inline size_t of_modalias(const struct device_node *np, char *str,
+				 size_t len)
 {
-	return -ENODEV;
+	return 0;
 }
 
 static inline int of_request_module(const struct device_node *np)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 9d3dac38a3f4..6a4f99b39de0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2169,10 +2169,10 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 
 	/* simple case without anything any more format specifiers */
 	fmt++;
-	if (fmt[0] == '\0' || strcspn(fmt,"fnpPFcC") > 0)
+	if (fmt[0] == '\0' || strcspn(fmt,"fnpPFcCm") > 0)
 		fmt = "f";
 
-	for (pass = false; strspn(fmt,"fnpPFcC"); fmt++, pass = true) {
+	for (pass = false; strspn(fmt,"fnpPFcCm"); fmt++, pass = true) {
 		int precision;
 		if (pass) {
 			if (buf < end)
@@ -2226,6 +2226,9 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 				has_mult = true;
 			}
 			break;
+		case 'm':
+			buf += of_modalias(dn, buf, end - buf);
+			break;
 		default:
 			break;
 		}
-- 
2.45.2

Powered by blists - more mailing lists