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@xxxxxxxxxxx> Signed-off-by: Rob Herring (Arm) <robh@xxxxxxxxxx> --- 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