[PATCH v6 0/8] libfdt: Allow more control of code size

[PATCH v6 0/8] libfdt: Allow more control of code size

[Simon Glass]

Since v1.4.6 libfdt has gained a number of checks for incorrect
device-tree input and incorrect parameters. These are valuable and should
be enabled by default. Some are basic sanity checks and some are designed
to avoid security risks with carefully crafted device-tree input.

However the checks have added to code size to such an extent that many
U-Boot boards can no-longer build/boot in SPL. This has prevented recent
versions of libfdt from being used by U-Boot.

U-Boot SPL generally reads a device tree which has been set up by the
build system and is known to be correct (e.g. it may be cryptographically
signed by the build system). Therefore sanity checks in SPL should never
fail, and just contribute to longer run-time and larger code size.

During review of those patches[1] the code-size impact was discussed and
a possible solution was suggested.

This series adds a new ASSUME_MASK option, which allows for some control
over the checks used in libfdt. With all assumptions enabled, libfdt
assumes that the input data and parameters are all correct and that
internal errors cannot happen. This allows U-Boot SPL to continue to use
later versions of libfdt.

Within the code, preprocessor macros and an inline function are used which
resolve to true or false to control the use of checks in the code. This
seems better than using '#ifdefs' throughout the code. It also ensures
that all code is compiled regardless of which checks are enabled (useful
for build coverage).

This series reduces the size of libfdt by about 3KB on 64-bit x86
(about 6%) when all assumptions are enabled. Future work is planned to
reduce this further, but this is a good start. It enables U-Boot to move
to the latest libfdt.

Note: libfdt includes a number of assignments in conditional statements.
I have only changed these where necessary. It might be desirable to remove
them all, to reduce the output from checkpatch.pl.

Series available at https://github.com/sglass68/dtc/tree/small6

[1] https://www.spinics.net/lists/devicetree-compiler/msg02166.html

Changes in v6:
- Add a new patch to disable the ordering check and fixup
- Add an option to skip the ordering check
- Always call fdt_ro_probe_(), etc. and have that function do the check
- Change check in fdt_get_property_namelen_() to VALID_DTB
- Don't add VALID_INPUT to fdt_get_property_namelen_()
- Drop assumption in fdt_nodename_eq_()

Changes in v5:
- Include just VALID_INPUT checks in this patch
- Rename _can_assume() to can_assume_()
- Split SANE into VALID_DTB and VALID_INPUT
- Split out VALID_DTB checks into a separate patch
- Update comment for ASSUME_PERFECT to mention invalid parameters

Changes in v4:
- Add a can_assume() macros and squash the inline functions into one
- Add fdt_header_size to version.lds
- Drop unnecessary FDT_ prefix
- Fix 'Incosistencies' typo
- Merge the 'friendly' and 'sane' checks into one
- Update and expand comments

Changes in v3:
- Add a new patch to de-inline fdt_header_size()
- Expand the comments about each check option
- Fix 'santiy' typo
- Instead of excluding fdt_check_full() put it in its own file
- Invert the checks to be called assumptions
- Move header-version code to fdt.c
- Move the comments on check options to the enum
- Rearrange code in terms of checks instead of files changed, to aid review
- Replace 'unsigned' with 'unsigned int'
- Update commit message a little
- Use hex for CHK_MASK

Changes in v2:
- Add a comment to fdt_find_add_string_()
- Add an fdt_ prefix to avoid namespace conflicts
- Correct inverted version checks in a few cases
- Drop optimised code path in fdt_nodename_eq_()
- Update to use new check functions
- Use fdt_chk_base() in fdt_blocks_misordered_()
- Use symbolic names for _check functions and drop leading underscores

Simon Glass (8):
  libfdt: De-inline fdt_header_size()
  Add a way to control the level of checks in the code
  libfdt: Add support for disabling dtb checks
  libfdt: Add support for disabling sanity checks
  libfdt: Add support for disabling rollback handling
  libfdt: Add support for disabling version checks
  libfdt: Add support for disabling ordering check/fixup
  libfdt: Allow exclusion of fdt_check_full()

 Makefile                 |   6 +-
 libfdt/Makefile.libfdt   |   2 +-
 libfdt/fdt.c             | 100 +++++++++++++++++----------
 libfdt/fdt_check.c       |  74 ++++++++++++++++++++
 libfdt/fdt_ro.c          | 143 ++++++++++++++-------------------------
 libfdt/fdt_rw.c          |  29 ++++++--
 libfdt/fdt_sw.c          |  19 ++++--
 libfdt/libfdt.h          |   9 +--
 libfdt/libfdt_internal.h | 104 ++++++++++++++++++++++++++++
 libfdt/version.lds       |   1 +
 tests/testutils.c        |   1 -
 11 files changed, 338 insertions(+), 150 deletions(-)
 create mode 100644 libfdt/fdt_check.c

-- 
2.25.0.341.g760bfbb309-goog

[PATCH v6 1/8] libfdt: De-inline fdt_header_size()

[Simon Glass]

There does not seem to be a strong reason to inline this function. Also
we are about to add some extra code to it which will increase its size.

Move it into fdt.c and use a simple declaration in libfdt.h

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v6: None
Changes in v5: None
Changes in v4:
- Add fdt_header_size to version.lds

Changes in v3:
- Add a new patch to de-inline fdt_header_size()

Changes in v2: None

 libfdt/fdt.c       | 5 +++++
 libfdt/libfdt.h    | 9 +++++----
 libfdt/version.lds | 1 +
 tests/testutils.c  | 1 -
 4 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index d6ce7c0..3e37a4b 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -70,6 +70,11 @@ size_t fdt_header_size_(uint32_t version)
 		return FDT_V17_SIZE;
 }
 
+size_t fdt_header_size(const void *fdt)
+{
+	return fdt_header_size_(fdt_version(fdt));
+}
+
 int fdt_check_header(const void *fdt)
 {
 	size_t hdrsize;
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index fc4c496..48f375c 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -266,11 +266,12 @@ fdt_set_hdr_(size_dt_struct);
  * fdt_header_size - return the size of the tree's header
  * @fdt: pointer to a flattened device tree
  */
+size_t fdt_header_size(const void *fdt);
+
+/**
+ * fdt_header_size_ - internal function which takes a version number
+ */
 size_t fdt_header_size_(uint32_t version);
-static inline size_t fdt_header_size(const void *fdt)
-{
-	return fdt_header_size_(fdt_version(fdt));
-}
 
 /**
  * fdt_check_header - sanity check a device tree header
diff --git a/libfdt/version.lds b/libfdt/version.lds
index ae32924..7ab85f1 100644
--- a/libfdt/version.lds
+++ b/libfdt/version.lds
@@ -20,6 +20,7 @@ LIBFDT_1.2 {
 		fdt_get_alias_namelen;
 		fdt_get_alias;
 		fdt_get_path;
+                fdt_header_size;
 		fdt_supernode_atdepth_offset;
 		fdt_node_depth;
 		fdt_parent_offset;
diff --git a/tests/testutils.c b/tests/testutils.c
index 5e494c5..53cb35f 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -261,7 +261,6 @@ void vg_prepare_blob(void *fdt, size_t bufsize)
 
 	VALGRIND_MAKE_MEM_UNDEFINED(blob, bufsize);
 	VALGRIND_MAKE_MEM_DEFINED(blob, FDT_V1_SIZE);
-	VALGRIND_MAKE_MEM_DEFINED(blob, fdt_header_size(fdt));
 
 	if (fdt_magic(fdt) == FDT_MAGIC) {
 		off_strings = fdt_off_dt_strings(fdt);
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v6 2/8] Add a way to control the level of checks in the code

[Simon Glass]

Add a new ASSUME_MASK option, which allows for some control over the
checks used in libfdt. With all assumptions enabled, libfdt assumes that
the input data and parameters are all correct and that internal errors
cannot happen.

By default no assumptions are made and all checks are enabled.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v6:
- Add an option to skip the ordering check

Changes in v5:
- Rename _can_assume() to can_assume_()
- Split SANE into VALID_DTB and VALID_INPUT
- Update comment for ASSUME_PERFECT to mention invalid parameters

Changes in v4:
- Add a can_assume() macros and squash the inline functions into one
- Drop unnecessary FDT_ prefix
- Fix 'Incosistencies' typo
- Merge the 'friendly' and 'sane' checks into one
- Update and expand comments

Changes in v3:
- Expand the comments about each check option
- Invert the checks to be called assumptions
- Move header-version code to fdt.c
- Move the comments on check options to the enum
- Use hex for CHK_MASK

Changes in v2:
- Add an fdt_ prefix to avoid namespace conflicts
- Use symbolic names for _check functions and drop leading underscores

 Makefile                 |   6 ++-
 libfdt/libfdt_internal.h | 104 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e73576a..0dff03f 100644
--- a/Makefile
+++ b/Makefile
@@ -16,7 +16,11 @@ EXTRAVERSION =
 LOCAL_VERSION =
 CONFIG_LOCALVERSION =
 
-CPPFLAGS = -I libfdt -I .
+# Control the assumptions made (e.g. risking security issues) in the code.
+# See libfdt_internal.h for details
+ASSUME_MASK ?= 0
+
+CPPFLAGS = -I libfdt -I . -DFDT_ASSUME_MASK=$(ASSUME_MASK)
 WARNINGS = -Wall -Wpointer-arith -Wcast-qual -Wnested-externs \
 	-Wstrict-prototypes -Wmissing-prototypes -Wredundant-decls -Wshadow
 CFLAGS = -g -Os $(SHAREDLIB_CFLAGS) -Werror $(WARNINGS) $(EXTRA_CFLAGS)
diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h
index 058c735..1b96613 100644
--- a/libfdt/libfdt_internal.h
+++ b/libfdt/libfdt_internal.h
@@ -48,4 +48,108 @@ static inline struct fdt_reserve_entry *fdt_mem_rsv_w_(void *fdt, int n)
 
 #define FDT_SW_MAGIC		(~FDT_MAGIC)
 
+/**********************************************************************/
+/* Checking controls                                                  */
+/**********************************************************************/
+
+#ifndef FDT_ASSUME_MASK
+#define FDT_ASSUME_MASK 0
+#endif
+
+/*
+ * Defines assumptions which can be enabled. Each of these can be enabled
+ * individually. For maximum saftey, don't enable any assumptions!
+ *
+ * For minimal code size and no safety, use ASSUME_PERFECT at your own risk.
+ * You should have another method of validating the device tree, such as a
+ * signature or hash check before using libfdt.
+ *
+ * For situations where security is not a concern it may be safe to enable
+ * ASSUME_SANE.
+ */
+enum {
+	/*
+	 * This does essentially no checks. Only the latest device-tree
+	 * version is correctly handled. Inconsistencies or errors in the device
+	 * tree may cause undefined behaviour or crashes. Invalid parameters
+	 * passed to libfdt may do the same.
+	 *
+	 * If an error occurs when modifying the tree it may leave the tree in
+	 * an intermediate (but valid) state. As an example, adding a property
+	 * where there is insufficient space may result in the property name
+	 * being added to the string table even though the property itself is
+	 * not added to the struct section.
+	 *
+	 * Only use this if you have a fully validated device tree with
+	 * the latest supported version and wish to minimise code size.
+	 */
+	ASSUME_PERFECT		= 0xff,
+
+	/*
+	 * This assumes that the device tree is sane. i.e. header metadata
+	 * and basic hierarchy are correct.
+	 *
+	 * With this assumption enabled, normal device trees produced by libfdt
+	 * and the compiler should be handled safely. Malicious device trees and
+	 * complete garbage may cause libfdt to behave badly or crash.
+	 *
+	 * Note: Only checks that relate exclusively to the device tree itself
+	 * (not the parameters passed to libfdt) are disabled by this
+	 * assumption. This includes checking headers, tags and the like.
+	 */
+	ASSUME_VALID_DTB	= 1 << 0,
+
+	/*
+	 * This builds on ASSUME_VALID_DTB and further assumes that libfdt
+	 * functions are called with valid parameters, i.e. not trigger
+	 * FDT_ERR_BADOFFSET or offsets that are out of bounds. It disables any
+	 * extensive checking of parameters and the device tree, making various
+	 * assumptions about correctness.
+	 *
+	 * It doesn't make sense to enable this assumption unless
+	 * ASSUME_VALID_DTB is also enabled.
+	 */
+	ASSUME_VALID_INPUT	= 1 << 1,
+
+	/*
+	 * This disables checks for device-tree version and removes all code
+	 * which handles older versions.
+	 *
+	 * Only enable this if you know you have a device tree with the latest
+	 * version.
+	 */
+	ASSUME_LATEST		= 1 << 2,
+
+	/*
+	 * This assume that it is OK for a failed additional to the device tree
+	 * due to lack of space or some other problem can skip any rollback
+	 * steps (such as dropping the property name from the string table).
+	 * This is safe to enable in most circumstances, even though it may
+	 * leave the tree in a sub-optimal state.
+	 */
+	ASSUME_NO_ROLLBACK	= 1 << 3,
+
+	/*
+	 * This assumes that the device tree components appear in the correct
+	 * order. As such it disables a check in fdt_open_into() and removes the
+	 * ability to fix the problem there. This is safe if you know that the
+	 * device tree is correctly ordered. See fdt_blocks_misordered_().
+	 */
+	ASSUME_CORRECT_ORDER	= 1 << 4,
+};
+
+/**
+ * can_assume_() - check if a particular assumption is enabled
+ *
+ * @mask: Mask to check (ASSUME_...)
+ * @return true if that assumption is enabled, else false
+ */
+static inline bool can_assume_(int mask)
+{
+	return FDT_ASSUME_MASK & mask;
+}
+
+/** helper macros for checking assumptions */
+#define can_assume(_assume)	can_assume_(ASSUME_ ## _assume)
+
 #endif /* LIBFDT_INTERNAL_H */
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v6 3/8] libfdt: Add support for disabling dtb checks

[Simon Glass]

Support ASSUME_VALID_DTB to disable some sanity checks

If we assume that the DTB itself is valid then we can skip some checks and
save code space. Add various conditions to handle this.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v6:
- Always call fdt_ro_probe_(), etc. and have that function do the check
- Change check in fdt_get_property_namelen_() to VALID_DTB

Changes in v5:
- Split out VALID_DTB checks into a separate patch

Changes in v4: None
Changes in v3: None
Changes in v2: None

 libfdt/fdt.c    | 53 +++++++++++++++++++++++++++++--------------------
 libfdt/fdt_ro.c |  3 ++-
 libfdt/fdt_rw.c |  2 ++
 libfdt/fdt_sw.c | 19 +++++++++++-------
 4 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index 3e37a4b..d4daf60 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -19,6 +19,9 @@ int32_t fdt_ro_probe_(const void *fdt)
 {
 	uint32_t totalsize = fdt_totalsize(fdt);
 
+	if (can_assume(VALID_DTB))
+		return totalsize;
+
 	if (fdt_magic(fdt) == FDT_MAGIC) {
 		/* Complete tree */
 		if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
@@ -81,38 +84,44 @@ int fdt_check_header(const void *fdt)
 
 	if (fdt_magic(fdt) != FDT_MAGIC)
 		return -FDT_ERR_BADMAGIC;
-	hdrsize = fdt_header_size(fdt);
 	if ((fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
 	    || (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION))
 		return -FDT_ERR_BADVERSION;
 	if (fdt_version(fdt) < fdt_last_comp_version(fdt))
 		return -FDT_ERR_BADVERSION;
+	hdrsize = fdt_header_size(fdt);
+	if (!can_assume(VALID_DTB)) {
 
-	if ((fdt_totalsize(fdt) < hdrsize)
-	    || (fdt_totalsize(fdt) > INT_MAX))
-		return -FDT_ERR_TRUNCATED;
-
-	/* Bounds check memrsv block */
-	if (!check_off_(hdrsize, fdt_totalsize(fdt), fdt_off_mem_rsvmap(fdt)))
-		return -FDT_ERR_TRUNCATED;
+		if ((fdt_totalsize(fdt) < hdrsize)
+		    || (fdt_totalsize(fdt) > INT_MAX))
+			return -FDT_ERR_TRUNCATED;
 
-	/* Bounds check structure block */
-	if (fdt_version(fdt) < 17) {
+		/* Bounds check memrsv block */
 		if (!check_off_(hdrsize, fdt_totalsize(fdt),
-				fdt_off_dt_struct(fdt)))
+				fdt_off_mem_rsvmap(fdt)))
 			return -FDT_ERR_TRUNCATED;
-	} else {
+	}
+
+	if (!can_assume(VALID_DTB)) {
+		/* Bounds check structure block */
+		if (fdt_version(fdt) < 17) {
+			if (!check_off_(hdrsize, fdt_totalsize(fdt),
+					fdt_off_dt_struct(fdt)))
+				return -FDT_ERR_TRUNCATED;
+		} else {
+			if (!check_block_(hdrsize, fdt_totalsize(fdt),
+					  fdt_off_dt_struct(fdt),
+					  fdt_size_dt_struct(fdt)))
+				return -FDT_ERR_TRUNCATED;
+		}
+
+		/* Bounds check strings block */
 		if (!check_block_(hdrsize, fdt_totalsize(fdt),
-				  fdt_off_dt_struct(fdt),
-				  fdt_size_dt_struct(fdt)))
+				  fdt_off_dt_strings(fdt),
+				  fdt_size_dt_strings(fdt)))
 			return -FDT_ERR_TRUNCATED;
 	}
 
-	/* Bounds check strings block */
-	if (!check_block_(hdrsize, fdt_totalsize(fdt),
-			  fdt_off_dt_strings(fdt), fdt_size_dt_strings(fdt)))
-		return -FDT_ERR_TRUNCATED;
-
 	return 0;
 }
 
@@ -142,7 +151,7 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
 
 	*nextoffset = -FDT_ERR_TRUNCATED;
 	tagp = fdt_offset_ptr(fdt, offset, FDT_TAGSIZE);
-	if (!tagp)
+	if (!can_assume(VALID_DTB) && !tagp)
 		return FDT_END; /* premature end */
 	tag = fdt32_to_cpu(*tagp);
 	offset += FDT_TAGSIZE;
@@ -154,13 +163,13 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
 		do {
 			p = fdt_offset_ptr(fdt, offset++, 1);
 		} while (p && (*p != '\0'));
-		if (!p)
+		if (!can_assume(VALID_DTB) && !p)
 			return FDT_END; /* premature end */
 		break;
 
 	case FDT_PROP:
 		lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp));
-		if (!lenp)
+		if (!can_assume(VALID_DTB) && !lenp)
 			return FDT_END; /* premature end */
 		/* skip-name offset, length and value */
 		offset += sizeof(struct fdt_property) - FDT_TAGSIZE
diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index a5c2797..4c26fbe 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -388,7 +388,8 @@ static const struct fdt_property *fdt_get_property_namelen_(const void *fdt,
 	     (offset = fdt_next_property_offset(fdt, offset))) {
 		const struct fdt_property *prop;
 
-		if (!(prop = fdt_get_property_by_offset_(fdt, offset, lenp))) {
+		prop = fdt_get_property_by_offset_(fdt, offset, lenp);
+		if (!can_assume(VALID_DTB) && !prop) {
 			offset = -FDT_ERR_INTERNAL;
 			break;
 		}
diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 8795947..707c00a 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -24,6 +24,8 @@ static int fdt_blocks_misordered_(const void *fdt,
 
 static int fdt_rw_probe_(void *fdt)
 {
+	if (can_assume(VALID_DTB))
+		return 0;
 	FDT_RO_PROBE(fdt);
 
 	if (fdt_version(fdt) < 17)
diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c
index 76bea22..96365b4 100644
--- a/libfdt/fdt_sw.c
+++ b/libfdt/fdt_sw.c
@@ -12,10 +12,13 @@
 
 static int fdt_sw_probe_(void *fdt)
 {
-	if (fdt_magic(fdt) == FDT_MAGIC)
-		return -FDT_ERR_BADSTATE;
-	else if (fdt_magic(fdt) != FDT_SW_MAGIC)
-		return -FDT_ERR_BADMAGIC;
+	if (!can_assume(VALID_DTB)) {
+		if (fdt_magic(fdt) == FDT_MAGIC)
+			return -FDT_ERR_BADSTATE;
+		else if (fdt_magic(fdt) != FDT_SW_MAGIC)
+			return -FDT_ERR_BADMAGIC;
+	}
+
 	return 0;
 }
 
@@ -38,7 +41,7 @@ static int fdt_sw_probe_memrsv_(void *fdt)
 	if (err)
 		return err;
 
-	if (fdt_off_dt_strings(fdt) != 0)
+	if (!can_assume(VALID_DTB) && fdt_off_dt_strings(fdt) != 0)
 		return -FDT_ERR_BADSTATE;
 	return 0;
 }
@@ -46,7 +49,8 @@ static int fdt_sw_probe_memrsv_(void *fdt)
 #define FDT_SW_PROBE_MEMRSV(fdt) \
 	{ \
 		int err; \
-		if ((err = fdt_sw_probe_memrsv_(fdt)) != 0) \
+		if (!can_assume(VALID_DTB) && \
+		    (err = fdt_sw_probe_memrsv_(fdt)) != 0) \
 			return err; \
 	}
 
@@ -151,7 +155,8 @@ int fdt_resize(void *fdt, void *buf, int bufsize)
 	headsize = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
 	tailsize = fdt_size_dt_strings(fdt);
 
-	if ((headsize + tailsize) > fdt_totalsize(fdt))
+	if (!can_assume(VALID_DTB) &&
+	    headsize + tailsize > fdt_totalsize(fdt))
 		return -FDT_ERR_INTERNAL;
 
 	if ((headsize + tailsize) > bufsize)
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v6 4/8] libfdt: Add support for disabling sanity checks

[Simon Glass]

Allow enabling ASSUME_VALID_INPUT to disable sanity checks on the device
tree and the parameters to libfdt. This assumption covers that cases where
the problem could be with either.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v6:
- Don't add VALID_INPUT to fdt_get_property_namelen_()
- Drop assumption in fdt_nodename_eq_()

Changes in v5:
- Include just VALID_INPUT checks in this patch

Changes in v4: None
Changes in v3: None
Changes in v2: None

 libfdt/fdt.c    | 14 ++++++++----
 libfdt/fdt_ro.c | 61 +++++++++++++++++++++++++++++++++----------------
 2 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index d4daf60..f6600ff 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -129,10 +129,11 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
 {
 	unsigned absoffset = offset + fdt_off_dt_struct(fdt);
 
-	if ((absoffset < offset)
-	    || ((absoffset + len) < absoffset)
-	    || (absoffset + len) > fdt_totalsize(fdt))
-		return NULL;
+	if (!can_assume(VALID_INPUT))
+		if ((absoffset < offset)
+		    || ((absoffset + len) < absoffset)
+		    || (absoffset + len) > fdt_totalsize(fdt))
+			return NULL;
 
 	if (fdt_version(fdt) >= 0x11)
 		if (((offset + len) < offset)
@@ -188,7 +189,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_INPUT) &&
+	    !fdt_offset_ptr(fdt, startoffset, offset - startoffset))
 		return FDT_END; /* premature end */
 
 	*nextoffset = FDT_TAGALIGN(offset);
@@ -197,6 +199,8 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
 
 int fdt_check_node_offset_(const void *fdt, int offset)
 {
+	if (can_assume(VALID_INPUT))
+		return offset;
 	if ((offset < 0) || (offset % FDT_TAGSIZE)
 	    || (fdt_next_tag(fdt, offset, &offset) != FDT_BEGIN_NODE))
 		return -FDT_ERR_BADOFFSET;
diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 4c26fbe..9c32559 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -33,17 +33,26 @@ static int fdt_nodename_eq_(const void *fdt, int offset,
 
 const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
 {
-	int32_t totalsize = fdt_ro_probe_(fdt);
-	uint32_t absoffset = stroffset + fdt_off_dt_strings(fdt);
+	int32_t totalsize;
+	uint32_t absoffset;
 	size_t len;
 	int err;
 	const char *s, *n;
 
+	if (can_assume(VALID_INPUT)) {
+		s = (const char *)fdt + fdt_off_dt_strings(fdt) + stroffset;
+
+		if (lenp)
+			*lenp = strlen(s);
+		return s;
+	}
+	totalsize = fdt_ro_probe_(fdt);
 	err = totalsize;
 	if (totalsize < 0)
 		goto fail;
 
 	err = -FDT_ERR_BADOFFSET;
+	absoffset = stroffset + fdt_off_dt_strings(fdt);
 	if (absoffset >= totalsize)
 		goto fail;
 	len = totalsize - absoffset;
@@ -151,10 +160,13 @@ static const struct fdt_reserve_entry *fdt_mem_rsv(const void *fdt, int n)
 	int offset = n * sizeof(struct fdt_reserve_entry);
 	int absoffset = fdt_off_mem_rsvmap(fdt) + offset;
 
-	if (absoffset < fdt_off_mem_rsvmap(fdt))
-		return NULL;
-	if (absoffset > fdt_totalsize(fdt) - sizeof(struct fdt_reserve_entry))
-		return NULL;
+	if (!can_assume(VALID_INPUT)) {
+		if (absoffset < fdt_off_mem_rsvmap(fdt))
+			return NULL;
+		if (absoffset > fdt_totalsize(fdt) -
+		    sizeof(struct fdt_reserve_entry))
+			return NULL;
+	}
 	return fdt_mem_rsv_(fdt, n);
 }
 
@@ -164,7 +176,7 @@ int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size)
 
 	FDT_RO_PROBE(fdt);
 	re = fdt_mem_rsv(fdt, n);
-	if (!re)
+	if (!can_assume(VALID_INPUT) && !re)
 		return -FDT_ERR_BADOFFSET;
 
 	*address = fdt64_ld(&re->address);
@@ -346,7 +358,8 @@ static const struct fdt_property *fdt_get_property_by_offset_(const void *fdt,
 	int err;
 	const struct fdt_property *prop;
 
-	if ((err = fdt_check_prop_offset_(fdt, offset)) < 0) {
+	if (!can_assume(VALID_INPUT) &&
+	    (err = fdt_check_prop_offset_(fdt, offset)) < 0) {
 		if (lenp)
 			*lenp = err;
 		return NULL;
@@ -462,14 +475,19 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
 	if (namep) {
 		const char *name;
 		int namelen;
-		name = fdt_get_string(fdt, fdt32_ld(&prop->nameoff),
-				      &namelen);
-		if (!name) {
-			if (lenp)
-				*lenp = namelen;
-			return NULL;
+
+		if (!can_assume(VALID_INPUT)) {
+			name = fdt_get_string(fdt, fdt32_ld(&prop->nameoff),
+					      &namelen);
+			if (!name) {
+				if (lenp)
+					*lenp = namelen;
+				return NULL;
+			}
+			*namep = name;
+		} else {
+			*namep = fdt_string(fdt, fdt32_ld(&prop->nameoff));
 		}
-		*namep = name;
 	}
 
 	/* Handle realignment */
@@ -599,10 +617,12 @@ int fdt_supernode_atdepth_offset(const void *fdt, int nodeoffset,
 		}
 	}
 
-	if ((offset == -FDT_ERR_NOTFOUND) || (offset >= 0))
-		return -FDT_ERR_BADOFFSET;
-	else if (offset == -FDT_ERR_BADOFFSET)
-		return -FDT_ERR_BADSTRUCTURE;
+	if (!can_assume(VALID_INPUT)) {
+		if ((offset == -FDT_ERR_NOTFOUND) || (offset >= 0))
+			return -FDT_ERR_BADOFFSET;
+		else if (offset == -FDT_ERR_BADOFFSET)
+			return -FDT_ERR_BADSTRUCTURE;
+	}
 
 	return offset; /* error from fdt_next_node() */
 }
@@ -614,7 +634,8 @@ int fdt_node_depth(const void *fdt, int nodeoffset)
 
 	err = fdt_supernode_atdepth_offset(fdt, nodeoffset, 0, &nodedepth);
 	if (err)
-		return (err < 0) ? err : -FDT_ERR_INTERNAL;
+		return (can_assume(VALID_INPUT) || err < 0) ? err :
+			-FDT_ERR_INTERNAL;
 	return nodedepth;
 }
 
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v6 5/8] libfdt: Add support for disabling rollback handling

[Simon Glass]

Allow enabling FDT_ASSUME_NO_ROLLBACK to disable rolling back after a
failed operation.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 libfdt/fdt_rw.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 707c00a..4a95ce2 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -114,6 +114,15 @@ static int fdt_splice_string_(void *fdt, int newlen)
 	return 0;
 }
 
+/**
+ * fdt_find_add_string_() - Find or allocate a string
+ *
+ * @fdt: pointer to the device tree to check/adjust
+ * @s: string to find/add
+ * @allocated: Set to 0 if the string was found, 1 if not found and so
+ *	allocated. Ignored if can_assume(NO_ROLLBACK)
+ * @return offset of string in the string table (whether found or added)
+ */
 static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
 {
 	char *strtab = (char *)fdt + fdt_off_dt_strings(fdt);
@@ -122,7 +131,8 @@ static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
 	int len = strlen(s) + 1;
 	int err;
 
-	*allocated = 0;
+	if (!can_assume(NO_ROLLBACK))
+		*allocated = 0;
 
 	p = fdt_find_string_(strtab, fdt_size_dt_strings(fdt), s);
 	if (p)
@@ -134,7 +144,8 @@ static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
 	if (err)
 		return err;
 
-	*allocated = 1;
+	if (!can_assume(NO_ROLLBACK))
+		*allocated = 1;
 
 	memcpy(new, s, len);
 	return (new - strtab);
@@ -208,7 +219,8 @@ static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
 
 	err = fdt_splice_struct_(fdt, *prop, 0, proplen);
 	if (err) {
-		if (allocated)
+		/* Delete the string if we failed to add it */
+		if (!can_assume(NO_ROLLBACK) && allocated)
 			fdt_del_last_string_(fdt, name);
 		return err;
 	}
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v6 6/8] libfdt: Add support for disabling version checks

[Simon Glass]

Allow enabling FDT_ASSUME_LATEST to disable version checks.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 libfdt/fdt.c    | 34 +++++++++++++++++++++-------------
 libfdt/fdt_ro.c | 16 ++++++++--------
 libfdt/fdt_rw.c |  6 +++---
 3 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index f6600ff..78d3b87 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -24,10 +24,13 @@ int32_t fdt_ro_probe_(const void *fdt)
 
 	if (fdt_magic(fdt) == FDT_MAGIC) {
 		/* Complete tree */
-		if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
-			return -FDT_ERR_BADVERSION;
-		if (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION)
-			return -FDT_ERR_BADVERSION;
+		if (!can_assume(LATEST)) {
+			if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
+				return -FDT_ERR_BADVERSION;
+			if (fdt_last_comp_version(fdt) >
+					FDT_LAST_SUPPORTED_VERSION)
+				return -FDT_ERR_BADVERSION;
+		}
 	} else if (fdt_magic(fdt) == FDT_SW_MAGIC) {
 		/* Unfinished sequential-write blob */
 		if (fdt_size_dt_struct(fdt) == 0)
@@ -75,7 +78,8 @@ size_t fdt_header_size_(uint32_t version)
 
 size_t fdt_header_size(const void *fdt)
 {
-	return fdt_header_size_(fdt_version(fdt));
+	return can_assume(LATEST) ? FDT_V17_SIZE :
+		fdt_header_size_(fdt_version(fdt));
 }
 
 int fdt_check_header(const void *fdt)
@@ -84,11 +88,14 @@ int fdt_check_header(const void *fdt)
 
 	if (fdt_magic(fdt) != FDT_MAGIC)
 		return -FDT_ERR_BADMAGIC;
-	if ((fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
-	    || (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION))
-		return -FDT_ERR_BADVERSION;
-	if (fdt_version(fdt) < fdt_last_comp_version(fdt))
-		return -FDT_ERR_BADVERSION;
+	if (!can_assume(LATEST)) {
+		if ((fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
+		    || (fdt_last_comp_version(fdt) >
+			FDT_LAST_SUPPORTED_VERSION))
+			return -FDT_ERR_BADVERSION;
+		if (fdt_version(fdt) < fdt_last_comp_version(fdt))
+			return -FDT_ERR_BADVERSION;
+	}
 	hdrsize = fdt_header_size(fdt);
 	if (!can_assume(VALID_DTB)) {
 
@@ -104,7 +111,7 @@ int fdt_check_header(const void *fdt)
 
 	if (!can_assume(VALID_DTB)) {
 		/* Bounds check structure block */
-		if (fdt_version(fdt) < 17) {
+		if (!can_assume(LATEST) && fdt_version(fdt) < 17) {
 			if (!check_off_(hdrsize, fdt_totalsize(fdt),
 					fdt_off_dt_struct(fdt)))
 				return -FDT_ERR_TRUNCATED;
@@ -135,7 +142,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
 		    || (absoffset + len) > fdt_totalsize(fdt))
 			return NULL;
 
-	if (fdt_version(fdt) >= 0x11)
+	if (can_assume(LATEST) || fdt_version(fdt) >= 0x11)
 		if (((offset + len) < offset)
 		    || ((offset + len) > fdt_size_dt_struct(fdt)))
 			return NULL;
@@ -175,7 +182,8 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
 		/* skip-name offset, length and value */
 		offset += sizeof(struct fdt_property) - FDT_TAGSIZE
 			+ fdt32_to_cpu(*lenp);
-		if (fdt_version(fdt) < 0x10 && fdt32_to_cpu(*lenp) >= 8 &&
+		if (!can_assume(LATEST) &&
+		    fdt_version(fdt) < 0x10 && fdt32_to_cpu(*lenp) >= 8 &&
 		    ((offset - fdt32_to_cpu(*lenp)) % 8) != 0)
 			offset += 4;
 		break;
diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 9c32559..2fa768e 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -60,7 +60,7 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
 	if (fdt_magic(fdt) == FDT_MAGIC) {
 		if (stroffset < 0)
 			goto fail;
-		if (fdt_version(fdt) >= 17) {
+		if (can_assume(LATEST) || fdt_version(fdt) >= 17) {
 			if (stroffset >= fdt_size_dt_strings(fdt))
 				goto fail;
 			if ((fdt_size_dt_strings(fdt) - stroffset) < len)
@@ -307,7 +307,7 @@ const char *fdt_get_name(const void *fdt, int nodeoffset, int *len)
 
 	nameptr = nh->name;
 
-	if (fdt_version(fdt) < 0x10) {
+	if (!can_assume(LATEST) && fdt_version(fdt) < 0x10) {
 		/*
 		 * For old FDT versions, match the naming conventions of V16:
 		 * give only the leaf name (after all /). The actual tree
@@ -380,7 +380,7 @@ const struct fdt_property *fdt_get_property_by_offset(const void *fdt,
 	/* Prior to version 16, properties may need realignment
 	 * and this API does not work. fdt_getprop_*() will, however. */
 
-	if (fdt_version(fdt) < 0x10) {
+	if (!can_assume(LATEST) && fdt_version(fdt) < 0x10) {
 		if (lenp)
 			*lenp = -FDT_ERR_BADVERSION;
 		return NULL;
@@ -427,7 +427,7 @@ const struct fdt_property *fdt_get_property_namelen(const void *fdt,
 {
 	/* Prior to version 16, properties may need realignment
 	 * and this API does not work. fdt_getprop_*() will, however. */
-	if (fdt_version(fdt) < 0x10) {
+	if (!can_assume(LATEST) && fdt_version(fdt) < 0x10) {
 		if (lenp)
 			*lenp = -FDT_ERR_BADVERSION;
 		return NULL;
@@ -458,8 +458,8 @@ const void *fdt_getprop_namelen(const void *fdt, int nodeoffset,
 		return NULL;
 
 	/* Handle realignment */
-	if (fdt_version(fdt) < 0x10 && (poffset + sizeof(*prop)) % 8 &&
-	    fdt32_ld(&prop->len) >= 8)
+	if (!can_assume(LATEST) && fdt_version(fdt) < 0x10 &&
+	    (poffset + sizeof(*prop)) % 8 && fdt32_ld(&prop->len) >= 8)
 		return prop->data + 4;
 	return prop->data;
 }
@@ -491,8 +491,8 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
 	}
 
 	/* Handle realignment */
-	if (fdt_version(fdt) < 0x10 && (offset + sizeof(*prop)) % 8 &&
-	    fdt32_ld(&prop->len) >= 8)
+	if (!can_assume(LATEST) && fdt_version(fdt) < 0x10 &&
+	    (offset + sizeof(*prop)) % 8 && fdt32_ld(&prop->len) >= 8)
 		return prop->data + 4;
 	return prop->data;
 }
diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 4a95ce2..fe13671 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -28,12 +28,12 @@ static int fdt_rw_probe_(void *fdt)
 		return 0;
 	FDT_RO_PROBE(fdt);
 
-	if (fdt_version(fdt) < 17)
+	if (!can_assume(LATEST) && fdt_version(fdt) < 17)
 		return -FDT_ERR_BADVERSION;
 	if (fdt_blocks_misordered_(fdt, sizeof(struct fdt_reserve_entry),
 				   fdt_size_dt_struct(fdt)))
 		return -FDT_ERR_BADLAYOUT;
-	if (fdt_version(fdt) > 17)
+	if (!can_assume(LATEST) && fdt_version(fdt) > 17)
 		fdt_set_version(fdt, 17);
 
 	return 0;
@@ -425,7 +425,7 @@ int fdt_open_into(const void *fdt, void *buf, int bufsize)
 	mem_rsv_size = (fdt_num_mem_rsv(fdt)+1)
 		* sizeof(struct fdt_reserve_entry);
 
-	if (fdt_version(fdt) >= 17) {
+	if (can_assume(LATEST) || fdt_version(fdt) >= 17) {
 		struct_size = fdt_size_dt_struct(fdt);
 	} else {
 		struct_size = 0;
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v6 7/8] libfdt: Add support for disabling ordering check/fixup

[Simon Glass]

Add a way to remove this check and the reordering code, which is
unnecessary if the dtb is known to be correctly ordered.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v6:
- Add a new patch to disable the ordering check and fixup

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 libfdt/fdt_rw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index fe13671..0d9d1b2 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -435,7 +435,8 @@ int fdt_open_into(const void *fdt, void *buf, int bufsize)
 			return struct_size;
 	}
 
-	if (!fdt_blocks_misordered_(fdt, mem_rsv_size, struct_size)) {
+	if (can_assume(CORRECT_ORDER) |
+	    !fdt_blocks_misordered_(fdt, mem_rsv_size, struct_size)) {
 		/* no further work necessary */
 		err = fdt_move(fdt, buf, bufsize);
 		if (err)
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v6 8/8] libfdt: Allow exclusion of fdt_check_full()

[Simon Glass]

This function is used to perform a full check of the device tree. Allow
it to be excluded if all assumptions are enabled.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3:
- Fix 'santiy' typo
- Instead of excluding fdt_check_full() put it in its own file
- Rearrange code in terms of checks instead of files changed, to aid review
- Replace 'unsigned' with 'unsigned int'
- Update commit message a little

Changes in v2:
- Add a comment to fdt_find_add_string_()
- Correct inverted version checks in a few cases
- Drop optimised code path in fdt_nodename_eq_()
- Update to use new check functions
- Use fdt_chk_base() in fdt_blocks_misordered_()

 libfdt/Makefile.libfdt |  2 +-
 libfdt/fdt_check.c     | 74 ++++++++++++++++++++++++++++++++++++++++++
 libfdt/fdt_ro.c        | 63 -----------------------------------
 3 files changed, 75 insertions(+), 64 deletions(-)
 create mode 100644 libfdt/fdt_check.c

diff --git a/libfdt/Makefile.libfdt b/libfdt/Makefile.libfdt
index e546397..b6d8fc0 100644
--- a/libfdt/Makefile.libfdt
+++ b/libfdt/Makefile.libfdt
@@ -8,7 +8,7 @@ LIBFDT_soname = libfdt.$(SHAREDLIB_EXT).1
 LIBFDT_INCLUDES = fdt.h libfdt.h libfdt_env.h
 LIBFDT_VERSION = version.lds
 LIBFDT_SRCS = fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c fdt_empty_tree.c \
-	fdt_addresses.c fdt_overlay.c
+	fdt_addresses.c fdt_overlay.c fdt_check.c
 LIBFDT_OBJS = $(LIBFDT_SRCS:%.c=%.o)
 LIBFDT_LIB = libfdt-$(DTC_VERSION).$(SHAREDLIB_EXT)
 
diff --git a/libfdt/fdt_check.c b/libfdt/fdt_check.c
new file mode 100644
index 0000000..7f6a96c
--- /dev/null
+++ b/libfdt/fdt_check.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+/*
+ * libfdt - Flat Device Tree manipulation
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ */
+#include "libfdt_env.h"
+
+#include <fdt.h>
+#include <libfdt.h>
+
+#include "libfdt_internal.h"
+
+int fdt_check_full(const void *fdt, size_t bufsize)
+{
+	int err;
+	int num_memrsv;
+	int offset, nextoffset = 0;
+	uint32_t tag;
+	unsigned int depth = 0;
+	const void *prop;
+	const char *propname;
+
+	if (bufsize < FDT_V1_SIZE)
+		return -FDT_ERR_TRUNCATED;
+	err = fdt_check_header(fdt);
+	if (err != 0)
+		return err;
+	if (bufsize < fdt_totalsize(fdt))
+		return -FDT_ERR_TRUNCATED;
+
+	num_memrsv = fdt_num_mem_rsv(fdt);
+	if (num_memrsv < 0)
+		return num_memrsv;
+
+	while (1) {
+		offset = nextoffset;
+		tag = fdt_next_tag(fdt, offset, &nextoffset);
+
+		if (nextoffset < 0)
+			return nextoffset;
+
+		switch (tag) {
+		case FDT_NOP:
+			break;
+
+		case FDT_END:
+			if (depth != 0)
+				return -FDT_ERR_BADSTRUCTURE;
+			return 0;
+
+		case FDT_BEGIN_NODE:
+			depth++;
+			if (depth > INT_MAX)
+				return -FDT_ERR_BADSTRUCTURE;
+			break;
+
+		case FDT_END_NODE:
+			if (depth == 0)
+				return -FDT_ERR_BADSTRUCTURE;
+			depth--;
+			break;
+
+		case FDT_PROP:
+			prop = fdt_getprop_by_offset(fdt, offset, &propname,
+						     &err);
+			if (!prop)
+				return err;
+			break;
+
+		default:
+			return -FDT_ERR_INTERNAL;
+		}
+	}
+}
diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 2fa768e..1945503 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -855,66 +855,3 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
 
 	return offset; /* error from fdt_next_node() */
 }
-
-int fdt_check_full(const void *fdt, size_t bufsize)
-{
-	int err;
-	int num_memrsv;
-	int offset, nextoffset = 0;
-	uint32_t tag;
-	unsigned depth = 0;
-	const void *prop;
-	const char *propname;
-
-	if (bufsize < FDT_V1_SIZE)
-		return -FDT_ERR_TRUNCATED;
-	err = fdt_check_header(fdt);
-	if (err != 0)
-		return err;
-	if (bufsize < fdt_totalsize(fdt))
-		return -FDT_ERR_TRUNCATED;
-
-	num_memrsv = fdt_num_mem_rsv(fdt);
-	if (num_memrsv < 0)
-		return num_memrsv;
-
-	while (1) {
-		offset = nextoffset;
-		tag = fdt_next_tag(fdt, offset, &nextoffset);
-
-		if (nextoffset < 0)
-			return nextoffset;
-
-		switch (tag) {
-		case FDT_NOP:
-			break;
-
-		case FDT_END:
-			if (depth != 0)
-				return -FDT_ERR_BADSTRUCTURE;
-			return 0;
-
-		case FDT_BEGIN_NODE:
-			depth++;
-			if (depth > INT_MAX)
-				return -FDT_ERR_BADSTRUCTURE;
-			break;
-
-		case FDT_END_NODE:
-			if (depth == 0)
-				return -FDT_ERR_BADSTRUCTURE;
-			depth--;
-			break;
-
-		case FDT_PROP:
-			prop = fdt_getprop_by_offset(fdt, offset, &propname,
-						     &err);
-			if (!prop)
-				return err;
-			break;
-
-		default:
-			return -FDT_ERR_INTERNAL;
-		}
-	}
-}
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH v6 1/8] libfdt: De-inline fdt_header_size()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2917 bytes --]

On Tue, Feb 11, 2020 at 01:09:38PM -0700, Simon Glass wrote:
> There does not seem to be a strong reason to inline this function. Also
> we are about to add some extra code to it which will increase its size.
> 
> Move it into fdt.c and use a simple declaration in libfdt.h
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> 
> Changes in v6: None
> Changes in v5: None
> Changes in v4:
> - Add fdt_header_size to version.lds
> 
> Changes in v3:
> - Add a new patch to de-inline fdt_header_size()
> 
> Changes in v2: None
> 
>  libfdt/fdt.c       | 5 +++++
>  libfdt/libfdt.h    | 9 +++++----
>  libfdt/version.lds | 1 +
>  tests/testutils.c  | 1 -
>  4 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index d6ce7c0..3e37a4b 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -70,6 +70,11 @@ size_t fdt_header_size_(uint32_t version)
>  		return FDT_V17_SIZE;
>  }
>  
> +size_t fdt_header_size(const void *fdt)
> +{
> +	return fdt_header_size_(fdt_version(fdt));
> +}
> +
>  int fdt_check_header(const void *fdt)
>  {
>  	size_t hdrsize;
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index fc4c496..48f375c 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -266,11 +266,12 @@ fdt_set_hdr_(size_dt_struct);
>   * fdt_header_size - return the size of the tree's header
>   * @fdt: pointer to a flattened device tree
>   */
> +size_t fdt_header_size(const void *fdt);
> +
> +/**
> + * fdt_header_size_ - internal function which takes a version number
> + */
>  size_t fdt_header_size_(uint32_t version);
> -static inline size_t fdt_header_size(const void *fdt)
> -{
> -	return fdt_header_size_(fdt_version(fdt));
> -}
>  
>  /**
>   * fdt_check_header - sanity check a device tree header
> diff --git a/libfdt/version.lds b/libfdt/version.lds
> index ae32924..7ab85f1 100644
> --- a/libfdt/version.lds
> +++ b/libfdt/version.lds
> @@ -20,6 +20,7 @@ LIBFDT_1.2 {
>  		fdt_get_alias_namelen;
>  		fdt_get_alias;
>  		fdt_get_path;
> +                fdt_header_size;
>  		fdt_supernode_atdepth_offset;
>  		fdt_node_depth;
>  		fdt_parent_offset;
> diff --git a/tests/testutils.c b/tests/testutils.c
> index 5e494c5..53cb35f 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -261,7 +261,6 @@ void vg_prepare_blob(void *fdt, size_t bufsize)
>  
>  	VALGRIND_MAKE_MEM_UNDEFINED(blob, bufsize);
>  	VALGRIND_MAKE_MEM_DEFINED(blob, FDT_V1_SIZE);
> -	VALGRIND_MAKE_MEM_DEFINED(blob, fdt_header_size(fdt));

Uh.. this doesn't look right..

>  
>  	if (fdt_magic(fdt) == FDT_MAGIC) {
>  		off_strings = fdt_off_dt_strings(fdt);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 2/8] Add a way to control the level of checks in the code

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On Tue, Feb 11, 2020 at 01:09:39PM -0700, Simon Glass wrote:
> Add a new ASSUME_MASK option, which allows for some control over the
> checks used in libfdt. With all assumptions enabled, libfdt assumes that
> the input data and parameters are all correct and that internal errors
> cannot happen.
> 
> By default no assumptions are made and all checks are enabled.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

LGTM, except...

[snip]
> +	/*
> +	 * This assumes that the device tree components appear in the correct
> +	 * order. As such it disables a check in fdt_open_into() and removes the
> +	 * ability to fix the problem there. This is safe if you know that the
> +	 * device tree is correctly ordered. See fdt_blocks_misordered_().
> +	 */
> +	ASSUME_CORRECT_ORDER	= 1 << 4,

.. I don't like this name.  The spec doesn't specify an order, so
having it in a different order isn't incorrect, just a bit harder to
work with.

> +};
> +
> +/**
> + * can_assume_() - check if a particular assumption is enabled
> + *
> + * @mask: Mask to check (ASSUME_...)
> + * @return true if that assumption is enabled, else false
> + */
> +static inline bool can_assume_(int mask)
> +{
> +	return FDT_ASSUME_MASK & mask;
> +}
> +
> +/** helper macros for checking assumptions */
> +#define can_assume(_assume)	can_assume_(ASSUME_ ## _assume)
> +
>  #endif /* LIBFDT_INTERNAL_H */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 3/8] libfdt: Add support for disabling dtb checks

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7166 bytes --]

On Tue, Feb 11, 2020 at 01:09:40PM -0700, Simon Glass wrote:
> Support ASSUME_VALID_DTB to disable some sanity checks
> 
> If we assume that the DTB itself is valid then we can skip some checks and
> save code space. Add various conditions to handle this.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> 
> Changes in v6:
> - Always call fdt_ro_probe_(), etc. and have that function do the check
> - Change check in fdt_get_property_namelen_() to VALID_DTB
> 
> Changes in v5:
> - Split out VALID_DTB checks into a separate patch
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  libfdt/fdt.c    | 53 +++++++++++++++++++++++++++++--------------------
>  libfdt/fdt_ro.c |  3 ++-
>  libfdt/fdt_rw.c |  2 ++
>  libfdt/fdt_sw.c | 19 +++++++++++-------
>  4 files changed, 47 insertions(+), 30 deletions(-)
> 
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index 3e37a4b..d4daf60 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -19,6 +19,9 @@ int32_t fdt_ro_probe_(const void *fdt)
>  {
>  	uint32_t totalsize = fdt_totalsize(fdt);
>  
> +	if (can_assume(VALID_DTB))
> +		return totalsize;
> +
>  	if (fdt_magic(fdt) == FDT_MAGIC) {
>  		/* Complete tree */
>  		if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
> @@ -81,38 +84,44 @@ int fdt_check_header(const void *fdt)
>  
>  	if (fdt_magic(fdt) != FDT_MAGIC)
>  		return -FDT_ERR_BADMAGIC;
> -	hdrsize = fdt_header_size(fdt);
>  	if ((fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
>  	    || (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION))
>  		return -FDT_ERR_BADVERSION;
>  	if (fdt_version(fdt) < fdt_last_comp_version(fdt))
>  		return -FDT_ERR_BADVERSION;
> +	hdrsize = fdt_header_size(fdt);
> +	if (!can_assume(VALID_DTB)) {
>  
> -	if ((fdt_totalsize(fdt) < hdrsize)
> -	    || (fdt_totalsize(fdt) > INT_MAX))
> -		return -FDT_ERR_TRUNCATED;
> -
> -	/* Bounds check memrsv block */
> -	if (!check_off_(hdrsize, fdt_totalsize(fdt), fdt_off_mem_rsvmap(fdt)))
> -		return -FDT_ERR_TRUNCATED;
> +		if ((fdt_totalsize(fdt) < hdrsize)
> +		    || (fdt_totalsize(fdt) > INT_MAX))
> +			return -FDT_ERR_TRUNCATED;
>  
> -	/* Bounds check structure block */
> -	if (fdt_version(fdt) < 17) {
> +		/* Bounds check memrsv block */
>  		if (!check_off_(hdrsize, fdt_totalsize(fdt),
> -				fdt_off_dt_struct(fdt)))
> +				fdt_off_mem_rsvmap(fdt)))
>  			return -FDT_ERR_TRUNCATED;
> -	} else {
> +	}
> +
> +	if (!can_assume(VALID_DTB)) {
> +		/* Bounds check structure block */
> +		if (fdt_version(fdt) < 17) {
> +			if (!check_off_(hdrsize, fdt_totalsize(fdt),
> +					fdt_off_dt_struct(fdt)))
> +				return -FDT_ERR_TRUNCATED;
> +		} else {
> +			if (!check_block_(hdrsize, fdt_totalsize(fdt),
> +					  fdt_off_dt_struct(fdt),
> +					  fdt_size_dt_struct(fdt)))
> +				return -FDT_ERR_TRUNCATED;
> +		}
> +
> +		/* Bounds check strings block */
>  		if (!check_block_(hdrsize, fdt_totalsize(fdt),
> -				  fdt_off_dt_struct(fdt),
> -				  fdt_size_dt_struct(fdt)))
> +				  fdt_off_dt_strings(fdt),
> +				  fdt_size_dt_strings(fdt)))
>  			return -FDT_ERR_TRUNCATED;
>  	}
>  
> -	/* Bounds check strings block */
> -	if (!check_block_(hdrsize, fdt_totalsize(fdt),
> -			  fdt_off_dt_strings(fdt), fdt_size_dt_strings(fdt)))
> -		return -FDT_ERR_TRUNCATED;
> -
>  	return 0;
>  }
>  
> @@ -142,7 +151,7 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>  
>  	*nextoffset = -FDT_ERR_TRUNCATED;
>  	tagp = fdt_offset_ptr(fdt, offset, FDT_TAGSIZE);
> -	if (!tagp)
> +	if (!can_assume(VALID_DTB) && !tagp)
>  		return FDT_END; /* premature end */
>  	tag = fdt32_to_cpu(*tagp);
>  	offset += FDT_TAGSIZE;
> @@ -154,13 +163,13 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>  		do {
>  			p = fdt_offset_ptr(fdt, offset++, 1);
>  		} while (p && (*p != '\0'));
> -		if (!p)
> +		if (!can_assume(VALID_DTB) && !p)
>  			return FDT_END; /* premature end */
>  		break;
>  
>  	case FDT_PROP:
>  		lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp));
> -		if (!lenp)
> +		if (!can_assume(VALID_DTB) && !lenp)
>  			return FDT_END; /* premature end */
>  		/* skip-name offset, length and value */
>  		offset += sizeof(struct fdt_property) - FDT_TAGSIZE
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index a5c2797..4c26fbe 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -388,7 +388,8 @@ static const struct fdt_property *fdt_get_property_namelen_(const void *fdt,
>  	     (offset = fdt_next_property_offset(fdt, offset))) {
>  		const struct fdt_property *prop;
>  
> -		if (!(prop = fdt_get_property_by_offset_(fdt, offset, lenp))) {
> +		prop = fdt_get_property_by_offset_(fdt, offset, lenp);
> +		if (!can_assume(VALID_DTB) && !prop) {
>  			offset = -FDT_ERR_INTERNAL;
>  			break;
>  		}
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index 8795947..707c00a 100644
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -24,6 +24,8 @@ static int fdt_blocks_misordered_(const void *fdt,
>  
>  static int fdt_rw_probe_(void *fdt)
>  {
> +	if (can_assume(VALID_DTB))
> +		return 0;
>  	FDT_RO_PROBE(fdt);
>  
>  	if (fdt_version(fdt) < 17)
> diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c
> index 76bea22..96365b4 100644
> --- a/libfdt/fdt_sw.c
> +++ b/libfdt/fdt_sw.c
> @@ -12,10 +12,13 @@
>  
>  static int fdt_sw_probe_(void *fdt)
>  {
> -	if (fdt_magic(fdt) == FDT_MAGIC)
> -		return -FDT_ERR_BADSTATE;
> -	else if (fdt_magic(fdt) != FDT_SW_MAGIC)
> -		return -FDT_ERR_BADMAGIC;
> +	if (!can_assume(VALID_DTB)) {
> +		if (fdt_magic(fdt) == FDT_MAGIC)
> +			return -FDT_ERR_BADSTATE;
> +		else if (fdt_magic(fdt) != FDT_SW_MAGIC)
> +			return -FDT_ERR_BADMAGIC;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -38,7 +41,7 @@ static int fdt_sw_probe_memrsv_(void *fdt)
>  	if (err)
>  		return err;
>  
> -	if (fdt_off_dt_strings(fdt) != 0)
> +	if (!can_assume(VALID_DTB) && fdt_off_dt_strings(fdt) != 0)
>  		return -FDT_ERR_BADSTATE;
>  	return 0;
>  }
> @@ -46,7 +49,8 @@ static int fdt_sw_probe_memrsv_(void *fdt)
>  #define FDT_SW_PROBE_MEMRSV(fdt) \
>  	{ \
>  		int err; \
> -		if ((err = fdt_sw_probe_memrsv_(fdt)) != 0) \
> +		if (!can_assume(VALID_DTB) && \
> +		    (err = fdt_sw_probe_memrsv_(fdt)) != 0) \

Nit: with the can_assume() inside fdt_sw_probe_memrsv_(), I don't
think you need it here as well.

Otherwise, LGTM.

>  			return err; \
>  	}
>  
> @@ -151,7 +155,8 @@ int fdt_resize(void *fdt, void *buf, int bufsize)
>  	headsize = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
>  	tailsize = fdt_size_dt_strings(fdt);
>  
> -	if ((headsize + tailsize) > fdt_totalsize(fdt))
> +	if (!can_assume(VALID_DTB) &&
> +	    headsize + tailsize > fdt_totalsize(fdt))
>  		return -FDT_ERR_INTERNAL;
>  
>  	if ((headsize + tailsize) > bufsize)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 4/8] libfdt: Add support for disabling sanity checks

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 6413 bytes --]

On Tue, Feb 11, 2020 at 01:09:41PM -0700, Simon Glass wrote:
> Allow enabling ASSUME_VALID_INPUT to disable sanity checks on the device
> tree and the parameters to libfdt. This assumption covers that cases where
> the problem could be with either.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> 
> Changes in v6:
> - Don't add VALID_INPUT to fdt_get_property_namelen_()
> - Drop assumption in fdt_nodename_eq_()
> 
> Changes in v5:
> - Include just VALID_INPUT checks in this patch
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  libfdt/fdt.c    | 14 ++++++++----
>  libfdt/fdt_ro.c | 61 +++++++++++++++++++++++++++++++++----------------
>  2 files changed, 50 insertions(+), 25 deletions(-)
> 
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index d4daf60..f6600ff 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -129,10 +129,11 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
>  {
>  	unsigned absoffset = offset + fdt_off_dt_struct(fdt);
>  
> -	if ((absoffset < offset)
> -	    || ((absoffset + len) < absoffset)
> -	    || (absoffset + len) > fdt_totalsize(fdt))
> -		return NULL;
> +	if (!can_assume(VALID_INPUT))
> +		if ((absoffset < offset)
> +		    || ((absoffset + len) < absoffset)
> +		    || (absoffset + len) > fdt_totalsize(fdt))
> +			return NULL;
>  
>  	if (fdt_version(fdt) >= 0x11)
>  		if (((offset + len) < offset)
> @@ -188,7 +189,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_INPUT) &&
> +	    !fdt_offset_ptr(fdt, startoffset, offset - startoffset))

If fdt_offset_ptr() has a can_assume() inside, can't some of these
calls to it avoid the can_assume()?

>  		return FDT_END; /* premature end */
>  
>  	*nextoffset = FDT_TAGALIGN(offset);
> @@ -197,6 +199,8 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>  
>  int fdt_check_node_offset_(const void *fdt, int offset)
>  {
> +	if (can_assume(VALID_INPUT))
> +		return offset;
>  	if ((offset < 0) || (offset % FDT_TAGSIZE)
>  	    || (fdt_next_tag(fdt, offset, &offset) != FDT_BEGIN_NODE))
>  		return -FDT_ERR_BADOFFSET;
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index 4c26fbe..9c32559 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -33,17 +33,26 @@ static int fdt_nodename_eq_(const void *fdt, int offset,
>  
>  const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
>  {
> -	int32_t totalsize = fdt_ro_probe_(fdt);
> -	uint32_t absoffset = stroffset + fdt_off_dt_strings(fdt);
> +	int32_t totalsize;
> +	uint32_t absoffset;
>  	size_t len;
>  	int err;
>  	const char *s, *n;
>  
> +	if (can_assume(VALID_INPUT)) {
> +		s = (const char *)fdt + fdt_off_dt_strings(fdt) + stroffset;
> +
> +		if (lenp)
> +			*lenp = strlen(s);
> +		return s;
> +	}
> +	totalsize = fdt_ro_probe_(fdt);
>  	err = totalsize;
>  	if (totalsize < 0)
>  		goto fail;
>  
>  	err = -FDT_ERR_BADOFFSET;
> +	absoffset = stroffset + fdt_off_dt_strings(fdt);
>  	if (absoffset >= totalsize)
>  		goto fail;
>  	len = totalsize - absoffset;
> @@ -151,10 +160,13 @@ static const struct fdt_reserve_entry *fdt_mem_rsv(const void *fdt, int n)
>  	int offset = n * sizeof(struct fdt_reserve_entry);
>  	int absoffset = fdt_off_mem_rsvmap(fdt) + offset;
>  
> -	if (absoffset < fdt_off_mem_rsvmap(fdt))
> -		return NULL;
> -	if (absoffset > fdt_totalsize(fdt) - sizeof(struct fdt_reserve_entry))
> -		return NULL;
> +	if (!can_assume(VALID_INPUT)) {
> +		if (absoffset < fdt_off_mem_rsvmap(fdt))
> +			return NULL;
> +		if (absoffset > fdt_totalsize(fdt) -
> +		    sizeof(struct fdt_reserve_entry))
> +			return NULL;
> +	}
>  	return fdt_mem_rsv_(fdt, n);
>  }
>  
> @@ -164,7 +176,7 @@ int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size)
>  
>  	FDT_RO_PROBE(fdt);
>  	re = fdt_mem_rsv(fdt, n);
> -	if (!re)
> +	if (!can_assume(VALID_INPUT) && !re)
>  		return -FDT_ERR_BADOFFSET;
>  
>  	*address = fdt64_ld(&re->address);
> @@ -346,7 +358,8 @@ static const struct fdt_property *fdt_get_property_by_offset_(const void *fdt,
>  	int err;
>  	const struct fdt_property *prop;
>  
> -	if ((err = fdt_check_prop_offset_(fdt, offset)) < 0) {
> +	if (!can_assume(VALID_INPUT) &&
> +	    (err = fdt_check_prop_offset_(fdt, offset)) < 0) {
>  		if (lenp)
>  			*lenp = err;
>  		return NULL;
> @@ -462,14 +475,19 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
>  	if (namep) {
>  		const char *name;
>  		int namelen;
> -		name = fdt_get_string(fdt, fdt32_ld(&prop->nameoff),
> -				      &namelen);
> -		if (!name) {
> -			if (lenp)
> -				*lenp = namelen;
> -			return NULL;
> +
> +		if (!can_assume(VALID_INPUT)) {
> +			name = fdt_get_string(fdt, fdt32_ld(&prop->nameoff),
> +					      &namelen);
> +			if (!name) {
> +				if (lenp)
> +					*lenp = namelen;
> +				return NULL;
> +			}
> +			*namep = name;
> +		} else {
> +			*namep = fdt_string(fdt, fdt32_ld(&prop->nameoff));
>  		}
> -		*namep = name;
>  	}
>  
>  	/* Handle realignment */
> @@ -599,10 +617,12 @@ int fdt_supernode_atdepth_offset(const void *fdt, int nodeoffset,
>  		}
>  	}
>  
> -	if ((offset == -FDT_ERR_NOTFOUND) || (offset >= 0))
> -		return -FDT_ERR_BADOFFSET;
> -	else if (offset == -FDT_ERR_BADOFFSET)
> -		return -FDT_ERR_BADSTRUCTURE;
> +	if (!can_assume(VALID_INPUT)) {
> +		if ((offset == -FDT_ERR_NOTFOUND) || (offset >= 0))
> +			return -FDT_ERR_BADOFFSET;
> +		else if (offset == -FDT_ERR_BADOFFSET)
> +			return -FDT_ERR_BADSTRUCTURE;
> +	}
>  
>  	return offset; /* error from fdt_next_node() */
>  }
> @@ -614,7 +634,8 @@ int fdt_node_depth(const void *fdt, int nodeoffset)
>  
>  	err = fdt_supernode_atdepth_offset(fdt, nodeoffset, 0, &nodedepth);
>  	if (err)
> -		return (err < 0) ? err : -FDT_ERR_INTERNAL;
> +		return (can_assume(VALID_INPUT) || err < 0) ? err :
> +			-FDT_ERR_INTERNAL;
>  	return nodedepth;
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 7/8] libfdt: Add support for disabling ordering check/fixup

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1387 bytes --]

On Tue, Feb 11, 2020 at 01:09:44PM -0700, Simon Glass wrote:
> Add a way to remove this check and the reordering code, which is
> unnecessary if the dtb is known to be correctly ordered.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

.. except for not liking the name.

> ---
> 
> Changes in v6:
> - Add a new patch to disable the ordering check and fixup
> 
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  libfdt/fdt_rw.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index fe13671..0d9d1b2 100644
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -435,7 +435,8 @@ int fdt_open_into(const void *fdt, void *buf, int bufsize)
>  			return struct_size;
>  	}
>  
> -	if (!fdt_blocks_misordered_(fdt, mem_rsv_size, struct_size)) {
> +	if (can_assume(CORRECT_ORDER) |
> +	    !fdt_blocks_misordered_(fdt, mem_rsv_size, struct_size)) {
>  		/* no further work necessary */
>  		err = fdt_move(fdt, buf, bufsize);
>  		if (err)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 7/8] libfdt: Add support for disabling ordering check/fixup

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1939 bytes --]

On Tue, Feb 11, 2020 at 10:26:21PM -0700, Simon Glass wrote:
> On Tue, Feb 11, 2020, 21:58 David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> wrote:
> 
> > On Tue, Feb 11, 2020 at 01:09:44PM -0700, Simon Glass wrote:
> > > Add a way to remove this check and the reordering code, which is
> > > unnecessary if the dtb is known to be correctly ordered.
> > >
> > > Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >
> > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> >
> > .. except for not liking the name.
> 
> NORMAL_ORDER?
> LIBFDT_ORDERING?
> EXPECTED_ORDER?

Oh, yes, I should have suggested something.  Ummm.....

let's go with LIBFDT_ORDER.

> 
> 
> > > ---
> > >
> > > Changes in v6:
> > > - Add a new patch to disable the ordering check and fixup
> > >
> > > Changes in v5: None
> > > Changes in v4: None
> > > Changes in v3: None
> > > Changes in v2: None
> > >
> > >  libfdt/fdt_rw.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> > > index fe13671..0d9d1b2 100644
> > > --- a/libfdt/fdt_rw.c
> > > +++ b/libfdt/fdt_rw.c
> > > @@ -435,7 +435,8 @@ int fdt_open_into(const void *fdt, void *buf, int
> > bufsize)
> > >                       return struct_size;
> > >       }
> > >
> > > -     if (!fdt_blocks_misordered_(fdt, mem_rsv_size, struct_size)) {
> > > +     if (can_assume(CORRECT_ORDER) |
> > > +         !fdt_blocks_misordered_(fdt, mem_rsv_size, struct_size)) {
> > >               /* no further work necessary */
> > >               err = fdt_move(fdt, buf, bufsize);
> > >               if (err)
> >
> >

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]