[PATCH 1/3] checks: Fix segmentation fault in check_graph_node

[PATCH 1/3] checks: Fix segmentation fault in check_graph_node

[Johannes Beisswenger]

Dts files which contain an 'endpoint' node as a direct child of the
root node cause a segmentation fault inside check_graph_node(). This
type of error can easily happen when a 'remote-endpoint' property is
accidentally placed outside the corresponding endpoint and port nodes.

Example with 'endpoint' node:
/dts-v1/;
/ {	endpoint {};  };

Example with remote-endpoint property:
/dts-v1/;
/ {
	foo {
                remote-endpoint = <0xdeadbeef>;
	};
};

Signed-off-by: Johannes Beisswenger <johannes.beisswenger-QHh2hmjso9JBDgjK7y7TUQ@public.gmane.org>
---

Hello,

this patch series is equivalent to the following GitHub pull request https://github.com/dgibson/dtc/pull/92
which also contains a short summary of the issue.
In short: the segmentation fault happens because check_graph_node() dereferences
node->parent without checking whether it is NULL.

The actual code changes are quite small, nevertheless please let me know if there are any issues.

Best regards
Johannes Beisswenger

CETITEC GmbH
Mannheimer Str. 17
D-75179 Pforzheim, Germany 


 checks.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/checks.c b/checks.c
index 16bc7f6..8ed7a60 100644
--- a/checks.c
+++ b/checks.c
@@ -1767,6 +1767,11 @@ static void check_graph_nodes(struct check *c, struct dt_info *dti,
 		      get_property(child, "remote-endpoint")))
 			continue;
 
+                /* The root node cannot be a port */
+		if (!node->parent) {
+			FAIL(c, dti, node, "root node contains endpoint node '%s', potentially misplaced remote-endpoint property", child->name);
+			continue;
+		}
 		node->bus = &graph_port_bus;
 
 		/* The parent of 'port' nodes can be either 'ports' or a device */
-- 
2.40.0

[PATCH 2/3] tests: Add test cases for bad endpoint node and remote-endpoint prop checks

[Johannes Beisswenger]

Signed-off-by: Johannes Beisswenger <johannes.beisswenger-QHh2hmjso9JBDgjK7y7TUQ@public.gmane.org>
---
 tests/bad-graph-root1.dts |  7 +++++++
 tests/bad-graph-root2.dts |  2 ++
 tests/bad-graph-root3.dts | 14 ++++++++++++++
 tests/bad-graph-root4.dts | 19 +++++++++++++++++++
 tests/run_tests.sh        |  4 ++++
 5 files changed, 46 insertions(+)
 create mode 100644 tests/bad-graph-root1.dts
 create mode 100644 tests/bad-graph-root2.dts
 create mode 100644 tests/bad-graph-root3.dts
 create mode 100644 tests/bad-graph-root4.dts

diff --git a/tests/bad-graph-root1.dts b/tests/bad-graph-root1.dts
new file mode 100644
index 0000000..06cf915
--- /dev/null
+++ b/tests/bad-graph-root1.dts
@@ -0,0 +1,7 @@
+/dts-v1/;
+
+/ {
+	foo {
+                remote-endpoint = <0xdeadbeef>;
+	};
+};
diff --git a/tests/bad-graph-root2.dts b/tests/bad-graph-root2.dts
new file mode 100644
index 0000000..026f207
--- /dev/null
+++ b/tests/bad-graph-root2.dts
@@ -0,0 +1,2 @@
+/dts-v1/;
+/ {	endpoint {};  };
diff --git a/tests/bad-graph-root3.dts b/tests/bad-graph-root3.dts
new file mode 100644
index 0000000..7360013
--- /dev/null
+++ b/tests/bad-graph-root3.dts
@@ -0,0 +1,14 @@
+/dts-v1/;
+
+ / {
+	bar: bar {
+		port {
+			bar_con: endpoint {
+				remote-endpoint = <&foo_con>;
+			};
+		};
+	};
+	foo_con: endpoint {
+		remote-endpoint = <&bar_con>;
+	};
+};
diff --git a/tests/bad-graph-root4.dts b/tests/bad-graph-root4.dts
new file mode 100644
index 0000000..5b1a1ea
--- /dev/null
+++ b/tests/bad-graph-root4.dts
@@ -0,0 +1,19 @@
+/dts-v1/;
+
+ / {
+
+	bar: bar {
+		port {
+			bar_con: endpoint {
+				remote-endpoint = <&foo_con>;
+			};
+		};
+	};
+	foo {
+		remote-endpoint = <&bar_con>; // misplaced remote-endpoint property
+		port {
+			foo_con: endpoint {
+			};
+		};
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index f899d8c..2af8c15 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -730,6 +730,10 @@ dtc_tests () {
     check_tests "$SRCDIR/bad-graph.dts" graph_child_address
     check_tests "$SRCDIR/bad-graph.dts" graph_port
     check_tests "$SRCDIR/bad-graph.dts" graph_endpoint
+    check_tests "$SRCDIR/bad-graph-root1.dts" graph_nodes
+    check_tests "$SRCDIR/bad-graph-root2.dts" graph_nodes
+    check_tests "$SRCDIR/bad-graph-root3.dts" graph_nodes
+    check_tests "$SRCDIR/bad-graph-root4.dts" graph_nodes
     run_sh_test "$SRCDIR/dtc-checkfails.sh" deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb "$SRCDIR/bad-gpio.dts"
     run_sh_test "$SRCDIR/dtc-checkfails.sh" -n deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb "$SRCDIR/good-gpio.dts"
     check_tests "$SRCDIR/bad-interrupt-cells.dts" interrupts_property
-- 
2.40.0

[PATCH 3/3] checks: Fix crash in graph_child_address if 'reg' cell size > 1

[Johannes Beisswenger]

If an endpoint node has a 'reg' property which consists of more than
one cell and given that matching '#address-cells' and '#size-cells'
properties are specified on the port node an assertion is triggered in
check_graph_child_address() before the relevant diagnostic checks in
check_graph_reg() (called by check_graph_port()) are executed.

Example dts file triggering the issue:

/dts-v1/;
 / {
	bar: bar {
		port {
			bar_con: endpoint {
				remote-endpoint = <&foo_con>;
			};
		};
	};
	foo {
		port {
			#address-cells = <1>;
			#size-cells = <1>; // should always be 0
			foo_con: endpoint@1 {
				reg = <1 2>; // causes assertion failure instead of diagnostic
				remote-endpoint = <&bar_con>;
			};
		};
	};
};

Signed-off-by: Johannes Beisswenger <johannes.beisswenger-QHh2hmjso9JBDgjK7y7TUQ@public.gmane.org>
---
 checks.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/checks.c b/checks.c
index 8ed7a60..4648c44 100644
--- a/checks.c
+++ b/checks.c
@@ -1798,8 +1798,10 @@ static void check_graph_child_address(struct check *c, struct dt_info *dti,
 		struct property *prop = get_property(child, "reg");
 
 		/* No error if we have any non-zero unit address */
-		if (prop && propval_cell(prop) != 0)
+		/* We have a check for val.len == sizeof(cell_t) elsewhere */
+                if (prop && propval_cell_n(prop, 0)  != 0 ) {
 			return;
+                }
 
 		cnt++;
 	}
-- 
2.40.0