From 6d9d15c397f37a8ca93abb1e7522274bb7f949a2 Mon Sep 17 00:00:00 2001 From: Axel Heider Date: Sun, 26 Sep 2021 23:55:25 +0200 Subject: [PATCH] boot/arm: pass DTB as paddr/len and check location - Pass on the DTB as paddr/len. - Fail boot if DTB region is invalid. Logging error messages requires reordering the code. - Do not copy the whole DTB region, but only the actual DTB data to bootinfo. - Align the ARM and RISC-V implementations. Signed-off-by: Axel Heider --- src/arch/arm/kernel/boot.c | 85 +++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 37 deletions(-) diff --git a/src/arch/arm/kernel/boot.c b/src/arch/arm/kernel/boot.c index 039c0b3dc..4efc43054 100644 --- a/src/arch/arm/kernel/boot.c +++ b/src/arch/arm/kernel/boot.c @@ -316,8 +316,8 @@ static BOOT_CODE bool_t try_init_kernel( paddr_t ui_p_reg_end, sword_t pv_offset, vptr_t v_entry, - paddr_t dtb_addr_start, - paddr_t dtb_addr_end + paddr_t dtb_phys_addr, + word_t dtb_size ) { cap_t root_cnode_cap; @@ -328,8 +328,7 @@ static BOOT_CODE bool_t try_init_kernel( ui_p_reg_start, ui_p_reg_end }; region_t ui_reg = paddr_to_pptr_reg(ui_p_reg); - region_t dtb_reg; - word_t extra_bi_size; + word_t extra_bi_size = 0; pptr_t extra_bi_offset = 0; vptr_t extra_bi_frame_vptr; vptr_t bi_frame_vptr; @@ -347,27 +346,6 @@ static BOOT_CODE bool_t try_init_kernel( bi_frame_vptr = ipcbuf_vptr + BIT(PAGE_BITS); extra_bi_frame_vptr = bi_frame_vptr + BIT(PAGE_BITS); - /* If no DTB was provided, skip allocating extra bootinfo */ - p_region_t dtb_p_reg = { - dtb_addr_start, ROUND_UP(dtb_addr_end, PAGE_BITS) - }; - if (dtb_addr_start == 0) { - extra_bi_size = 0; - dtb_reg = (region_t) { - 0, 0 - }; - } else { - dtb_reg = paddr_to_pptr_reg(dtb_p_reg); - extra_bi_size = sizeof(seL4_BootInfoHeader) + (dtb_reg.end - dtb_reg.start); - } - word_t extra_bi_size_bits = calculate_extra_bi_size_bits(extra_bi_size); - - /* The region of the initial thread is the user image + ipcbuf and boot info */ - v_region_t it_v_reg = { - .start = ui_v_reg.start, - .end = extra_bi_frame_vptr + BIT(extra_bi_size_bits) - }; - /* setup virtual memory for the kernel */ map_kernel_window(); @@ -383,6 +361,43 @@ static BOOT_CODE bool_t try_init_kernel( /* initialise the platform */ init_plat(); + /* If a DTB was provided, pass the data on as extra bootinfo */ + p_region_t dtb_p_reg = P_REG_EMPTY; + if (dtb_size > 0) { + paddr_t dtb_phys_end = ROUND_UP(dtb_phys_addr + dtb_size, PAGE_BITS); + if (dtb_phys_end < dtb_phys_addr) { + /* An integer overflow happened in DTB end address calculation, the + * location or size passed seems invalid. + */ + printf("ERROR: DTB location at %"SEL4_PRIx_word + " len %"SEL4_PRIu_word" invalid\n", + dtb_phys_addr, dtb_size); + return false; + } + /* If the DTB is located in physical memory that is not mapped in the + * kernel window we cannot access it. + */ + if (dtb_phys_end >= PADDR_TOP) { + printf("ERROR: DTB at [%"SEL4_PRIx_word"..%"SEL4_PRIx_word"] " + "exceeds PADDR_TOP (%"SEL4_PRIx_word")\n", + dtb_phys_addr, dtb_phys_end, PADDR_TOP); + return false; + } + /* DTB seems valid and accessible, pass it on in bootinfo. */ + extra_bi_size += sizeof(seL4_BootInfoHeader) + dtb_size; + /* Remember the page aligned memory region it uses. */ + dtb_p_reg = (p_region_t) { + .start = dtb_phys_addr, + .end = dtb_phys_end + }; + } + + /* The region of the initial thread is the user image + ipcbuf and boot info */ + word_t extra_bi_size_bits = calculate_extra_bi_size_bits(extra_bi_size); + v_region_t it_v_reg = { + .start = ui_v_reg.start, + .end = extra_bi_frame_vptr + BIT(extra_bi_size_bits) + }; if (it_v_reg.end >= USER_TOP) { /* Variable arguments for printf() require well defined integer types to * work properly. Unfortunately, the definition of USER_TOP differs @@ -420,14 +435,15 @@ static BOOT_CODE bool_t try_init_kernel( /* put DTB in the bootinfo block, if present. */ seL4_BootInfoHeader header; - if (dtb_reg.start) { + if (dtb_size > 0) { header.id = SEL4_BOOTINFO_HEADER_FDT; - header.len = sizeof(header) + dtb_reg.end - dtb_reg.start; + header.len = sizeof(header) + dtb_size; *(seL4_BootInfoHeader *)(rootserver.extra_bi + extra_bi_offset) = header; extra_bi_offset += sizeof(header); - memcpy((void *)(rootserver.extra_bi + extra_bi_offset), (void *)dtb_reg.start, - dtb_reg.end - dtb_reg.start); - extra_bi_offset += (dtb_reg.end - dtb_reg.start); + memcpy((void *)(rootserver.extra_bi + extra_bi_offset), + paddr_to_pptr(dtb_phys_addr), + dtb_size); + extra_bi_offset += dtb_size; } if (extra_bi_size > extra_bi_offset) { @@ -609,11 +625,6 @@ BOOT_CODE VISIBLE void init_kernel( ) { bool_t result; - paddr_t dtb_end_p = 0; - - if (dtb_addr_p) { - dtb_end_p = dtb_addr_p + dtb_size; - } #ifdef ENABLE_SMP_SUPPORT /* we assume there exists a cpu with id 0 and will use it for bootstrapping */ @@ -622,7 +633,7 @@ BOOT_CODE VISIBLE void init_kernel( ui_p_reg_end, pv_offset, v_entry, - dtb_addr_p, dtb_end_p); + dtb_addr_p, dtb_size); } else { result = try_init_kernel_secondary_core(); } @@ -632,7 +643,7 @@ BOOT_CODE VISIBLE void init_kernel( ui_p_reg_end, pv_offset, v_entry, - dtb_addr_p, dtb_end_p); + dtb_addr_p, dtb_size); #endif /* ENABLE_SMP_SUPPORT */