[PATCH 02/12] libfdt: Make fdt_check_header() more thorough

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Currently fdt_check_header() performs only some rudimentary checks, which
is not really what the name suggests.  This strengthens fdt_check_header()
to check as much about the blob as is possible from the header alone:  as
well as checking the magic number and version, it checks that the total
size is sane, and that all the sub-blocks within the blob lie within the
total size.

 * This broadens the meaning of FDT_ERR_TRUNCATED to cover all sorts of
   improperly terminated blocks as well as just a structure block without
   FDT_END.

 * This makes fdt_check_header() only succeed on "complete" blobs, not
   in-progress sequential write blobs.  The only reason this didn't fail
   before was that this function used to be called by many RO functions
   which are supposed to also work on incomplete SW blobs.

Signed-off-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
---
 libfdt/fdt.c         |  57 ++++++++++++++++++++++-
 libfdt/libfdt.h      |   5 +-
 libfdt/libfdt_env.h  |   1 +
 tests/.gitignore     |   1 +
 tests/Makefile.tests |   2 +-
 tests/check_header.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/run_tests.sh   |   3 ++
 7 files changed, 193 insertions(+), 4 deletions(-)
 create mode 100644 tests/check_header.c

diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index af2f513..4503e9c 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -74,9 +74,64 @@ int fdt_ro_probe_(const void *fdt)
 	return 0;
 }
 
+static int check_off_(uint32_t hdrsize, uint32_t totalsize, uint32_t off)
+{
+	return (off >= hdrsize) && (off <= totalsize);
+}
+
+static int check_block_(uint32_t hdrsize, uint32_t totalsize,
+			uint32_t base, uint32_t size)
+{
+	if (!check_off_(hdrsize, totalsize, base))
+		return 0; /* block start out of bounds */
+	if ((base + size) < base)
+		return 0; /* overflow */
+	if (!check_off_(hdrsize, totalsize, base + size))
+		return 0; /* block end out of bounds */
+	return 1;
+}
+
 int fdt_check_header(const void *fdt)
 {
-	return fdt_ro_probe_(fdt);
+	size_t hdrsize = FDT_V16_SIZE;
+
+	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 (fdt_version(fdt) >= 17)
+		hdrsize = FDT_V17_SIZE;
+
+	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;
+
+	/* 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_strings(fdt), fdt_size_dt_strings(fdt)))
+		return -FDT_ERR_TRUNCATED;
+
+	return 0;
 }
 
 const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index c8c00fa..1032d38 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -90,8 +90,9 @@
 
 /* Error codes: codes for bad device tree blobs */
 #define FDT_ERR_TRUNCATED	8
-	/* FDT_ERR_TRUNCATED: Structure block of the given device tree
-	 * ends without an FDT_END tag. */
+	/* FDT_ERR_TRUNCATED: FDT or a sub-block is improperly
+	 * terminated (overflows, goes outside allowed bounds, or
+	 * isn't properly terminated).  */
 #define FDT_ERR_BADMAGIC	9
 	/* FDT_ERR_BADMAGIC: Given "device tree" appears not to be a
 	 * device tree at all - it is missing the flattened device
diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
index bd24746..eb20538 100644
--- a/libfdt/libfdt_env.h
+++ b/libfdt/libfdt_env.h
@@ -56,6 +56,7 @@
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
+#include <limits.h>
 
 #ifdef __CHECKER__
 #define FDT_FORCE __attribute__((force))
diff --git a/tests/.gitignore b/tests/.gitignore
index 9e209d5..1c25219 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -8,6 +8,7 @@ tmp.*
 /asm_tree_dump
 /boot-cpuid
 /char_literal
+/check_header
 /check_path
 /del_node
 /del_property
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index 262944a..1f0ba99 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -26,7 +26,7 @@ LIB_TESTS_L = get_mem_rsv \
 	property_iterate \
 	subnode_iterate \
 	overlay overlay_bad_fixup \
-	check_path
+	check_path check_header
 LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
 
 LIBTREE_TESTS_L = truncated_property
diff --git a/tests/check_header.c b/tests/check_header.c
new file mode 100644
index 0000000..5e37813
--- /dev/null
+++ b/tests/check_header.c
@@ -0,0 +1,128 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ *	Testcase for fdt_check_header
+ * Copyright (C) 2018 David Gibson
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdio.h>
+
+#include <libfdt.h>
+
+#include "tests.h"
+
+static void *dtdup(void *dt)
+{
+	size_t bufsize = fdt_totalsize(dt);
+	void *buf = xmalloc(bufsize);
+	fdt_move(dt, buf, bufsize);
+	return buf;
+}
+
+#define CHECK_MANGLE(exerr, code)					\
+	do {								\
+		void *fdt = dtdup(template);				\
+		{ code }						\
+		err = fdt_check_header(fdt);				\
+		verbose_printf("\"%s\" => %s\n", #code, fdt_strerror(err)); \
+		if (err != (exerr))					\
+			FAIL("fdt_check_header() didn't catch mangle %s", \
+			     #code);					\
+		free(fdt);						\
+	} while (0)
+
+int main(int argc, char *argv[])
+{
+	void *template;
+	int err;
+
+	test_init(argc, argv);
+	template = load_blob(argv[1]);
+
+	/* Check that the base dt is valid before mangling it */
+	err = fdt_check_header(template);
+	if (err != 0)
+		FAIL("Base tree fails: %s", fdt_strerror(err));
+
+	/* Check a no-op mangle doesn't break things */
+	CHECK_MANGLE(0, ; );
+
+	/* Mess up the magic number */
+	CHECK_MANGLE(-FDT_ERR_BADMAGIC,
+		fdt_set_magic(fdt, fdt_magic(fdt) ^ 0x1);
+	);
+	CHECK_MANGLE(-FDT_ERR_BADMAGIC,
+		fdt_set_magic(fdt, fdt_magic(fdt) ^ 0x80000000);
+	);
+
+	/* Mess up the version */
+	CHECK_MANGLE(-FDT_ERR_BADVERSION,
+		fdt_set_version(fdt, FDT_FIRST_SUPPORTED_VERSION - 1);
+		fdt_set_last_comp_version(fdt, FDT_FIRST_SUPPORTED_VERSION - 1);
+	);
+	CHECK_MANGLE(-FDT_ERR_BADVERSION,
+		fdt_set_version(fdt, FDT_LAST_SUPPORTED_VERSION + 1);
+		fdt_set_last_comp_version(fdt, FDT_LAST_SUPPORTED_VERSION + 1);
+	);
+	CHECK_MANGLE(-FDT_ERR_BADVERSION,
+		fdt_set_version(fdt, FDT_FIRST_SUPPORTED_VERSION);
+		fdt_set_last_comp_version(fdt, FDT_LAST_SUPPORTED_VERSION);
+	);
+
+	/* Out of bounds sizes */
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_totalsize(fdt, FDT_V1_SIZE - 1);
+	);
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		     fdt_set_totalsize(fdt, (uint32_t)INT_MAX + 1);
+	);
+
+	/* Truncate within various blocks */
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_totalsize(fdt, fdt_off_dt_struct(fdt) - 1);
+	);
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_totalsize(fdt, fdt_off_dt_strings(fdt) - 1);
+	);
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_totalsize(fdt, fdt_off_mem_rsvmap(fdt) - 1);
+	);
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_totalsize(fdt, fdt_off_dt_struct(fdt) + 1);
+	);
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_totalsize(fdt, fdt_off_dt_strings(fdt) + 1);
+	);
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_totalsize(fdt, fdt_off_mem_rsvmap(fdt) + 1);
+	);
+
+	/* Negative block sizes */
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_size_dt_struct(fdt, (uint32_t)-1);
+	);
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_size_dt_strings(fdt, (uint32_t)-1);
+	);
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		     fdt_set_size_dt_struct(fdt, (uint32_t)INT_MIN);
+	);
+	CHECK_MANGLE(-FDT_ERR_TRUNCATED,
+		fdt_set_size_dt_strings(fdt, (uint32_t)INT_MIN);
+	);
+
+	PASS();
+}
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index d937260..f2a0198 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -440,6 +440,9 @@ libfdt_tests () {
     run_wrap_error_test $DTC nul-in-line-info2.dts
 
     run_wrap_error_test $DTC -I dtb -O dts -o /dev/null ovf_size_strings.dtb
+
+    # Check header tests
+    run_test check_header test_tree1.dtb
 }
 
 dtc_tests () {
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux