[coreboot-gerrit] Change in coreboot[master]: devicetree: Check destination size while flattening

[Patrick Rudolph (Code Review)]

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41201 )

Change subject: devicetree: Check destination size while flattening
......................................................................

devicetree: Check destination size while flattening

* Check that destination buffer doesn't overflow while flattening
* Add 128 bytes to the requested space for the FDT in fit_payload.c

Change-Id: Ia8300df1abd012d286c10d12cc99bdc05c2710d8
Signed-off-by: Patrick Rudolph 
---
M src/include/device_tree.h
M src/lib/device_tree.c
M src/lib/fit_payload.c
M src/mainboard/sifive/hifive-unleashed/fixup_fdt.c
4 files changed, 25 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/41201/1

diff --git a/src/include/device_tree.h b/src/include/device_tree.h
index bd0d151..ddfb6e5 100644
--- a/src/include/device_tree.h
+++ b/src/include/device_tree.h
@@ -116,8 +116,8 @@

/* Figure out how big a device tree would be if it were flattened. */
 uint32_t dt_flat_size(const struct device_tree *tree);
-/* Flatten a device tree into the buffer pointed to by dest. */
-void dt_flatten(const struct device_tree *tree, void *dest);
+/* Flatten a device tree into the buffer with size len pointed to by dest. */
+void dt_flatten(const struct device_tree *tree, void *dest, size_t len);
 void dt_print_node(const struct device_tree_node *node);
 /* Read #address-cells and #size-cells properties from a node. */
 void dt_read_cell_props(const struct device_tree_node *node, u32 *addrcp,
diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c
index cb81d32..5d4b2b0 100644
--- a/src/lib/device_tree.c
+++ b/src/lib/device_tree.c
@@ -421,14 +421,21 @@
    *strings_start = dstrings;
 }

-void dt_flatten(const struct device_tree *tree, void *start_dest)
+void dt_flatten(const struct device_tree *tree, void *start_dest, size_t len)
 {
    uint8_t *dest = (uint8_t *)start_dest;
+	uint8_t *dest_max = ((uint8_t *)start_dest) + len;
+
+	if ((dest + tree->header_size) > dest_max)
+		goto err;

memcpy(dest, tree->header, tree->header_size);
    struct fdt_header *header = (struct fdt_header *)dest;
    dest += tree->header_size;

+	if ((dest + sizeof(uint64_t) * 2) > dest_max)
+		goto err;
+
    struct device_tree_reserve_map_entry *entry;
    list_for_each(entry, tree->reserve_map, list_node)
    	dt_flatten_map_entry(entry, (void **)&dest);
@@ -439,6 +446,9 @@
    uint32_t strings_size = 0;
    dt_flat_node_size(tree->root, &struct_size, &strings_size);

+	if ((dest + struct_size) > dest_max)
+		goto err;
+
    uint8_t *struct_start = dest;
    header->structure_offset = htobe32(dest - (uint8_t *)start_dest);
    header->structure_size = htobe32(struct_size);
@@ -447,6 +457,9 @@
    *((uint32_t *)dest) = htobe32(FDT_TOKEN_END);
    dest += sizeof(uint32_t);

+	if ((dest + strings_size) > dest_max)
+		goto err;
+
    uint8_t *strings_start = dest;
    header->strings_offset = htobe32(dest - (uint8_t *)start_dest);
    header->strings_size = htobe32(strings_size);
@@ -456,6 +469,11 @@
    		(void **)&strings_start);

header->totalsize = htobe32(dest - (uint8_t *)start_dest);
+
+	return;
+err:
+	printk(BIOS_CRIT, "%s: Buffer overflowed (used %d, max %zd) while flattening devicetree\n",
+	       __func__, (dest - (uint8_t *)start_dest), len);
 }

diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c
index 9cf1542..1b97ad9 100644
--- a/src/lib/fit_payload.c
+++ b/src/lib/fit_payload.c
@@ -23,7 +23,7 @@
    printk(BIOS_INFO, "FIT: Flattening FDT to %p\n",
           (void *)fdt->offset);

-	dt_flatten(dt, (void *)fdt->offset);
+	dt_flatten(dt, (void *)fdt->offset, fdt->size);
    prog_segment_loaded(fdt->offset, fdt->size, 0);
 }

@@ -219,7 +219,8 @@

/* Collect infos for fit_payload_arch */
    kernel.size = config->kernel->size;
-	fdt.size = dt_flat_size(dt);
+	/* Add additional space for fit_add_ramdisk() */
+	fdt.size = dt_flat_size(dt) + 128;
    initrd.size = config->ramdisk ? config->ramdisk->size : 0;

/* Invoke arch specific payload placement and fixups */
diff --git a/src/mainboard/sifive/hifive-unleashed/fixup_fdt.c b/src/mainboard/sifive/hifive-unleashed/fixup_fdt.c
index c270857..3cffb97 100644
--- a/src/mainboard/sifive/hifive-unleashed/fixup_fdt.c
+++ b/src/mainboard/sifive/hifive-unleashed/fixup_fdt.c
@@ -96,7 +96,7 @@
    	return;
    }

-	dt_flatten(tree, dt);
+	dt_flatten(tree, dt, dt_flat_size(tree));

/* update HLS */
    for (int i = 0; i < CONFIG_MAX_CPUS; i++)