[Skiboot] [PATCH] core/device.c: Fix dt_find_compatible_node

[Alistair Popple]

dt_find_compatible_node() and dt_find_compatible_node_on_chip() are used to
find device nodes under a parent/root node with a given compatible
property.

dt_next(root, prev) is used to walk the child nodes of the given parent and
takes two arguments - root contains the parent node to walk whilst prev
contains the previous child to search from so that it can be used as an
iterator over all children nodes.

The first iteration of dt_find_compatible_node(root, prev) calls
dt_next(root, root) which is not a well defined operation as prev is
assumed to be child of the root node. The result is that when a node
contains no children it will start returning the parent nodes siblings
until it hits the top of the tree at which point a NULL derefence is
attempted when looking for the root nodes parent.

Dereferencing NULL can result in undesirable data exceptions during system
boot and untimely non-hilarious system crashes. dt_next() should not be
called with prev == root. Instead we add a check to dt_next() such that
passing prev = NULL will cause it to start iterating from the first child
node (if any).

Also add a unit test for this case to run-device.c.

Signed-off-by: Alistair Popple <alistair at popple.id.au>
---
 core/device.c          | 16 ++++++++++------
 core/test/run-device.c |  1 +
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/core/device.c b/core/device.c
index fc1db568..194eaef3 100644
--- a/core/device.c
+++ b/core/device.c
@@ -608,6 +608,12 @@ struct dt_node *dt_first(const struct dt_node *root)
 struct dt_node *dt_next(const struct dt_node *root,
 			const struct dt_node *prev)
 {
+	if (!prev)
+		prev = dt_first(root);
+
+	if (!prev)
+		return NULL;
+
 	/* Children? */
 	if (!list_empty(&prev->children))
 		return dt_first(prev);
@@ -719,10 +725,9 @@ struct dt_node *dt_find_compatible_node(struct dt_node *root,
 					struct dt_node *prev,
 					const char *compat)
 {
-	struct dt_node *node;
+	struct dt_node *node = prev;
 
-	node = prev ? dt_next(root, prev) : root;
-	for (; node; node = dt_next(root, node))
+	while ((node = dt_next(root, node)))
 		if (dt_node_is_compatible(node, compat))
 			return node;
 	return NULL;
@@ -964,10 +969,9 @@ struct dt_node *dt_find_compatible_node_on_chip(struct dt_node *root,
 						const char *compat,
 						uint32_t chip_id)
 {
-	struct dt_node *node;
+	struct dt_node *node = prev;
 
-	node = prev ? dt_next(root, prev) : root;
-	for (; node; node = dt_next(root, node)) {
+	while ((node = dt_next(root, node))) {
 		u32 cid = __dt_get_chip_id(node);
 		if (cid == chip_id &&
 		    dt_node_is_compatible(node, compat))
diff --git a/core/test/run-device.c b/core/test/run-device.c
index 326be3ff..6d3842e6 100644
--- a/core/test/run-device.c
+++ b/core/test/run-device.c
@@ -327,6 +327,7 @@ int main(void)
 	gc2 = dt_new(c1, "node-without-compatible");
 	assert(__dt_find_property(gc2, "compatible") == NULL);
 	assert(!dt_node_is_compatible(gc2, "any-property"));
+	assert(dt_find_compatible_node(gc2, NULL, "node-without-compatible") == NULL);
 
 	assert(dt_find_compatible_node(root, NULL, "generic-fake-bus") == c2);
 	assert(dt_find_compatible_node(root, c2, "generic-fake-bus") == NULL);
-- 
2.11.0