libfdt: overlay: ensure that existing phandles are not overwritten

A phandle in an overlay is not supposed to overwrite a phandle that already exists in the base dtb as this breaks references to the respective node in the base. So add another iteration over the fdto that checks for such overwrites and fixes the fdto phandle's value to match the fdt's. A test is added that checks that newly added phandles and existing phandles work as expected. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Message-ID: <20240225175422.156393-2-u.kleine-koenig@pengutronix.de> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
8 months ago · 1fad065080
4 changed files with 334 additions and 0 deletions
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@ -498,6 +498,249 @@ static int overlay_fixup_phandles(void *fdt, void *fdto)
				@@ -498,6 +498,249 @@ static int overlay_fixup_phandles(void *fdt, void *fdto)
 	return 0;
 }

+/**
+ * overlay_adjust_local_conflicting_phandle: Changes a phandle value
+ * @fdto: Device tree overlay
+ * @node: The node the phandle is set for
+ * @fdt_phandle: The new value for the phandle
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_adjust_local_conflicting_phandle(void *fdto, int node,
+						    uint32_t fdt_phandle)
+{
+	const fdt32_t *php;
+	int len, ret;
+
+	php = fdt_getprop(fdto, node, "phandle", &len);
+	if (php && len == sizeof(*php)) {
+		ret = fdt_setprop_inplace_u32(fdto, node, "phandle", fdt_phandle);
+		if (ret)
+			return ret;
+	}
+
+	php = fdt_getprop(fdto, node, "linux,phandle", &len);
+	if (php && len == sizeof(*php)) {
+		ret = fdt_setprop_inplace_u32(fdto, node, "linux,phandle", fdt_phandle);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * overlay_update_node_conflicting_references - Recursively replace phandle values
+ * @fdto: Device tree overlay blob
+ * @tree_node: Node to recurse into
+ * @fixup_node: Node offset of the matching local fixups node
+ * @fdt_phandle: Value to replace phandles with
+ * @fdto_phandle: Value to be replaced
+ *
+ * Replaces all phandles with value @fdto_phandle by @fdt_phandle.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_update_node_conflicting_references(void *fdto, int tree_node,
+						      int fixup_node,
+						      uint32_t fdt_phandle,
+						      uint32_t fdto_phandle)
+{
+	int fixup_prop;
+	int fixup_child;
+	int ret;
+
+	fdt_for_each_property_offset(fixup_prop, fdto, fixup_node) {
+		const fdt32_t *fixup_val;
+		const char *name;
+		char *tree_val;
+		int fixup_len;
+		int tree_len;
+		int i;
+
+		fixup_val = fdt_getprop_by_offset(fdto, fixup_prop,
+						  &name, &fixup_len);
+		if (!fixup_val)
+			return fixup_len;
+
+		if (fixup_len % sizeof(uint32_t))
+			return -FDT_ERR_BADOVERLAY;
+		fixup_len /= sizeof(uint32_t);
+
+		tree_val = fdt_getprop_w(fdto, tree_node, name, &tree_len);
+		if (!tree_val) {
+			if (tree_len == -FDT_ERR_NOTFOUND)
+				return -FDT_ERR_BADOVERLAY;
+
+			return tree_len;
+		}
+
+		for (i = 0; i < fixup_len; i++) {
+			fdt32_t *refp;
+			uint32_t valp;
+
+			refp = (fdt32_t *)(tree_val + fdt32_ld_(fixup_val + i));
+			valp = fdt32_ld(refp);
+
+			if (valp == fdto_phandle)
+				fdt32_st(refp, fdt_phandle);
+		}
+	}
+
+	fdt_for_each_subnode(fixup_child, fdto, fixup_node) {
+		const char *fixup_child_name = fdt_get_name(fdto, fixup_child, NULL);
+		int tree_child;
+
+		tree_child = fdt_subnode_offset(fdto, tree_node, fixup_child_name);
+
+		if (tree_child == -FDT_ERR_NOTFOUND)
+			return -FDT_ERR_BADOVERLAY;
+		if (tree_child < 0)
+			return tree_child;
+
+		ret = overlay_update_node_conflicting_references(fdto, tree_child,
+								 fixup_child,
+								 fdt_phandle,
+								 fdto_phandle);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * overlay_update_local_conflicting_references - Recursively replace phandle values
+ * @fdto: Device tree overlay blob
+ * @fdt_phandle: Value to replace phandles with
+ * @fdto_phandle: Value to be replaced
+ *
+ * Replaces all phandles with value @fdto_phandle by @fdt_phandle.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_update_local_conflicting_references(void *fdto,
+						       uint32_t fdt_phandle,
+						       uint32_t fdto_phandle)
+{
+	int fixups;
+
+	fixups = fdt_path_offset(fdto, "/__local_fixups__");
+	if (fixups == -FDT_ERR_NOTFOUND)
+		return 0;
+	if (fixups < 0)
+		return fixups;
+
+	return overlay_update_node_conflicting_references(fdto, 0, fixups,
+							  fdt_phandle,
+							  fdto_phandle);
+}
+
+/**
+ * overlay_prevent_phandle_overwrite_node - Helper function for overlay_prevent_phandle_overwrite
+ * @fdt: Base Device tree blob
+ * @fdtnode: Node in fdt that is checked for an overwrite
+ * @fdto: Device tree overlay blob
+ * @fdtonode: Node in fdto matching @fdtnode
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_prevent_phandle_overwrite_node(void *fdt, int fdtnode,
+						  void *fdto, int fdtonode)
+{
+	uint32_t fdt_phandle, fdto_phandle;
+	int fdtochild;
+
+	fdt_phandle = fdt_get_phandle(fdt, fdtnode);
+	fdto_phandle = fdt_get_phandle(fdto, fdtonode);
+
+	if (fdt_phandle && fdto_phandle) {
+		int ret;
+
+		ret = overlay_adjust_local_conflicting_phandle(fdto, fdtonode,
+							       fdt_phandle);
+		if (ret)
+			return ret;
+
+		ret = overlay_update_local_conflicting_references(fdto,
+								  fdt_phandle,
+								  fdto_phandle);
+		if (ret)
+			return ret;
+	}
+
+	fdt_for_each_subnode(fdtochild, fdto, fdtonode) {
+		const char *name = fdt_get_name(fdto, fdtochild, NULL);
+		int fdtchild;
+		int ret;
+
+		fdtchild = fdt_subnode_offset(fdt, fdtnode, name);
+		if (fdtchild == -FDT_ERR_NOTFOUND)
+			/*
+			 * no further overwrites possible here as this node is
+			 * new
+			 */
+			continue;
+
+		ret = overlay_prevent_phandle_overwrite_node(fdt, fdtchild,
+							     fdto, fdtochild);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * overlay_prevent_phandle_overwrite - Fixes overlay phandles to not overwrite base phandles
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ *
+ * Checks recursively if applying fdto overwrites phandle values in the base
+ * dtb. When such a phandle is found, the fdto is changed to use the fdt's
+ * phandle value to not break references in the base.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_prevent_phandle_overwrite(void *fdt, void *fdto)
+{
+	int fragment;
+
+	fdt_for_each_subnode(fragment, fdto, 0) {
+		int overlay;
+		int target;
+		int ret;
+
+		overlay = fdt_subnode_offset(fdto, fragment, "__overlay__");
+		if (overlay == -FDT_ERR_NOTFOUND)
+			continue;
+
+		if (overlay < 0)
+			return overlay;
+
+		target = fdt_overlay_target_offset(fdt, fdto, fragment, NULL);
+		if (target < 0)
+			return target;
+
+		ret = overlay_prevent_phandle_overwrite_node(fdt, target,
+							     fdto, overlay);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 /**
 * overlay_apply_node - Merges a node into the base device tree
 * @fdt: Base Device Tree blob
@ -802,18 +1045,26 @@ int fdt_overlay_apply(void *fdt, void *fdto)
				@@ -802,18 +1045,26 @@ int fdt_overlay_apply(void *fdt, void *fdto)
 	if (ret)
 		goto err;

+	/* Increase all phandles in the fdto by delta */
 	ret = overlay_adjust_local_phandles(fdto, delta);
 	if (ret)
 		goto err;

+	/* Adapt the phandle values in fdto to the above increase */
 	ret = overlay_update_local_references(fdto, delta);
 	if (ret)
 		goto err;

+	/* Update fdto's phandles using symbols from fdt */
 	ret = overlay_fixup_phandles(fdt, fdto);
 	if (ret)
 		goto err;

+	/* Don't overwrite phandles in fdt */
+	ret = overlay_prevent_phandle_overwrite(fdt, fdto);
+	if (ret)
+		goto err;
+
 	ret = overlay_merge(fdt, fdto);
 	if (ret)
 		goto err;
--- a/tests/overlay_base_phandle.dts
+++ b/tests/overlay_base_phandle.dts
@ -0,0 +1,21 @@
				@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2024 Uwe Kleine-König
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/dts-v1/;
+
+/ {
+	node_a: a {
+		value = <17>;
+	};
+
+	node_b: b {
+		a = <&node_a>;
+	};
+
+	node_d: d {
+		b = <&node_b>;
+	};
+};
--- a/tests/overlay_overlay_phandle.dts
+++ b/tests/overlay_overlay_phandle.dts
@ -0,0 +1,34 @@
				@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2024 Uwe Kleine-König
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/dts-v1/;
+/plugin/;
+
+&{/} {
+	/*
+	 * /b already has a label "node_b" in the base dts, also b is already
+	 * referenced in the base, so both the base's b and this b have a
+	 * phandle value.
+	 */
+	node_b2: b {
+		value = <42>;
+		d = <&node_d>;
+	};
+
+	node_c: c {
+		value = <23>;
+		b = <&node_b2>;
+	};
+
+	d {
+		a = <&node_a>;
+		c = <&node_c>;
+	};
+};
+
+&node_a {
+	value = <32>;
+};
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@ -1040,6 +1040,34 @@ fdtoverlay_tests() {
				@@ -1040,6 +1040,34 @@ fdtoverlay_tests() {
    run_dtc_test -@ -I dts -O dtb -o $stacked_addlabeldtb $stacked_addlabel

    run_fdtoverlay_test baz "/foonode/barnode/baznode" "baz-property" "-ts" ${stacked_base_nolabeldtb} ${stacked_addlabel_targetdtb} ${stacked_addlabeldtb} ${stacked_bardtb} ${stacked_bazdtb}
+
+    # verify that phandles are not overwritten
+    run_dtc_test -@ -I dts -O dtb -o overlay_base_phandle.test.dtb "$SRCDIR/overlay_base_phandle.dts"
+    run_dtc_test -@ -I dts -O dtb -o overlay_overlay_phandle.test.dtb "$SRCDIR/overlay_overlay_phandle.dts"
+    run_wrap_test $FDTOVERLAY -i overlay_base_phandle.test.dtb -o overlay_base_phandleO.test.dtb overlay_overlay_phandle.test.dtb
+
+    pa=$($DTGET overlay_base_phandleO.test.dtb /a phandle)
+    pb=$($DTGET overlay_base_phandleO.test.dtb /b phandle)
+    pc=$($DTGET overlay_base_phandleO.test.dtb /c phandle)
+    pd=$($DTGET overlay_base_phandleO.test.dtb /d phandle)
+    ba=$($DTGET overlay_base_phandleO.test.dtb /b a)
+    bd=$($DTGET overlay_base_phandleO.test.dtb /b d)
+    cb=$($DTGET overlay_base_phandleO.test.dtb /c b)
+    da=$($DTGET overlay_base_phandleO.test.dtb /d a)
+    db=$($DTGET overlay_base_phandleO.test.dtb /d b)
+    dc=$($DTGET overlay_base_phandleO.test.dtb /d c)
+
+    shorten_echo "check phandle to noda a wasn't overwritten:	"
+    run_wrap_test test "$ba-$da" = "$pa-$pa"
+
+    shorten_echo "check phandle to noda b wasn't overwritten:	"
+    run_wrap_test test "$cb-$db" = "$pb-$pb"
+
+    shorten_echo "check phandle to noda c wasn't overwritten:	"
+    run_wrap_test test "$dc" = "$pc"
+
+    shorten_echo "check phandle to noda d wasn't overwritten:	"
+    run_wrap_test test "$bd" = "$pd"
 }

 pylibfdt_tests () {