From 37525d309a27cefb579e4e9130d7568b439795dc Mon Sep 17 00:00:00 2001 From: kettenis Date: Mon, 14 Mar 2022 19:09:32 +0000 Subject: [PATCH] The current FDT code we use in the bootloader is buggy and will write into memory beyond the actual FDT data structure when adding information to the device tree. This is especially problematic on ACPI systems where we add lots of information to the device tree based on ACPI tables. Fix the FDT code to never write beyond the end of the data structure and panic if we run out of free space. Raise the amount of free space frm 4K to 16K for the proto-FDT we use on ACPI systems. Bump the version number of the arm64 bootloader. ok visa@, patrick@ --- sys/arch/arm64/stand/efiboot/Makefile | 4 +-- sys/arch/arm64/stand/efiboot/conf.c | 4 +-- sys/arch/arm64/stand/efiboot/dt_blob.S | 2 +- sys/arch/arm64/stand/efiboot/fdt.c | 47 ++++++++++++-------------- sys/arch/armv7/stand/efiboot/fdt.c | 47 ++++++++++++-------------- sys/arch/riscv64/stand/efiboot/fdt.c | 47 ++++++++++++-------------- 6 files changed, 71 insertions(+), 80 deletions(-) diff --git a/sys/arch/arm64/stand/efiboot/Makefile b/sys/arch/arm64/stand/efiboot/Makefile index 1c77528a846..5ed83a20382 100644 --- a/sys/arch/arm64/stand/efiboot/Makefile +++ b/sys/arch/arm64/stand/efiboot/Makefile @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile,v 1.14 2021/12/14 11:05:37 kettenis Exp $ +# $OpenBSD: Makefile,v 1.15 2022/03/14 19:09:32 kettenis Exp $ NOMAN= # @@ -85,6 +85,6 @@ NOPROG=yes .endif DTC= dtc -DTCOPTS= -p 4096 -H epapr +DTCOPTS= -p 16384 -H epapr update-blob: ${DTC} ${DTCOPTS} -O asm -o ${.CURDIR}/dt_blob.S ${.CURDIR}/acpi.dts diff --git a/sys/arch/arm64/stand/efiboot/conf.c b/sys/arch/arm64/stand/efiboot/conf.c index 18f860f42e8..201bac31a8d 100644 --- a/sys/arch/arm64/stand/efiboot/conf.c +++ b/sys/arch/arm64/stand/efiboot/conf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: conf.c,v 1.35 2021/12/14 11:05:37 kettenis Exp $ */ +/* $OpenBSD: conf.c,v 1.36 2022/03/14 19:09:32 kettenis Exp $ */ /* * Copyright (c) 1996 Michael Shalayeff @@ -46,7 +46,7 @@ #include "efipxe.h" #include "softraid_arm64.h" -const char version[] = "1.7"; +const char version[] = "1.8"; int debug = 0; struct fs_ops file_system[] = { diff --git a/sys/arch/arm64/stand/efiboot/dt_blob.S b/sys/arch/arm64/stand/efiboot/dt_blob.S index 9e42117e46c..cbdac794404 100644 --- a/sys/arch/arm64/stand/efiboot/dt_blob.S +++ b/sys/arch/arm64/stand/efiboot/dt_blob.S @@ -375,7 +375,7 @@ _dt_strings_end: .globl dt_blob_end dt_blob_end: _dt_blob_end: - .space 4096, 0 + .space 16384, 0 .globl dt_blob_abs_end dt_blob_abs_end: _dt_blob_abs_end: diff --git a/sys/arch/arm64/stand/efiboot/fdt.c b/sys/arch/arm64/stand/efiboot/fdt.c index 48b4ba8f178..e2c3a8a2393 100644 --- a/sys/arch/arm64/stand/efiboot/fdt.c +++ b/sys/arch/arm64/stand/efiboot/fdt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: fdt.c,v 1.5 2021/03/11 11:16:56 jsg Exp $ */ +/* $OpenBSD: fdt.c,v 1.6 2022/03/14 19:09:32 kettenis Exp $ */ /* * Copyright (c) 2009 Dariusz Swiderski @@ -141,13 +141,10 @@ fdt_add_str(char *name) size_t len = roundup(strlen(name) + 1, sizeof(uint32_t)); char *end = tree.strings + tree.strings_size; - memmove(end + len, end, tree.end - end); + if (end + len > tree.end) + panic("FDT overflow"); + tree.strings_size += len; - if (tree.tree > tree.strings) - tree.tree += len; - if (tree.memory > tree.strings) - tree.memory += len; - tree.end += len; memset(end, 0, len); memcpy(end, name, strlen(name)); @@ -222,6 +219,7 @@ fdt_node_property(void *node, char *name, char **out) int fdt_node_set_property(void *node, char *name, void *data, int len) { + char *end = tree.strings + tree.strings_size; uint32_t *ptr, *next; uint32_t nameid; uint32_t curlen; @@ -246,14 +244,13 @@ fdt_node_set_property(void *node, char *name, void *data, int len) curlen = betoh32(*(ptr + 1)); delta = roundup(len, sizeof(uint32_t)) - roundup(curlen, sizeof(uint32_t)); + if (end + delta > tree.end) + panic("FDT overflow"); + memmove((char *)next + delta, next, - tree.end - (char *)next); + end - (char *)next); tree.struct_size += delta; - if (tree.strings > tree.tree) - tree.strings += delta; - if (tree.memory > tree.tree) - tree.memory += delta; - tree.end += delta; + tree.strings += delta; *(ptr + 1) = htobe32(len); memcpy(ptr + 3, data, len); return 1; @@ -266,6 +263,7 @@ fdt_node_set_property(void *node, char *name, void *data, int len) int fdt_node_add_property(void *node, char *name, void *data, int len) { + char *end = tree.strings + tree.strings_size; char *dummy; if (!tree_inited) @@ -277,15 +275,14 @@ fdt_node_add_property(void *node, char *name, void *data, int len) if (betoh32(*ptr) != FDT_NODE_BEGIN) return 0; + if (end + 3 * sizeof(uint32_t) > tree.end) + panic("FDT overflow"); + ptr = skip_node_name(ptr + 1); - memmove(ptr + 3, ptr, tree.end - (char *)ptr); + memmove(ptr + 3, ptr, end - (char *)ptr); tree.struct_size += 3 * sizeof(uint32_t); - if (tree.strings > tree.tree) - tree.strings += 3 * sizeof(uint32_t); - if (tree.memory > tree.tree) - tree.memory += 3 * sizeof(uint32_t); - tree.end += 3 * sizeof(uint32_t); + tree.strings += 3 * sizeof(uint32_t); *ptr++ = htobe32(FDT_PROPERTY); *ptr++ = htobe32(0); *ptr++ = htobe32(fdt_add_str(name)); @@ -298,6 +295,7 @@ int fdt_node_add_node(void *node, char *name, void **child) { size_t len = roundup(strlen(name) + 1, sizeof(uint32_t)) + 8; + char *end = tree.strings + tree.strings_size; uint32_t *ptr = (uint32_t *)node; if (!tree_inited) @@ -306,6 +304,9 @@ fdt_node_add_node(void *node, char *name, void **child) if (betoh32(*ptr) != FDT_NODE_BEGIN) return 0; + if (end + len > tree.end) + panic("FDT overflow"); + ptr = skip_node_name(ptr + 1); ptr = skip_props(ptr); @@ -313,13 +314,9 @@ fdt_node_add_node(void *node, char *name, void **child) while (betoh32(*ptr) == FDT_NODE_BEGIN) ptr = skip_node(ptr); - memmove((char *)ptr + len, ptr, tree.end - (char *)ptr); + memmove((char *)ptr + len, ptr, end - (char *)ptr); tree.struct_size += len; - if (tree.strings > tree.tree) - tree.strings += len; - if (tree.memory > tree.tree) - tree.memory += len; - tree.end += len; + tree.strings += len; *child = ptr; *ptr++ = htobe32(FDT_NODE_BEGIN); diff --git a/sys/arch/armv7/stand/efiboot/fdt.c b/sys/arch/armv7/stand/efiboot/fdt.c index 53bc8eca772..3556a5a6b23 100644 --- a/sys/arch/armv7/stand/efiboot/fdt.c +++ b/sys/arch/armv7/stand/efiboot/fdt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: fdt.c,v 1.7 2021/03/11 11:16:56 jsg Exp $ */ +/* $OpenBSD: fdt.c,v 1.8 2022/03/14 19:09:32 kettenis Exp $ */ /* * Copyright (c) 2009 Dariusz Swiderski @@ -141,13 +141,10 @@ fdt_add_str(char *name) size_t len = roundup(strlen(name) + 1, sizeof(uint32_t)); char *end = tree.strings + tree.strings_size; - memmove(end + len, end, tree.end - end); + if (end + len > tree.end) + panic("FDT overflow"); + tree.strings_size += len; - if (tree.tree > tree.strings) - tree.tree += len; - if (tree.memory > tree.strings) - tree.memory += len; - tree.end += len; memset(end, 0, len); memcpy(end, name, strlen(name)); @@ -222,6 +219,7 @@ fdt_node_property(void *node, char *name, char **out) int fdt_node_set_property(void *node, char *name, void *data, int len) { + char *end = tree.strings + tree.strings_size; uint32_t *ptr, *next; uint32_t nameid; uint32_t curlen; @@ -246,14 +244,13 @@ fdt_node_set_property(void *node, char *name, void *data, int len) curlen = betoh32(*(ptr + 1)); delta = roundup(len, sizeof(uint32_t)) - roundup(curlen, sizeof(uint32_t)); + if (end + delta > tree.end) + panic("FDT overflow"); + memmove((char *)next + delta, next, - tree.end - (char *)next); + end - (char *)next); tree.struct_size += delta; - if (tree.strings > tree.tree) - tree.strings += delta; - if (tree.memory > tree.tree) - tree.memory += delta; - tree.end += delta; + tree.strings += delta; *(ptr + 1) = htobe32(len); memcpy(ptr + 3, data, len); return 1; @@ -266,6 +263,7 @@ fdt_node_set_property(void *node, char *name, void *data, int len) int fdt_node_add_property(void *node, char *name, void *data, int len) { + char *end = tree.strings + tree.strings_size; char *dummy; if (!tree_inited) @@ -277,15 +275,14 @@ fdt_node_add_property(void *node, char *name, void *data, int len) if (betoh32(*ptr) != FDT_NODE_BEGIN) return 0; + if (end + 3 * sizeof(uint32_t) > tree.end) + panic("FDT overflow"); + ptr = skip_node_name(ptr + 1); - memmove(ptr + 3, ptr, tree.end - (char *)ptr); + memmove(ptr + 3, ptr, end - (char *)ptr); tree.struct_size += 3 * sizeof(uint32_t); - if (tree.strings > tree.tree) - tree.strings += 3 * sizeof(uint32_t); - if (tree.memory > tree.tree) - tree.memory += 3 * sizeof(uint32_t); - tree.end += 3 * sizeof(uint32_t); + tree.strings += 3 * sizeof(uint32_t); *ptr++ = htobe32(FDT_PROPERTY); *ptr++ = htobe32(0); *ptr++ = htobe32(fdt_add_str(name)); @@ -298,6 +295,7 @@ int fdt_node_add_node(void *node, char *name, void **child) { size_t len = roundup(strlen(name) + 1, sizeof(uint32_t)) + 8; + char *end = tree.strings + tree.strings_size; uint32_t *ptr = (uint32_t *)node; if (!tree_inited) @@ -306,6 +304,9 @@ fdt_node_add_node(void *node, char *name, void **child) if (betoh32(*ptr) != FDT_NODE_BEGIN) return 0; + if (end + len > tree.end) + panic("FDT overflow"); + ptr = skip_node_name(ptr + 1); ptr = skip_props(ptr); @@ -313,13 +314,9 @@ fdt_node_add_node(void *node, char *name, void **child) while (betoh32(*ptr) == FDT_NODE_BEGIN) ptr = skip_node(ptr); - memmove((char *)ptr + len, ptr, tree.end - (char *)ptr); + memmove((char *)ptr + len, ptr, end - (char *)ptr); tree.struct_size += len; - if (tree.strings > tree.tree) - tree.strings += len; - if (tree.memory > tree.tree) - tree.memory += len; - tree.end += len; + tree.strings += len; *child = ptr; *ptr++ = htobe32(FDT_NODE_BEGIN); diff --git a/sys/arch/riscv64/stand/efiboot/fdt.c b/sys/arch/riscv64/stand/efiboot/fdt.c index 0985fa3bea6..45a280f9465 100644 --- a/sys/arch/riscv64/stand/efiboot/fdt.c +++ b/sys/arch/riscv64/stand/efiboot/fdt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: fdt.c,v 1.1 2021/04/28 19:01:00 drahn Exp $ */ +/* $OpenBSD: fdt.c,v 1.2 2022/03/14 19:09:32 kettenis Exp $ */ /* * Copyright (c) 2009 Dariusz Swiderski @@ -141,13 +141,10 @@ fdt_add_str(char *name) size_t len = roundup(strlen(name) + 1, sizeof(uint32_t)); char *end = tree.strings + tree.strings_size; - memmove(end + len, end, tree.end - end); + if (end + len > tree.end) + panic("FDT overflow"); + tree.strings_size += len; - if (tree.tree > tree.strings) - tree.tree += len; - if (tree.memory > tree.strings) - tree.memory += len; - tree.end += len; memset(end, 0, len); memcpy(end, name, strlen(name)); @@ -222,6 +219,7 @@ fdt_node_property(void *node, char *name, char **out) int fdt_node_set_property(void *node, char *name, void *data, int len) { + char *end = tree.strings + tree.strings_size; uint32_t *ptr, *next; uint32_t nameid; uint32_t curlen; @@ -246,14 +244,13 @@ fdt_node_set_property(void *node, char *name, void *data, int len) curlen = betoh32(*(ptr + 1)); delta = roundup(len, sizeof(uint32_t)) - roundup(curlen, sizeof(uint32_t)); + if (end + delta > tree.end) + panic("FDT overflow"); + memmove((char *)next + delta, next, - tree.end - (char *)next); + end - (char *)next); tree.struct_size += delta; - if (tree.strings > tree.tree) - tree.strings += delta; - if (tree.memory > tree.tree) - tree.memory += delta; - tree.end += delta; + tree.strings += delta; *(ptr + 1) = htobe32(len); memcpy(ptr + 3, data, len); return 1; @@ -266,6 +263,7 @@ fdt_node_set_property(void *node, char *name, void *data, int len) int fdt_node_add_property(void *node, char *name, void *data, int len) { + char *end = tree.strings + tree.strings_size; char *dummy; if (!tree_inited) @@ -277,15 +275,14 @@ fdt_node_add_property(void *node, char *name, void *data, int len) if (betoh32(*ptr) != FDT_NODE_BEGIN) return 0; + if (end + 3 * sizeof(uint32_t) > tree.end) + panic("FDT overflow"); + ptr = skip_node_name(ptr + 1); - memmove(ptr + 3, ptr, tree.end - (char *)ptr); + memmove(ptr + 3, ptr, end - (char *)ptr); tree.struct_size += 3 * sizeof(uint32_t); - if (tree.strings > tree.tree) - tree.strings += 3 * sizeof(uint32_t); - if (tree.memory > tree.tree) - tree.memory += 3 * sizeof(uint32_t); - tree.end += 3 * sizeof(uint32_t); + tree.strings += 3 * sizeof(uint32_t); *ptr++ = htobe32(FDT_PROPERTY); *ptr++ = htobe32(0); *ptr++ = htobe32(fdt_add_str(name)); @@ -298,6 +295,7 @@ int fdt_node_add_node(void *node, char *name, void **child) { size_t len = roundup(strlen(name) + 1, sizeof(uint32_t)) + 8; + char *end = tree.strings + tree.strings_size; uint32_t *ptr = (uint32_t *)node; if (!tree_inited) @@ -306,6 +304,9 @@ fdt_node_add_node(void *node, char *name, void **child) if (betoh32(*ptr) != FDT_NODE_BEGIN) return 0; + if (end + len > tree.end) + panic("FDT overflow"); + ptr = skip_node_name(ptr + 1); ptr = skip_props(ptr); @@ -313,13 +314,9 @@ fdt_node_add_node(void *node, char *name, void **child) while (betoh32(*ptr) == FDT_NODE_BEGIN) ptr = skip_node(ptr); - memmove((char *)ptr + len, ptr, tree.end - (char *)ptr); + memmove((char *)ptr + len, ptr, end - (char *)ptr); tree.struct_size += len; - if (tree.strings > tree.tree) - tree.strings += len; - if (tree.memory > tree.tree) - tree.memory += len; - tree.end += len; + tree.strings += len; *child = ptr; *ptr++ = htobe32(FDT_NODE_BEGIN); -- 2.20.1