From: Paul Barker <pbarker-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
To: David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>,
Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>,
devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Paul Barker <pbarker-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>,
Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Pantelis Antoniou
<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>,
Scott Murray
<scott.murray-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>,
Jan Simon Moeller
<jsmoeller-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Subject: [PATCH] fdtoverlay: Prevent overlays from modifying phandle properties
Date: Sat, 19 Dec 2020 17:28:08 +0000 [thread overview]
Message-ID: <20201219172808.3101-1-pbarker@konsulko.com> (raw)
When applying an overlay fragment, we should take care not to overwrite
an existing phandle property of the target node as this could break
references to the target node elsewhere in the base dtb.
In addition to potentially breaking references within the resulting fdt,
if the overlay is built with symbols enabled (`-@` option to dtc) then
fdtoverlay will be unable to merge the overlay with a base dtb file.
A new test case is added to check how fdtoverlay handles this case.
Attempting to apply this test overlay without the fix in this patch
results in the following output:
input = tests/overlay_base_ref.test.dtb
output = tests/overlay_overlay_ref.fdtoverlay.dtb
overlay[0] = tests/overlay_overlay_ref.test.dtb
Failed to apply 'tests/overlay_overlay_ref.test.dtb': FDT_ERR_NOTFOUND
In this test case the __overlay__ node in question does not explicitly
contain a phandle property in the dts file, the phandle is added during
compilation as it is referenced by another node within the overlay dts.
This failure occurs due to a sequence of events in the functions called
by fdt_overlay_apply():
1) In overlay_fixup_phandles(), the target of the overlay fragment is
looked up and the target property is set to the phandle of the target
node.
2) In overlay_merge(), the target node is looked up by phandle via
overlay_get_target(). As the __overlay__ node in this test case
itself has a phandle property, the phandle of the target node is
modified.
3) In overlay_symbol_update(), the target node is again looked up by
phandle via overlay_get_target(). But this time the target node
cannot be found as its phandle property was modified.
The fix for this issue is to skip modification of the phandle property
of the target node in step (2) of the above sequence. If the target node
doesn't already contain a phandle property, we can add one without risk.
Signed-off-by: Paul Barker <pbarker-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
libfdt/fdt_overlay.c | 2 ++
tests/overlay_base_ref.dts | 19 +++++++++++++++++++
tests/overlay_overlay_ref.dts | 24 ++++++++++++++++++++++++
tests/run_tests.sh | 5 +++++
4 files changed, 50 insertions(+)
create mode 100644 tests/overlay_base_ref.dts
create mode 100644 tests/overlay_overlay_ref.dts
diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index d217e79..b3c217a 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -573,6 +573,8 @@ static int overlay_apply_node(void *fdt, int target,
if (prop_len < 0)
return prop_len;
+ if (!strcmp(name, "phandle") && fdt_getprop(fdt, target, name, NULL))
+ continue;
ret = fdt_setprop(fdt, target, name, prop, prop_len);
if (ret)
return ret;
diff --git a/tests/overlay_base_ref.dts b/tests/overlay_base_ref.dts
new file mode 100644
index 0000000..1fc02a2
--- /dev/null
+++ b/tests/overlay_base_ref.dts
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ * Copyright (c) 2016 Konsulko Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+/dts-v1/;
+
+/ {
+ test: test-node {
+ test-int-property = <42>;
+ };
+
+ test-refs {
+ refs = <&test>;
+ };
+};
diff --git a/tests/overlay_overlay_ref.dts b/tests/overlay_overlay_ref.dts
new file mode 100644
index 0000000..a45c95d
--- /dev/null
+++ b/tests/overlay_overlay_ref.dts
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ * Copyright (c) 2016 Konsulko Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+/dts-v1/;
+/plugin/;
+
+/ {
+ fragment@0 {
+ target = <&test>;
+
+ frag0: __overlay__ {
+ test-int-property = <43>;
+ };
+ };
+
+ test-ref {
+ ref = <&frag0>;
+ };
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 294585b..a65b166 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -329,6 +329,11 @@ dtc_overlay_tests () {
run_test check_path overlay_base_with_aliases.dtb not-exists "/__symbols__"
run_test check_path overlay_base_with_aliases.dtb not-exists "/__fixups__"
run_test check_path overlay_base_with_aliases.dtb not-exists "/__local_fixups__"
+
+ # Test taking a reference to an overlay fragment
+ run_dtc_test -@ -I dts -O dtb -o overlay_base_ref.test.dtb "$SRCDIR/overlay_base_ref.dts"
+ run_dtc_test -@ -I dts -O dtb -o overlay_overlay_ref.test.dtb "$SRCDIR/overlay_overlay_ref.dts"
+ run_wrap_test $FDTOVERLAY -i overlay_base_ref.test.dtb overlay_overlay_ref.test.dtb -o overlay_overlay_ref.fdtoverlay.dtb
}
tree1_tests () {
--
2.26.2
next reply other threads:[~2020-12-19 17:28 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-19 17:28 Paul Barker [this message]
[not found] ` <20201219172808.3101-1-pbarker-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2021-01-03 11:50 ` [PATCH] fdtoverlay: Prevent overlays from modifying phandle properties Paul Barker
[not found] ` <CAM9ZRVvBiqe1Su=KNQ0eFWURvnoi-s7rL9g4t-XFD4bxdMZ-ag-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2021-01-11 11:04 ` David Gibson
2021-02-17 5:25 ` David Gibson
[not found] ` <YCyo0CrHJiEaPEYA-l+x2Y8Cxqc4e6aEkudXLsA@public.gmane.org>
2021-02-19 9:28 ` Paul Barker
[not found] ` <CAM9ZRVsD9DfEHOH6B8P-dd_KAdOJpf1+GmcM0ph8x1b8ug=BDQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2021-02-22 6:47 ` David Gibson
[not found] ` <YDNTZW6bKT1bcJkX-l+x2Y8Cxqc4e6aEkudXLsA@public.gmane.org>
2021-02-22 8:39 ` Paul Barker
[not found] ` <CAM9ZRVuwsJsH0zJgBfBbUR9Vu8AEESJfvekV38aZM2gOX44ukw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2021-03-09 4:56 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201219172808.3101-1-pbarker@konsulko.com \
--to=pbarker-owpks81ov/fwk0htik3j/w@public.gmane.org \
--cc=david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org \
--cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=jdl-CYoMK+44s/E@public.gmane.org \
--cc=jsmoeller-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
--cc=pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org \
--cc=robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=scott.murray-OWPKS81ov/FWk0Htik3J/w@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).