libfdt: Safer access to memory reservations

fdt_num_mem_rsv() and fdt_get_mem_rsv() currently don't sanity check their
parameters, or the memory reserve section offset in the header.  That means
that on a corrupted blob they could access outside of the range of memory
that they should.

This improves their safety checking, meaning they shouldn't access outside
the blob's bounds, even if its contents are badly corrupted.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

7 files changed, 114 insertions, 7 deletions

diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index d4cec0e..c74b962 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -170,21 +170,42 @@ uint32_t fdt_get_max_phandle(const void *fdt)
 	return 0;
 }
 
+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;
+	return fdt_mem_rsv_(fdt, n);
+}
+
 int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size)
 {
+	const struct fdt_reserve_entry *re;
+
 	FDT_RO_PROBE(fdt);
-	*address = fdt64_to_cpu(fdt_mem_rsv_(fdt, n)->address);
-	*size = fdt64_to_cpu(fdt_mem_rsv_(fdt, n)->size);
+	re = fdt_mem_rsv(fdt, n);
+	if (!re)
+		return -FDT_ERR_BADOFFSET;
+
+	*address = fdt64_to_cpu(re->address);
+	*size = fdt64_to_cpu(re->size);
 	return 0;
 }
 
 int fdt_num_mem_rsv(const void *fdt)
 {
-	int i = 0;
+	int i;
+	const struct fdt_reserve_entry *re;
 
-	while (fdt64_to_cpu(fdt_mem_rsv_(fdt, i)->size) != 0)
-		i++;
-	return i;
+	for (i = 0; (re = fdt_mem_rsv(fdt, i)) != NULL; i++) {
+		if (fdt64_to_cpu(re->size) == 0)
+			return i;
+	}
+	return -FDT_ERR_TRUNCATED;
 }
 
 static int nextprop_(const void *fdt, int offset)
diff --git a/tests/.gitignore b/tests/.gitignore
index 0db6d78..ea437ca 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -62,5 +62,6 @@ tmp.*
 /sw_states
 /truncated_property
 /truncated_string
+/truncated_memrsv
 /utilfdt_test
 /value-labels
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index 97f7f8d..489886f 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -29,7 +29,7 @@ LIB_TESTS_L = get_mem_rsv \
 	check_path check_header
 LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
 
-LIBTREE_TESTS_L = truncated_property truncated_string
+LIBTREE_TESTS_L = truncated_property truncated_string truncated_memrsv
 LIBTREE_TESTS = $(LIBTREE_TESTS_L:%=$(TESTS_PREFIX)%)
 
 DL_LIB_TESTS_L = asm_tree_dump value-labels
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 0674d0a..f80029a 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -416,6 +416,7 @@ libfdt_tests () {
     # Tests for behaviour on various sorts of corrupted trees
     run_test truncated_property
     run_test truncated_string
+    run_test truncated_memrsv
 
     # Check aliases support in fdt_path_offset
     run_dtc_test -I dts -O dtb -o aliases.dtb aliases.dts
diff --git a/tests/testdata.h b/tests/testdata.h
index b257011..68dcbac 100644
--- a/tests/testdata.h
+++ b/tests/testdata.h
@@ -48,4 +48,5 @@ extern struct fdt_header bad_node_format;
 extern struct fdt_header bad_prop_char;
 extern struct fdt_header ovf_size_strings;
 extern struct fdt_header truncated_string;
+extern struct fdt_header truncated_memrsv;
 #endif /* ! __ASSEMBLY */
diff --git a/tests/trees.S b/tests/trees.S
index 7ae1bde..efab287 100644
--- a/tests/trees.S
+++ b/tests/trees.S
@@ -259,3 +259,23 @@ truncated_string_bad_string:
 	/* NOTE: terminating \0 deliberately missing */
 truncated_string_strings_end:
 truncated_string_end:
+
+
+	/* truncated_memrsv */
+	TREE_HDR(truncated_memrsv)
+
+truncated_memrsv_struct:
+	BEGIN_NODE("")
+	END_NODE
+	FDTLONG(FDT_END)
+truncated_memrsv_struct_end:
+
+truncated_memrsv_strings:
+truncated_memrsv_strings_end:
+
+	.balign	8
+truncated_memrsv_rsvmap:
+	RSVMAP_ENTRY(TEST_ADDR_1H, TEST_ADDR_1L, TEST_SIZE_1H, TEST_SIZE_1L)
+truncated_memrsv_rsvmap_end:
+
+truncated_memrsv_end:
diff --git a/tests/truncated_memrsv.c b/tests/truncated_memrsv.c
new file mode 100644
index 0000000..2013e98
--- /dev/null
+++ b/tests/truncated_memrsv.c
@@ -0,0 +1,63 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ *	Testcase for misbehaviour on a truncated string
+ * Copyright (C) 2018 David Gibson, IBM Corporation.
+ *
+ * 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 <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+
+#include <libfdt.h>
+
+#include "tests.h"
+#include "testdata.h"
+
+int main(int argc, char *argv[])
+{
+	void *fdt = &truncated_memrsv;
+	int err;
+	uint64_t addr, size;
+
+	test_init(argc, argv);
+
+	err = fdt_check_header(fdt);
+	if (err != 0)
+		FAIL("Bad header: %s", fdt_strerror(err));
+
+	err = fdt_num_mem_rsv(fdt);
+	if (err != -FDT_ERR_TRUNCATED)
+		FAIL("fdt_num_mem_rsv() returned %d instead of -FDT_ERR_TRUNCATED",
+		     err);
+
+	err = fdt_get_mem_rsv(fdt, 0, &addr, &size);
+	if (err != 0)
+		FAIL("fdt_get_mem_rsv() failed on first entry: %s",
+		     fdt_strerror(err));
+	if ((addr != TEST_ADDR_1) || (size != TEST_SIZE_1))
+		FAIL("Entry doesn't match: (0x%llx, 0x%llx) != (0x%llx, 0x%llx)",
+		     (unsigned long long)addr, (unsigned long long)size,
+		     TEST_ADDR_1, TEST_SIZE_1);
+	       
+	err = fdt_get_mem_rsv(fdt, 1, &addr, &size);
+	if (err != -FDT_ERR_BADOFFSET)
+		FAIL("fdt_get_mem_rsv(1) returned %d instead of -FDT_ERR_BADOFFSET",
+		     err);
+	
+	PASS();
+}