From 6c25e96cf6d6969d580f178e9e48f396249b50b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Wed, 11 Oct 2023 15:45:17 +0100 Subject: ANDROID: Use -Wno-macro-redefined for DTC tools As we use the flag when building libfdt, also enable it for the tools. Test: m dtc fdtget fdtoverlay Test: bazel build //:all Change-Id: I3c0775436bb9be1367e1937e99a6e87d1dce664c --- Android.bp | 1 + BUILD.bazel | 1 + 2 files changed, 2 insertions(+) diff --git a/Android.bp b/Android.bp index cb13748..a194eef 100644 --- a/Android.bp +++ b/Android.bp @@ -48,6 +48,7 @@ cc_defaults { cflags: [ "-Wall", "-Werror", + "-Wno-macro-redefined", "-Wno-sign-compare", "-Wno-missing-field-initializers", "-Wno-unused-parameter", diff --git a/BUILD.bazel b/BUILD.bazel index 439621d..5ef46e7 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -15,6 +15,7 @@ cc_library( COPTS = [ "-Wall", "-Werror", + "-Wno-macro-redefined", "-Wno-sign-compare", "-Wno-missing-field-initializers", "-Wno-unused-parameter", -- cgit v1.2.3 From 170bf1d4ce7ba36cc7b8b0f606ae9c169ea9a769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Wed, 11 Oct 2023 15:50:39 +0100 Subject: ANDROID: Use -Wall and more when building libfdt Build libfdt with the same warnings as the DTC tools. This effectively starts using -Wno-missing-field-initializers, -Wno-unused-parameter, and -Wall. Test: m libfdt Test: bazel build //:libfdt Change-Id: I5a2ae1ee86b4613d0aa7a0174d235976d7be45cc --- Android.bp | 11 +++++++++-- BUILD.bazel | 24 ++++++++++-------------- libfdt/Android.bp | 7 +------ 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/Android.bp b/Android.bp index a194eef..85c1b22 100644 --- a/Android.bp +++ b/Android.bp @@ -44,14 +44,21 @@ license { } cc_defaults { - name: "dt_defaults", + name: "dtc_cflags_defaults", cflags: [ "-Wall", "-Werror", "-Wno-macro-redefined", - "-Wno-sign-compare", "-Wno-missing-field-initializers", + "-Wno-sign-compare", "-Wno-unused-parameter", + ], +} + +cc_defaults { + name: "dt_defaults", + defaults: ["dtc_cflags_defaults"], + cflags: [ "-DNO_YAML" ], diff --git a/BUILD.bazel b/BUILD.bazel index 5ef46e7..d9fdb9a 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -1,26 +1,22 @@ +COPTS = [ + "-Wall", + "-Werror", + "-Wno-macro-redefined", + "-Wno-missing-field-initializers", + "-Wno-sign-compare", + "-Wno-unused-parameter", +] + cc_library( name = "libfdt", srcs = glob([ "libfdt/*.h", "libfdt/*.c", ]), - copts = [ - "-Werror", - "-Wno-macro-redefined", - "-Wno-sign-compare", - ], + copts = COPTS, includes = ["libfdt"], ) -COPTS = [ - "-Wall", - "-Werror", - "-Wno-macro-redefined", - "-Wno-sign-compare", - "-Wno-missing-field-initializers", - "-Wno-unused-parameter", -] - genrule( name = "lexer", srcs = [ diff --git a/libfdt/Android.bp b/libfdt/Android.bp index 8f4bdfd..0bf631a 100644 --- a/libfdt/Android.bp +++ b/libfdt/Android.bp @@ -7,12 +7,7 @@ package { cc_library { name: "libfdt", host_supported: true, - - cflags: [ - "-Werror", - "-Wno-macro-redefined", - "-Wno-sign-compare", - ], + defaults: ["dtc_cflags_defaults"], srcs: [ "fdt.c", "fdt_check.c", -- cgit v1.2.3 From 6127032bf7a8477115afb15c66fcefe86ed3601f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Wed, 11 Oct 2023 16:37:49 +0100 Subject: ANDROID: bazel: Fix dtc header dependencies As dtc compiles util.c, which includes util.h and version_non_gen.h (or version_gen.h upstream), the cc_binary should list those dependencies explicitly so that a change in them triggers a rebuild. Furthermore, dtc also depends on dtc.h and srcpos.h, which are only found as dependencies by Bazel through the overkill glob(["*.h"]) header list of dtc_gen so list them explicitly here so that that cc_library can be dropped. Introduce the UTILS list to DRY the build file. Test: bazel build //:dtc Change-Id: I32076c65b60820dc91f6dc84c3ee3cad310c1db5 --- BUILD.bazel | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 439621d..8a65ba8 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -55,18 +55,25 @@ cc_library( deps = [":libfdt"], ) +UTILS = [ + "util.c", + "util.h", + "version_non_gen.h", +] + cc_binary( name = "dtc", - srcs = [ + srcs = UTILS + [ "checks.c", "data.c", "dtc.c", + "dtc.h", "flattree.c", "fstree.c", "livetree.c", "srcpos.c", + "srcpos.h", "treesource.c", - "util.c", ], copts = COPTS, defines = ["NO_YAML"], @@ -78,11 +85,8 @@ cc_binary( cc_binary( name = "fdtget", - srcs = [ + srcs = UTILS + [ "fdtget.c", - "util.c", - "util.h", - "version_non_gen.h", ], copts = COPTS, defines = ["NO_YAML"], @@ -91,11 +95,8 @@ cc_binary( cc_binary( name = "fdtput", - srcs = [ + srcs = UTILS + [ "fdtput.c", - "util.c", - "util.h", - "version_non_gen.h", ], copts = COPTS, defines = ["NO_YAML"], @@ -104,11 +105,8 @@ cc_binary( cc_binary( name = "fdtdump", - srcs = [ + srcs = UTILS + [ "fdtdump.c", - "util.c", - "util.h", - "version_non_gen.h", ], copts = COPTS, defines = ["NO_YAML"], @@ -117,11 +115,8 @@ cc_binary( cc_binary( name = "fdtoverlay", - srcs = [ + srcs = UTILS + [ "fdtoverlay.c", - "util.c", - "util.h", - "version_non_gen.h", ], copts = COPTS, defines = ["NO_YAML"], -- cgit v1.2.3 From c8d1d863ab72b74efe7bda4c80579968716708a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Wed, 11 Oct 2023 16:37:55 +0100 Subject: ANDROID: bazel: Clean up lexer and parser rules Simplify the build file as - dtc_gen creates an unnecessarily broad (and confusing) dependency list by using glob([".h"]) as its header list. Instead, dtc now lists explicitly the few headers it actually needs; - generating dtc-lexer.lex.c does not require dtc-parser.{c,h}; - Bison can be told to directly create dtc-parser.{c,h} so no need for an unnecessarily broad copying of *.[ch] to move some intermediate results. As a result, we get two genrule() wrapping the source files respectively generated through lex and bison, which can be listed as srcs of dtc. Test: bazel build //:all Change-Id: I238d963af8a338c46f39c8ba9e4314fe536948cf --- BUILD.bazel | 35 ++++++++--------------------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 8a65ba8..634115c 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -21,38 +21,20 @@ COPTS = [ ] genrule( - name = "lexer", - srcs = [ - "dtc-lexer.l", - ":parser", - ], + name = "dtc_lexer_srcs", + srcs = ["dtc-lexer.l"], outs = ["dtc-lexer.lex.c"], - cmd = "lex -o$@ $(location dtc-lexer.l)", + cmd = "lex -o $@ $<", ) genrule( - name = "parser", + name = "dtc_parser_srcs", srcs = ["dtc-parser.y"], outs = [ "dtc-parser.c", "dtc-parser.h", ], - cmd = """ - bison -b dtc-parser -d $(location dtc-parser.y) - cp ./*.c $(location dtc-parser.c) - cp ./*.h $(location dtc-parser.h) - """, -) - -cc_library( - name = "dtc_gen", - srcs = [ - ":lexer", - ":parser", - ], - hdrs = glob(["*.h"]), - copts = COPTS, - deps = [":libfdt"], + cmd = "bison -d -o $(location dtc-parser.c) $(location dtc-parser.y)", ) UTILS = [ @@ -64,6 +46,8 @@ UTILS = [ cc_binary( name = "dtc", srcs = UTILS + [ + ":dtc_lexer_srcs", + ":dtc_parser_srcs", "checks.c", "data.c", "dtc.c", @@ -77,10 +61,7 @@ cc_binary( ], copts = COPTS, defines = ["NO_YAML"], - deps = [ - ":dtc_gen", - ":libfdt", - ], + deps = [":libfdt"], ) cc_binary( -- cgit v1.2.3 From 10b6dbab709bf16f3a5f497b273819cafc9286bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Wed, 11 Oct 2023 16:58:19 +0100 Subject: ANDROID: Revert "libfdt: fdt_path_offset_namelen: Reject empty paths" This reverts commit 3c28f3e3a1724c288d19f1b1a139cf57bfe1af33. Now that this change has been upstreamed, revert the Android patch in order to cherry-pick the upstream one. Test: N/A Change-Id: I5295898fa6670436533290f32a10e3f672e2696c --- libfdt/fdt_ro.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index 362cc4a..7912463 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -263,9 +263,6 @@ int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen) FDT_RO_PROBE(fdt); - if (namelen < 1) - return -FDT_ERR_BADPATH; - /* see if we have an alias */ if (*path != '/') { const char *q = memchr(path, '/', end - p); -- cgit v1.2.3 From b202d115bde8094148e386c34978910926f93f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Tue, 10 Oct 2023 10:28:22 +0100 Subject: FROMGIT: libfdt: fdt_path_offset_namelen: Reject empty path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reject empty paths and negative lengths, according to the DT spec v0.4: The convention for specifying a device path is: /node-name-1/node-name-2/node-name-N The path to the root node is /. This prevents the access to path[0] from ever being out-of-bounds. Signed-off-by: Pierre-Clément Tosi Message-ID: <20231010092822.qo2nxc3g47t26dqs@google.com> Signed-off-by: David Gibson Test: N/A Change-Id: I8efee39a06fa62f1f057a975e8133a295be0a656 --- libfdt/fdt_ro.c | 3 +++ tests/path_offset.c | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index 7912463..53db8ce 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -263,6 +263,9 @@ int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen) FDT_RO_PROBE(fdt); + if (!can_assume(VALID_INPUT) && namelen <= 0) + return -FDT_ERR_BADPATH; + /* see if we have an alias */ if (*path != '/') { const char *q = memchr(path, '/', end - p); diff --git a/tests/path_offset.c b/tests/path_offset.c index 82527d4..07e9d65 100644 --- a/tests/path_offset.c +++ b/tests/path_offset.c @@ -48,10 +48,13 @@ static void check_path_offset(void *fdt, char *path, int offset) verbose_printf("Checking offset of \"%s\" is %d...\n", path, offset); rc = fdt_path_offset(fdt, path); + if (rc == offset) + return; + if (rc < 0) FAIL("fdt_path_offset(\"%s\") failed: %s", path, fdt_strerror(rc)); - if (rc != offset) + else FAIL("fdt_path_offset(\"%s\") returned incorrect offset" " %d instead of %d", path, rc, offset); } @@ -102,6 +105,7 @@ int main(int argc, char *argv[]) check_path_offset(fdt, "/subnode@2/subsubnode", subsubnode2_offset2); /* Test paths with extraneous separators */ + check_path_offset(fdt, "", -FDT_ERR_BADPATH); check_path_offset(fdt, "//", 0); check_path_offset(fdt, "///", 0); check_path_offset(fdt, "//subnode@1", subnode1_offset); @@ -110,6 +114,8 @@ int main(int argc, char *argv[]) check_path_offset(fdt, "/subnode@2////subsubnode", subsubnode2_offset2); /* Test fdt_path_offset_namelen() */ + check_path_offset_namelen(fdt, "/subnode@1", -1, -FDT_ERR_BADPATH); + check_path_offset_namelen(fdt, "/subnode@1", 0, -FDT_ERR_BADPATH); check_path_offset_namelen(fdt, "/subnode@1", 1, 0); check_path_offset_namelen(fdt, "/subnode@1/subsubnode", 10, subnode1_offset); check_path_offset_namelen(fdt, "/subnode@1/subsubnode", 11, subnode1_offset); -- cgit v1.2.3 From bb2b54f19e202d5781ec6c05b3d584fcd85cddcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Fri, 6 Oct 2023 14:39:12 +0100 Subject: ANDROID: Revert "Fix integer wrap sanitisation." This reverts commit 0e783e26f75c08e421467ca4a6c21ff2589cd2fa. Revert the patch we've had in Android now that upstream has [1] commit 73590342fc85 ("libfdt: prevent integer overflow in fdt_next_tag") which addresses the same bug. As that patch is less rigorous w.r.t. the final value of 'offset' than the one, the last 'if' is upstreamed by [2], which will be cherry-picked here. [1]: https://android.googlesource.com/platform/external/dtc/+/73590342fc85ca207ca1e6cbc110179873a96962 [2]: https://lore.kernel.org/devicetree-compiler/20231011172427.g4tlsew3wsjtddil@google.com/ Test: N/A Change-Id: I662a599713b4090abd090322bca0a78e58f4c92c --- libfdt/fdt.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/libfdt/fdt.c b/libfdt/fdt.c index c17cad5..9fe7cf4 100644 --- a/libfdt/fdt.c +++ b/libfdt/fdt.c @@ -188,20 +188,12 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) break; case FDT_PROP: - lenp = fdt_offset_ptr(fdt, offset, sizeof(struct fdt_property) - FDT_TAGSIZE); + lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp)); if (!can_assume(VALID_DTB) && !lenp) return FDT_END; /* premature end */ - - /* skip name offset, length */ - offset += sizeof(struct fdt_property) - FDT_TAGSIZE; - - if (!can_assume(VALID_DTB) - && !fdt_offset_ptr(fdt, offset, fdt32_to_cpu(*lenp))) - return FDT_END; /* premature end */ - - /* skip value */ - offset += fdt32_to_cpu(*lenp); - + /* skip-name offset, length and value */ + offset += sizeof(struct fdt_property) - FDT_TAGSIZE + + fdt32_to_cpu(*lenp); if (!can_assume(LATEST) && fdt_version(fdt) < 0x10 && fdt32_to_cpu(*lenp) >= 8 && ((offset - fdt32_to_cpu(*lenp)) % 8) != 0) @@ -217,8 +209,7 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) return FDT_END; } - if (!can_assume(VALID_DTB) && (offset <= startoffset - || !fdt_offset_ptr(fdt, startoffset, offset - startoffset))) + if (!fdt_offset_ptr(fdt, startoffset, offset - startoffset)) return FDT_END; /* premature end */ *nextoffset = FDT_TAGALIGN(offset); -- cgit v1.2.3 From cbfd232da37f480bf3abcba01e16e44306149d45 Mon Sep 17 00:00:00 2001 From: Tadeusz Struk Date: Wed, 5 Oct 2022 16:29:30 -0700 Subject: FROMGIT: libfdt: prevent integer overflow in fdt_next_tag Since fdt_next_tag() in a public API function all input parameters, including the fdt blob should not be trusted. It is possible to forge a blob with invalid property length that will cause integer overflow during offset calculation. To prevent that, validate the property length read from the blob before doing calculations. Signed-off-by: Tadeusz Struk Message-Id: <20221005232931.3016047-1-tadeusz.struk@linaro.org> Signed-off-by: David Gibson (cherry-picked from commit 73590342fc85ca207ca1e6cbc110179873a96962 git://git.kernel.org/pub/scm/utils/dtc/dtc.git main) Test: N/A Change-Id: I894051ed101255800717001a71a5a74ac66fd897 --- libfdt/fdt.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/libfdt/fdt.c b/libfdt/fdt.c index 9fe7cf4..13b4b9b 100644 --- a/libfdt/fdt.c +++ b/libfdt/fdt.c @@ -165,7 +165,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len) uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) { const fdt32_t *tagp, *lenp; - uint32_t tag; + uint32_t tag, len, sum; int offset = startoffset; const char *p; @@ -191,12 +191,19 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp)); if (!can_assume(VALID_DTB) && !lenp) return FDT_END; /* premature end */ + + len = fdt32_to_cpu(*lenp); + sum = len + offset; + if (!can_assume(VALID_DTB) && + (INT_MAX <= sum || sum < (uint32_t) offset)) + return FDT_END; /* premature end */ + /* skip-name offset, length and value */ - offset += sizeof(struct fdt_property) - FDT_TAGSIZE - + fdt32_to_cpu(*lenp); + offset += sizeof(struct fdt_property) - FDT_TAGSIZE + len; + if (!can_assume(LATEST) && - fdt_version(fdt) < 0x10 && fdt32_to_cpu(*lenp) >= 8 && - ((offset - fdt32_to_cpu(*lenp)) % 8) != 0) + fdt_version(fdt) < 0x10 && len >= 8 && + ((offset - len) % 8) != 0) offset += 4; break; -- cgit v1.2.3 From 0493daab327ef269a7ab6a8ab61fae15f92e7666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Wed, 11 Oct 2023 18:24:27 +0100 Subject: FROMLIST: libdft: fdt_next_tag: Harden offset overflow check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As 'offset' is obtained through various paths within the function by adding user-provided values to 'startoffset' and as we validate its final value by substracting it from the initial one, there is a risk that one of the paths might have lead to an overflow, making the check validate a "negative" (wrong) length, potentially causing fdt_next_tag() to report an invalid offset as valid through 'nextoffset'. For example, when parsing an FDT_PROP, we currently validate that offset = startoffset + len + FDT_TAGSIZE doesn't overflow but then assign offset = startoffset + len + sizeof(struct fdt_property) so harden all paths by validating the offset in the very last check. Signed-off-by: Pierre-Clément Tosi (am from https://lore.kernel.org/devicetree-compiler/20231011172427.g4tlsew3wsjtddil@google.com/) Test: N/A Change-Id: I0b17b0827ccc0ece0a2d1795b388408fb599aed7 --- libfdt/fdt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libfdt/fdt.c b/libfdt/fdt.c index 13b4b9b..b8ffb33 100644 --- a/libfdt/fdt.c +++ b/libfdt/fdt.c @@ -216,7 +216,8 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) return FDT_END; } - if (!fdt_offset_ptr(fdt, startoffset, offset - startoffset)) + if (!can_assume(VALID_DTB) && (offset <= startoffset + || !fdt_offset_ptr(fdt, startoffset, offset - startoffset))) return FDT_END; /* premature end */ *nextoffset = FDT_TAGALIGN(offset); -- cgit v1.2.3 From f20cff01e7e83ef976787bab1571568d64c0b11d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Wed, 11 Oct 2023 17:04:33 +0100 Subject: ANDROID: Revert "libfdt: Validate alias property value is a valid string." This reverts commit 9308e7f9772bd226fea9925b1fc4d53c127ed4d5. Revert the Android patch to apply the upstream fix [1] instead. [1]: https://android.googlesource.com/platform/external/dtc/+/79b9e326a162b15ca5758ee214e350f4f7c038fe Test: N/A Change-Id: I702e619c875449b5efda529d01350117a1c4a435 --- libfdt/fdt_ro.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index 362cc4a..e61df25 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -10,14 +10,6 @@ #include "libfdt_internal.h" -/* Check if a buffer contains a nul-terminated string. - * Used for checking property values which should be strings. - */ -static bool is_nul_string(const char *buf, const size_t buf_len) { - return buf_len > 0 && buf[buf_len - 1] == '\0' && - strnlen(buf, buf_len) == buf_len - 1; -} - static int fdt_nodename_eq_(const void *fdt, int offset, const char *s, int len) { @@ -536,27 +528,13 @@ uint32_t fdt_get_phandle(const void *fdt, int nodeoffset) const char *fdt_get_alias_namelen(const void *fdt, const char *name, int namelen) { - const char *prop; int aliasoffset; - int prop_len; aliasoffset = fdt_path_offset(fdt, "/aliases"); if (aliasoffset < 0) return NULL; - prop = fdt_getprop_namelen(fdt, aliasoffset, name, namelen, &prop_len); - if (prop && !can_assume(VALID_INPUT)) { - /* Validate the alias value. From the devicetree spec v0.3: - * "An alias value is a device path and is encoded as a string. - * The value representes the full path to a node, ..." - * A full path must start at the root to prevent recursion. - */ - if (prop_len == 0 || *prop != '/' || !is_nul_string(prop, prop_len)) { - prop = NULL; - } - } - - return prop; + return fdt_getprop_namelen(fdt, aliasoffset, name, namelen, NULL); } const char *fdt_get_alias(const void *fdt, const char *name) -- cgit v1.2.3 From 19db536250d6357c47a75f0d06f691fd025a25c8 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 9 Mar 2023 11:28:32 +0100 Subject: FROMGIT: add fdt_path_getprop_namelen() helper Add a wrapper for fdt_getprop_namelen() allowing one to specify the node by path instead of offset. Signed-off-by: Rasmus Villemoes Signed-off-by: David Gibson (cherry-picked from commit df093279282ca0cff4d20ceb3bb5857117ed4cc4 git://git.kernel.org/pub/scm/utils/dtc/dtc.git main) Test: N/A Change-Id: If9a102a622dfa726b7cb10f58c38f1b52d233be6 --- libfdt/fdt_ro.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index e61df25..31f78e2 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -525,6 +525,18 @@ uint32_t fdt_get_phandle(const void *fdt, int nodeoffset) return fdt32_ld_(php); } +static const void *fdt_path_getprop_namelen(const void *fdt, const char *path, + const char *propname, int propnamelen, + int *lenp) +{ + int offset = fdt_path_offset(fdt, path); + + if (offset < 0) + return NULL; + + return fdt_getprop_namelen(fdt, offset, propname, propnamelen, lenp); +} + const char *fdt_get_alias_namelen(const void *fdt, const char *name, int namelen) { -- cgit v1.2.3 From 8106368cb0a70f327cb2cb411a46eb33cb4bded7 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 9 Mar 2023 11:32:39 +0100 Subject: FROMGIT: use fdt_path_getprop_namelen() in fdt_get_alias_namelen() Simplify the code by making use of the new helper. Signed-off-by: Rasmus Villemoes Signed-off-by: David Gibson (cherry-picked from commit 18f5ec12a10ec84e957222074dadf4a3e4cc8d59 git://git.kernel.org/pub/scm/utils/dtc/dtc.git main) Test: N/A Change-Id: I43c6ad22dbaa718cd77421a44d5e87d188d26ca0 --- libfdt/fdt_ro.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index 31f78e2..c7fc486 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -540,13 +540,7 @@ static const void *fdt_path_getprop_namelen(const void *fdt, const char *path, const char *fdt_get_alias_namelen(const void *fdt, const char *name, int namelen) { - int aliasoffset; - - aliasoffset = fdt_path_offset(fdt, "/aliases"); - if (aliasoffset < 0) - return NULL; - - return fdt_getprop_namelen(fdt, aliasoffset, name, namelen, NULL); + return fdt_path_getprop_namelen(fdt, "/aliases", name, namelen, NULL); } const char *fdt_get_alias(const void *fdt, const char *name) -- cgit v1.2.3 From 9964e3b4f954968406fd49b325c4d1ae86cc6e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Tue, 10 Oct 2023 10:27:25 +0100 Subject: FROMGIT: libfdt: fdt_get_alias_namelen: Validate aliases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensure that the alias found matches the device tree specification v0.4: Each property of the /aliases node defines an alias. The property name specifies the alias name. The property value specifies the full path to a node in the devicetree. This protects against a stack overflow caused by fdt_path_offset_namelen(fdt, path, namelen) calling fdt_path_offset(fdt, fdt_get_alias_namelen(fdt, path, namelen)) leading to infinite recursion on DTs with "circular" aliases. This fix was originally written by Mike McTernan for Android in [1]. [1]: https://android.googlesource.com/platform/external/dtc/+/9308e7f9772bd226fea9925b1fc4d53c127ed4d5 Signed-off-by: Pierre-Clément Tosi Acked-by: Mike McTernan Message-ID: <20231010092725.63h7c45p2fnmj577@google.com> Signed-off-by: David Gibson (cherry-picked from commit 79b9e326a162b15ca5758ee214e350f4f7c038fe git://git.kernel.org/pub/scm/utils/dtc/dtc.git main) Test: N/A Change-Id: I1e5a89039f6b70c82e17739379d97dbf130036e8 --- libfdt/fdt_ro.c | 11 ++++++++++- tests/aliases.dts | 4 ++++ tests/get_alias.c | 14 +++++++++++++- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index c7fc486..f40333b 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -540,7 +540,16 @@ static const void *fdt_path_getprop_namelen(const void *fdt, const char *path, const char *fdt_get_alias_namelen(const void *fdt, const char *name, int namelen) { - return fdt_path_getprop_namelen(fdt, "/aliases", name, namelen, NULL); + int len; + const char *alias; + + alias = fdt_path_getprop_namelen(fdt, "/aliases", name, namelen, &len); + + if (!can_assume(VALID_DTB) && + !(alias && len > 0 && alias[len - 1] == '\0' && *alias == '/')) + return NULL; + + return alias; } const char *fdt_get_alias(const void *fdt, const char *name) diff --git a/tests/aliases.dts b/tests/aliases.dts index 853479a..03ed675 100644 --- a/tests/aliases.dts +++ b/tests/aliases.dts @@ -5,6 +5,10 @@ #size-cells = <0>; aliases { + empty = ""; + loop = "loop"; + nonull = [626164]; + relative = "s1/subsubnode"; s1 = &sub1; ss1 = &subsub1; sss1 = &subsubsub1; diff --git a/tests/get_alias.c b/tests/get_alias.c index fb2c38c..d2888d6 100644 --- a/tests/get_alias.c +++ b/tests/get_alias.c @@ -21,9 +21,16 @@ static void check_alias(void *fdt, const char *path, const char *alias) aliaspath = fdt_get_alias(fdt, alias); - if (path && !aliaspath) + if (!path && !aliaspath) + return; + + if (!aliaspath) FAIL("fdt_get_alias(%s) failed\n", alias); + if (!path) + FAIL("fdt_get_alias(%s) returned %s instead of NULL", + alias, aliaspath); + if (strcmp(aliaspath, path) != 0) FAIL("fdt_get_alias(%s) returned %s instead of %s\n", alias, aliaspath, path); @@ -36,9 +43,14 @@ int main(int argc, char *argv[]) test_init(argc, argv); fdt = load_blob_arg(argc, argv); + check_alias(fdt, NULL, "empty"); + check_alias(fdt, NULL, "nonull"); + check_alias(fdt, NULL, "relative"); check_alias(fdt, "/subnode@1", "s1"); check_alias(fdt, "/subnode@1/subsubnode", "ss1"); check_alias(fdt, "/subnode@1/subsubnode/subsubsubnode", "sss1"); + check_alias(fdt, NULL, "loop"); // Might trigger a stack overflow + PASS(); } -- cgit v1.2.3 From 7eeadceb3eecdc37e19105d577dd2835a4babc53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Wed, 11 Oct 2023 14:09:22 +0100 Subject: ANDROID: Generate version_gen.h in Soong and Bazel Upstream provides a DTC_VERSION preprocessor macro to its C code by generating a version_gen.h header from either of its supported build systems: Make formats the header from its internal variables (VERSION, PATCHLEVEL, SUBLEVEL, ...) while Meson uses its vcs_tag() function on a template, version_gen.h.in. As AOSP doesn't make use of these build systems, aosp/204511 decided to hardcode a version_non_gen.h file and patch the corresponding #include. This unsurprisingly ended up bitrotting as the repo was being upgraded. Instead, replicate the version_gen.h.in patching in our build systems, extracting the version number from the METADATA file, which external_updater.sh will keep up-to-date. Note that this introduces a dependency on sed in the genrule(), the impact of which is minimized by making METADATA_version.sed POSIX-compliant. Keep appending the suffix '-Android-build' to the upstream version. Test: m dtc && ${ANDROID_HOST_OUT}/bin/dtc --version Test: bazel build //:dtc && bazel-bin/dtc --version Change-Id: I6b780c1dbe14d415891defeb652f0692988ed0b1 --- Android.bp | 15 +++++++++++++++ BUILD.bazel | 16 +++++++++++++++- METADATA_version.sed | 1 + util.c | 2 +- version_non_gen.h | 1 - 5 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 METADATA_version.sed delete mode 100644 version_non_gen.h diff --git a/Android.bp b/Android.bp index cb13748..ba0ab24 100644 --- a/Android.bp +++ b/Android.bp @@ -54,6 +54,7 @@ cc_defaults { "-DNO_YAML" ], + generated_headers: ["dtc_version_gen.h"], shared_libs: ["libfdt"], stl: "none", @@ -109,3 +110,17 @@ cc_binary_host { "util.c", ], } + +genrule { + name: "dtc_version_gen.h", + out: ["version_gen.h"], + srcs: ["version_gen.h.in"], + tool_files: [ + "METADATA", + "METADATA_version.sed", + ], + cmd: "version=$$(" + + "sed -f $(location METADATA_version.sed) -n $(location METADATA)" + + ")-Android-build;" + + "sed s/@VCS_TAG@/$${version}/ $(in) > $(out)", +} diff --git a/BUILD.bazel b/BUILD.bazel index 634115c..4a201d3 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -40,9 +40,23 @@ genrule( UTILS = [ "util.c", "util.h", - "version_non_gen.h", + ":version_gen_header", ] +genrule( + name = "version_gen_header", + outs = ["version_gen.h"], + srcs = [ + "METADATA", + "METADATA_version.sed", + "version_gen.h.in", + ], + cmd = """ + version="$$(sed -f $(location METADATA_version.sed) -n $(location METADATA))-Android-build" + sed s/@VCS_TAG@/$${version}/ $(location version_gen.h.in) > $@ + """, +) + cc_binary( name = "dtc", srcs = UTILS + [ diff --git a/METADATA_version.sed b/METADATA_version.sed new file mode 100644 index 0000000..9e7ea54 --- /dev/null +++ b/METADATA_version.sed @@ -0,0 +1 @@ +s/^[[:space:]]*version:[[:space:]]*"v\([[:digit:]][[:digit:]]*.[[:digit:]][[:digit:]]*.[[:digit:]][[:digit:]]*\)"[[:space:]]*/\1/p diff --git a/util.c b/util.c index 197fb19..507f012 100644 --- a/util.c +++ b/util.c @@ -21,7 +21,7 @@ #include "libfdt.h" #include "util.h" -#include "version_non_gen.h" +#include "version_gen.h" char *xstrdup(const char *s) { diff --git a/version_non_gen.h b/version_non_gen.h deleted file mode 100644 index 3376b35..0000000 --- a/version_non_gen.h +++ /dev/null @@ -1 +0,0 @@ -#define DTC_VERSION "DTC 1.6.0-Android-build" -- cgit v1.2.3