* [PATCH 2/2] drivers/of: validate status properties in reconfig state changes
2026-04-03 7:32 [PATCH 1/2] drivers/of: validate live-tree string properties before string use Pengpeng Hou
@ 2026-04-03 7:33 ` Pengpeng Hou
2026-04-13 17:14 ` [PATCH 1/2] drivers/of: validate live-tree string properties before string use Rob Herring
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Pengpeng Hou @ 2026-04-03 7:33 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan; +Cc: devicetree, linux-kernel, pengpeng
Live-tree reconfiguration properties also carry raw values plus explicit
lengths. `of_reconfig_get_state_change()` currently treats `status`
property values as NUL-terminated strings and feeds them straight into
`strcmp()`.
Factor the `"okay"` / `"ok"` check out into a helper that first verifies
that the property contains a bounded C string within `prop->length`.
Malformed `status` updates should be treated as not enabling the node.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
drivers/of/dynamic.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 1a06175def37..efee59ed371a 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -74,6 +74,20 @@ static const char *action_names[] = {
[OF_RECONFIG_UPDATE_PROPERTY] = "UPDATE_PROPERTY",
};
+static bool of_property_status_ok(const struct property *prop)
+{
+ const char *status;
+
+ if (!prop || !prop->value || prop->length <= 0)
+ return false;
+
+ status = prop->value;
+ if (strnlen(status, prop->length) >= prop->length)
+ return false;
+
+ return !strcmp(status, "okay") || !strcmp(status, "ok");
+}
+
#define _do_print(func, prefix, action, node, prop, ...) ({ \
func("changeset: " prefix "%-15s %pOF%s%s\n", \
##__VA_ARGS__, action_names[action], node, \
@@ -135,11 +149,9 @@ int of_reconfig_get_state_change(unsigned long action, struct of_reconfig_data *
if (prop && !strcmp(prop->name, "status")) {
is_status = 1;
- status_state = !strcmp(prop->value, "okay") ||
- !strcmp(prop->value, "ok");
+ status_state = of_property_status_ok(prop);
if (old_prop)
- old_status_state = !strcmp(old_prop->value, "okay") ||
- !strcmp(old_prop->value, "ok");
+ old_status_state = of_property_status_ok(old_prop);
}
switch (action) {
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] drivers/of: validate live-tree string properties before string use
2026-04-03 7:32 [PATCH 1/2] drivers/of: validate live-tree string properties before string use Pengpeng Hou
2026-04-03 7:33 ` [PATCH 2/2] drivers/of: validate status properties in reconfig state changes Pengpeng Hou
@ 2026-04-13 17:14 ` Rob Herring
2026-04-17 3:06 ` Pengpeng Hou
2026-04-17 12:36 ` [PATCH v2 " Pengpeng Hou
3 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2026-04-13 17:14 UTC (permalink / raw)
To: Pengpeng Hou; +Cc: Saravana Kannan, devicetree, linux-kernel
On Fri, Apr 03, 2026 at 03:32:30PM +0800, Pengpeng Hou wrote:
> `populate_properties()` stores live-tree property values as raw byte
> sequences plus a separate `length`. They are not globally guaranteed to
> be NUL-terminated.
>
> `of_prop_next_string()` currently advances through string-list
> properties with `strlen()`, `__of_node_is_type()` compares raw
> `device_type` bytes with `strcmp()`, `__of_device_is_status()` compares
> raw `status` bytes with `strcmp()`/`strncmp()`, and
> `of_alias_from_compatible()` treats the first `compatible` entry as a
> NUL-terminated string.
>
> Validate these strings within their property bounds before treating
> them as C strings. In particular, reject malformed string-list entries
> whose next string is not terminated before `of_prop_next_string()`
> returns it.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
>
> ---
> drivers/of/base.c | 39 +++++++++++++++++++++++----------------
> drivers/of/property.c | 30 +++++++++++++++++++++++++-----
> 2 files changed, 48 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 57420806c1a2..3c6af4051ad3 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -82,7 +82,14 @@ EXPORT_SYMBOL(of_node_name_prefix);
>
> static bool __of_node_is_type(const struct device_node *np, const char *type)
> {
> - const char *match = __of_get_property(np, "device_type", NULL);
> + const char *match;
> + int matchlen;
> +
> + match = __of_get_property(np, "device_type", &matchlen);
> + if (!match || matchlen <= 0)
> + return false;
> + if (strnlen(match, matchlen) >= matchlen)
> + return false;
Can't we use of_property_match_string() instead?
>
> return np && match && type && !strcmp(match, type);
> }
> @@ -491,22 +498,22 @@ static bool __of_device_is_status(const struct device_node *device,
> return false;
>
> status = __of_get_property(device, "status", &statlen);
> - if (status == NULL)
> + if (!status || statlen <= 0)
> + return false;
> + if (strnlen(status, statlen) >= statlen)
> return false;
>
> - if (statlen > 0) {
> - while (*strings) {
> - unsigned int len = strlen(*strings);
> + while (*strings) {
> + unsigned int len = strlen(*strings);
>
> - if ((*strings)[len - 1] == '-') {
> - if (!strncmp(status, *strings, len))
> - return true;
> - } else {
> - if (!strcmp(status, *strings))
> - return true;
> - }
> - strings++;
> + if ((*strings)[len - 1] == '-') {
> + if (!strncmp(status, *strings, len))
> + return true;
> + } else {
> + if (!strcmp(status, *strings))
> + return true;
> }
> + strings++;
> }
>
> return false;
> @@ -1217,10 +1224,10 @@ EXPORT_SYMBOL(of_find_matching_node_and_match);
> int of_alias_from_compatible(const struct device_node *node, char *alias, int len)
> {
> const char *compatible, *p;
> - int cplen;
> + int ret;
>
> - compatible = of_get_property(node, "compatible", &cplen);
> - if (!compatible || strlen(compatible) > cplen)
> + ret = of_property_read_string_index(node, "compatible", 0, &compatible);
> + if (ret)
> return -ENODEV;
> p = strchr(compatible, ',');
> strscpy(alias, p ? p + 1 : compatible, len);
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 50d95d512bf5..edbc7a95aa4c 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -648,16 +648,36 @@ EXPORT_SYMBOL_GPL(of_prop_next_u32);
>
> const char *of_prop_next_string(const struct property *prop, const char *cur)
> {
> - const void *curv = cur;
> + const char *curv = cur;
> + const char *end;
> + size_t len;
>
> - if (!prop)
> + if (!prop || !prop->value || !prop->length)
> return NULL;
>
> - if (!cur)
> + end = prop->value + prop->length;
> +
> + if (!cur) {
> + len = strnlen(prop->value, prop->length);
> + if (len >= prop->length)
> + return NULL;
> +
> return prop->value;
You return here, but cur is still NULL. So we never advance. We should
have a test case in the unit test for this. If not, please add one.
> + }
>
> - curv += strlen(cur) + 1;
> - if (curv >= prop->value + prop->length)
> + if (cur < (const char *)prop->value || cur >= end)
> + return NULL;
> +
> + len = strnlen(cur, end - cur);
> + if (len >= end - cur)
> + return NULL;
> +
> + curv += len + 1;
> + if (curv >= end)
> + return NULL;
> +
> + len = strnlen(curv, end - curv);
Can we make this so we're not doing strnlen() on each string twice. If
return the current string, then 'cur' can point to the next string.
> + if (len >= end - curv)
> return NULL;
>
> return curv;
> --
> 2.50.1 (Apple Git-155)
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] drivers/of: validate live-tree string properties before string use
2026-04-03 7:32 [PATCH 1/2] drivers/of: validate live-tree string properties before string use Pengpeng Hou
2026-04-03 7:33 ` [PATCH 2/2] drivers/of: validate status properties in reconfig state changes Pengpeng Hou
2026-04-13 17:14 ` [PATCH 1/2] drivers/of: validate live-tree string properties before string use Rob Herring
@ 2026-04-17 3:06 ` Pengpeng Hou
2026-04-17 12:36 ` [PATCH v2 " Pengpeng Hou
3 siblings, 0 replies; 6+ messages in thread
From: Pengpeng Hou @ 2026-04-17 3:06 UTC (permalink / raw)
To: Rob Herring; +Cc: Saravana Kannan, devicetree, linux-kernel, pengpeng
Hi Rob,
Thanks, I'll respin this.
I'll switch the `device_type` match to `of_property_match_string()`,
rework `of_prop_next_string()` so the `cur == NULL` case flows through a
cleaner iterator path without validating each returned string twice, and
add a unittest for that first-iteration case.
Thanks,
Pengpeng
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] drivers/of: validate live-tree string properties before string use
2026-04-03 7:32 [PATCH 1/2] drivers/of: validate live-tree string properties before string use Pengpeng Hou
` (2 preceding siblings ...)
2026-04-17 3:06 ` Pengpeng Hou
@ 2026-04-17 12:36 ` Pengpeng Hou
2026-04-17 12:40 ` [PATCH v2 2/2] drivers/of: validate status properties in reconfig state changes Pengpeng Hou
3 siblings, 1 reply; 6+ messages in thread
From: Pengpeng Hou @ 2026-04-17 12:36 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan; +Cc: devicetree, linux-kernel, pengpeng
`populate_properties()` stores live-tree property values as raw byte
sequences plus a separate `length`. They are not globally guaranteed to
be NUL-terminated.
`of_prop_next_string()` iterates string-list properties by walking raw
bytes, `__of_node_is_type()` checks `device_type`,
`__of_device_is_status()` checks `status`, and
`of_alias_from_compatible()` reads the first `compatible` entry. These
paths must validate that the relevant string fits within the property
bounds before they hand it to C string helpers.
Validate these live-tree string properties within their declared bounds.
In particular, make `of_prop_next_string()` reject malformed entries
before returning them, use `of_property_match_string()` for
`device_type`, and add unit coverage for malformed first and trailing
string-list entries.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1:
- use of_property_match_string() for device_type as suggested by
Rob Herring
- rework of_prop_next_string() so the first returned string is validated
through the same bounded path
- add of_unittest_property_string() coverage for malformed first and
trailing string-list entries
drivers/of/base.c | 36 ++++++++++++----------
drivers/of/property.c | 27 +++++++++++++++++-----
drivers/of/unittest.c | 32 ++++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 23 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 57420806c1a2..96e4d7a7d5b8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -82,9 +82,10 @@ EXPORT_SYMBOL(of_node_name_prefix);
static bool __of_node_is_type(const struct device_node *np, const char *type)
{
- const char *match = __of_get_property(np, "device_type", NULL);
+ if (!np || !type)
+ return false;
- return np && match && type && !strcmp(match, type);
+ return of_property_match_string(np, "device_type", type) == 0;
}
#define EXCLUDED_DEFAULT_CELLS_PLATFORMS ( \
@@ -491,22 +492,22 @@ static bool __of_device_is_status(const struct device_node *device,
return false;
status = __of_get_property(device, "status", &statlen);
- if (status == NULL)
+ if (!status || statlen <= 0)
+ return false;
+ if (strnlen(status, statlen) >= statlen)
return false;
- if (statlen > 0) {
- while (*strings) {
- unsigned int len = strlen(*strings);
+ while (*strings) {
+ unsigned int len = strlen(*strings);
- if ((*strings)[len - 1] == '-') {
- if (!strncmp(status, *strings, len))
- return true;
- } else {
- if (!strcmp(status, *strings))
- return true;
- }
- strings++;
+ if ((*strings)[len - 1] == '-') {
+ if (!strncmp(status, *strings, len))
+ return true;
+ } else {
+ if (!strcmp(status, *strings))
+ return true;
}
+ strings++;
}
return false;
@@ -1217,10 +1218,11 @@ EXPORT_SYMBOL(of_find_matching_node_and_match);
int of_alias_from_compatible(const struct device_node *node, char *alias, int len)
{
const char *compatible, *p;
- int cplen;
+ int ret;
- compatible = of_get_property(node, "compatible", &cplen);
- if (!compatible || strlen(compatible) > cplen)
+ ret = of_property_read_string_index(node, "compatible", 0,
+ &compatible);
+ if (ret)
return -ENODEV;
p = strchr(compatible, ',');
strscpy(alias, p ? p + 1 : compatible, len);
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 50d95d512bf5..e97bfe357808 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -648,16 +648,31 @@ EXPORT_SYMBOL_GPL(of_prop_next_u32);
const char *of_prop_next_string(const struct property *prop, const char *cur)
{
- const void *curv = cur;
+ const char *curv;
+ const char *end;
+ size_t len;
- if (!prop)
+ if (!prop || !prop->value || !prop->length)
return NULL;
- if (!cur)
- return prop->value;
+ curv = cur ? cur : prop->value;
+ end = prop->value + prop->length;
- curv += strlen(cur) + 1;
- if (curv >= prop->value + prop->length)
+ if (curv < (const char *)prop->value || curv >= end)
+ return NULL;
+
+ if (cur) {
+ len = strnlen(curv, end - curv);
+ if (len >= end - curv)
+ return NULL;
+
+ curv += len + 1;
+ if (curv >= end)
+ return NULL;
+ }
+
+ len = strnlen(curv, end - curv);
+ if (len >= end - curv)
return NULL;
return curv;
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 29402958f11c..ee53363dfa84 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -713,6 +713,7 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
static void __init of_unittest_property_string(void)
{
const char *strings[4];
+ const struct property *prop;
struct device_node *np;
int rc;
@@ -789,6 +790,37 @@ static void __init of_unittest_property_string(void)
strings[1] = NULL;
rc = of_property_read_string_array(np, "phandle-list-names", strings, 1);
unittest(rc == 1 && strings[1] == NULL, "Overwrote end of string array; rc=%i, str='%s'\n", rc, strings[1]);
+
+ /* of_prop_next_string() tests */
+ prop = of_find_property(np, "phandle-list-names", NULL);
+ strings[0] = of_prop_next_string(prop, NULL);
+ unittest(strings[0] && !strcmp(strings[0], "first"),
+ "of_prop_next_string() failure; got '%s'\n", strings[0]);
+ strings[0] = of_prop_next_string(prop, strings[0]);
+ unittest(strings[0] && !strcmp(strings[0], "second"),
+ "of_prop_next_string() failure; got '%s'\n", strings[0]);
+ strings[0] = of_prop_next_string(prop, strings[0]);
+ unittest(strings[0] && !strcmp(strings[0], "third"),
+ "of_prop_next_string() failure; got '%s'\n", strings[0]);
+ strings[0] = of_prop_next_string(prop, strings[0]);
+ unittest(!strings[0],
+ "of_prop_next_string() should return NULL at end of list\n");
+
+ prop = of_find_property(np, "unterminated-string", NULL);
+ strings[0] = of_prop_next_string(prop, NULL);
+ unittest(!strings[0],
+ "of_prop_next_string() should reject unterminated first string\n");
+
+ prop = of_find_property(np, "unterminated-string-list", NULL);
+ strings[0] = of_prop_next_string(prop, NULL);
+ unittest(strings[0] && !strcmp(strings[0], "first"),
+ "of_prop_next_string() failure; got '%s'\n", strings[0]);
+ strings[0] = of_prop_next_string(prop, strings[0]);
+ unittest(strings[0] && !strcmp(strings[0], "second"),
+ "of_prop_next_string() failure; got '%s'\n", strings[0]);
+ strings[0] = of_prop_next_string(prop, strings[0]);
+ unittest(!strings[0],
+ "of_prop_next_string() should reject unterminated trailing string\n");
}
#define propcmp(p1, p2) (((p1)->length == (p2)->length) && \
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] drivers/of: validate status properties in reconfig state changes
2026-04-17 12:36 ` [PATCH v2 " Pengpeng Hou
@ 2026-04-17 12:40 ` Pengpeng Hou
0 siblings, 0 replies; 6+ messages in thread
From: Pengpeng Hou @ 2026-04-17 12:40 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan; +Cc: devicetree, linux-kernel, pengpeng
Live-tree reconfiguration properties also carry raw values plus explicit
lengths. `of_reconfig_get_state_change()` currently treats `status`
property values as NUL-terminated strings and feeds them straight into
`strcmp()`.
Factor the `"okay"` / `"ok"` check out into a helper that first verifies
that the property contains a bounded C string within `prop->length`.
Malformed `status` updates should be treated as not enabling the node.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1:
- no change; carried in v2 because patch 1/2 was reworked
drivers/of/dynamic.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 1a06175def37..efee59ed371a 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -74,6 +74,20 @@ static const char *action_names[] = {
[OF_RECONFIG_UPDATE_PROPERTY] = "UPDATE_PROPERTY",
};
+static bool of_property_status_ok(const struct property *prop)
+{
+ const char *status;
+
+ if (!prop || !prop->value || prop->length <= 0)
+ return false;
+
+ status = prop->value;
+ if (strnlen(status, prop->length) >= prop->length)
+ return false;
+
+ return !strcmp(status, "okay") || !strcmp(status, "ok");
+}
+
#define _do_print(func, prefix, action, node, prop, ...) ({ \
func("changeset: " prefix "%-15s %pOF%s%s\n", \
##__VA_ARGS__, action_names[action], node, \
@@ -135,11 +149,9 @@ int of_reconfig_get_state_change(unsigned long action, struct of_reconfig_data *
if (prop && !strcmp(prop->name, "status")) {
is_status = 1;
- status_state = !strcmp(prop->value, "okay") ||
- !strcmp(prop->value, "ok");
+ status_state = of_property_status_ok(prop);
if (old_prop)
- old_status_state = !strcmp(old_prop->value, "okay") ||
- !strcmp(old_prop->value, "ok");
+ old_status_state = of_property_status_ok(old_prop);
}
switch (action) {
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 6+ messages in thread