| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20241217-of_core_fix-v3-6-3bc49a2e8bda@quicinc.com>
Date: Tue, 17 Dec 2024 21:07:30 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Rob Herring <robh@...nel.org>, Saravana Kannan <saravanak@...gle.com>,
Maxime Ripard <mripard@...nel.org>, Robin Murphy <robin.murphy@....com>,
Grant Likely <grant.likely@...retlab.ca>
Cc: Zijun Hu <zijun_hu@...oud.com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Zijun Hu <quic_zijuhu@...cinc.com>
Subject: [PATCH v3 6/7] of: Fix potential wrong MODALIAS uevent value
From: Zijun Hu <quic_zijuhu@...cinc.com>
API of_device_uevent_modalias() makes uevent "MODALIAS=ITS_VALUE" in two
steps, namely, produces "MODALIAS=" with add_uevent_var() fistly, then
remainning "ITS_VALUE" with of_modalias() finally, and that may result
in various wrong uevents as explained below:
"MODALIAS=\0" // @env->buf is full after the 1st step.
"MODALIAS=ITS_\0" // @env->buf is not enough during 2nd step.
"MODALIAS=ITS_VAR=VAR_VALUE\0" // another uevent "VAR=VAR_VALUE" comes.
The API depends on uevent internal design, so is not good practice as well.
Fix by:
1) Respin the callee of_modalias() with new prototype which is friendly
with its callers.
2) Invoke add_uevent_var() to make the whole MODALIAS uevent.
3) Adapt new of_modalias() for its other callers.
BTW, there are no external callers of of_modalias() now.
Closes: https://lore.kernel.org/all/CAL_JsqL+CRmCQMzcF4-A-PRBrCsfK8nduJtOO=RrsDtCUUR7og@mail.gmail.com
Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
---
drivers/of/device.c | 39 +++++++-------------
drivers/of/module.c | 103 +++++++++++++++++++++++++++++++---------------------
include/linux/of.h | 7 ++--
3 files changed, 79 insertions(+), 70 deletions(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index f24c19e7aba8e01656f503ae328a4e08aab5a5f3..6355707c200da9ced354132528adbcce24121247 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -195,19 +195,18 @@ EXPORT_SYMBOL(of_device_get_match_data);
ssize_t of_device_modalias(struct device *dev, char *str, ssize_t len)
{
ssize_t sl;
+ char *ptr __free(kfree) = NULL;
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)
+ ptr = of_modalias(dev->of_node, &sl);
+ if (IS_ERR(ptr))
+ return PTR_ERR(no_free_ptr(ptr));
+ if (sl + 2 > len)
return -ENOMEM;
- str[sl++] = '\n';
- str[sl] = 0;
- return sl;
+ return snprintf(str, len, "%s\n", ptr);
}
EXPORT_SYMBOL_GPL(of_device_modalias);
@@ -256,30 +255,20 @@ EXPORT_SYMBOL_GPL(of_device_uevent);
int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env *env)
{
- int sl;
- int pos;
+ int ret;
+ char *ptr;
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;
+ ptr = of_modalias(dev->of_node, NULL);
+ if (IS_ERR(ptr))
+ return PTR_ERR(ptr);
- /*
- * @env->buflen is pointing to the char after '\0' now
- * override the '\0' to save MODALIAS value.
- */
- pos = env->buflen - 1;
- sl = of_modalias(dev->of_node, &env->buf[pos],
- sizeof(env->buf) - pos);
- if (sl < 0)
- return sl;
- if (sl >= (sizeof(env->buf) - pos))
- return -ENOMEM;
- env->buflen = pos + sl + 1;
+ ret = add_uevent_var(env, "MODALIAS=%s", ptr);
- return 0;
+ kfree(ptr);
+ return ret;
}
EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
diff --git a/drivers/of/module.c b/drivers/of/module.c
index 1e735fc130ad3ea9046f08bfab2cc9a07914e633..03a2b1b381e5b353b6699dac183c03186afb0486 100644
--- a/drivers/of/module.c
+++ b/drivers/of/module.c
@@ -8,71 +8,92 @@
#include <linux/slab.h>
#include <linux/string.h>
-ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len)
+/*
+ * of_modalias - get MODALIAS string value for a OF device node
+ * @np: the OF device node
+ * @lenp: MODALIAS string length returned if set, exclude '\0'
+ *
+ * This function gets MODALIAS value for a device node.
+ *
+ * Returns MODALIAS string on success, or ERR_PTR() on error.
+ *
+ * Note: please kfree successful return value afer using it.
+ */
+char *of_modalias(const struct device_node *np, ssize_t *lenp)
{
const char *compat;
char *c;
struct property *p;
ssize_t csize;
ssize_t tsize;
+ char *str = NULL;
+ ssize_t len = 0;
+ ssize_t pos = 0;
+ int counting = 1;
+
+ if (lenp)
+ *lenp = 0;
/*
- * 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...
+ * Two cycles controlled by @counting, the fist cycle counts
+ * chars, the second saves chars.
*/
- if ((len > 0 && !str) || len < 0)
- return -EINVAL;
+ do {
+ /* Name & Type */
+ /* %p eats all alphanum characters, so %c must be used here */
+ csize = snprintf(str + pos, len - pos, "of:N%pOFn%c%s", np, 'T',
+ of_node_get_device_type(np));
+ if (counting)
+ tsize = csize;
+ else
+ pos += csize;
- /* Name & Type */
- /* %p eats all alphanum characters, so %c must be used here */
- csize = snprintf(str, len, "of:N%pOFn%c%s", np, 'T',
- of_node_get_device_type(np));
- tsize = csize;
- if (csize >= len)
- csize = len > 0 ? len - 1 : 0;
- len -= csize;
- str += csize;
+ of_property_for_each_string(np, "compatible", p, compat) {
+ csize = snprintf(str + pos, len - pos, "C%s", compat);
+ if (counting) {
+ tsize += csize;
+ continue;
+ }
- of_property_for_each_string(np, "compatible", p, compat) {
- csize = snprintf(str, len, "C%s", compat);
- tsize += csize;
- if (csize >= len)
- continue;
- for (c = str; c; ) {
- c = strchr(c, ' ');
- if (c)
- *c++ = '_';
+ for (c = str + pos; c; ) {
+ c = strchr(c, ' ');
+ if (c)
+ *c++ = '_';
+ }
+ pos += csize;
}
- len -= csize;
- str += csize;
- }
- return tsize;
+ if (counting) {
+ /* Include '\0' of MODALIAS string. */
+ len = tsize + 1;
+ /* MODALIAS value is too long */
+ if (unlikely(len > 2048))
+ return ERR_PTR(-EINVAL);
+
+ str = kmalloc(len, GFP_KERNEL);
+ if (!str)
+ return ERR_PTR(-ENOMEM);
+ }
+
+ } while (counting--);
+
+ if (lenp)
+ *lenp = tsize;
+ return str;
}
int of_request_module(const struct device_node *np)
{
char *str;
- ssize_t size;
int ret;
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);
- if (!str)
- return -ENOMEM;
+ str = of_modalias(np, NULL);
+ if (IS_ERR(str))
+ return PTR_ERR(str);
- of_modalias(np, str, size);
- str[size - 1] = '\0';
ret = request_module(str);
kfree(str);
diff --git a/include/linux/of.h b/include/linux/of.h
index fe5d7b74c23b9701743f5debc3d030b765bc914f..f36bab2caa8ccffbd43593d8b6720946e19503e0 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);
+char *of_modalias(const struct device_node *np, ssize_t *lenp);
extern int of_request_module(const struct device_node *np);
/* phandle iterator functions */
@@ -761,10 +761,9 @@ 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 char *of_modalias(const struct device_node *np, ssize_t *lenp)
{
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
}
static inline int of_request_module(const struct device_node *np)
--
2.34.1
Powered by blists - more mailing lists