share the DTS size bounds in fdt.h and state the window assumption

The WOLFBOOT_DTS_MAX_SIZE/WOLFBOOT_DTS_MIN_SIZE pair was defined
separately in update_disk.c (F-7066) and update_ram.c (pre-existing), so
the two copies could drift. Move it to include/fdt.h, the FDT dialect
header both translation units already pull in via image.h; the hal
override (nxp_ppc.h, included before fdt.h in boot_ppc.c) keeps its
precedence. Also replace the 'bounded by the staging region' comment,
which claimed more than the code guarantees: the copy is clamped to
WOLFBOOT_DTS_MAX_SIZE, so the staging window at
WOLFBOOT_LOAD_DTS_ADDRESS must be at least that large (or the bound
overridden for the target), and the header comment now says so.

Skoll review finding 3, 2026-08-21 wolfboot review.
pull/868/head
Daniele Lacamera 2026-08-21 09:08:26 +02:00
parent 1dcd481a9f
commit 9b1a48554a
3 changed files with 20 additions and 26 deletions

View File

@ -73,6 +73,18 @@ struct fdt_property {
#define FDT_ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1))
#define FDT_TAGALIGN(x) (FDT_ALIGN((x), FDT_TAGSIZE))
/* Bounds for the attacker-influenced fdt_totalsize before relocating or
* forwarding a DTB. MIN is the FDT v17 header size (also enforced by the
* signer): fdt_check_header validates magic/version but not totalsize, so a
* crafted header with a tiny totalsize must be rejected rather than
* loaded/forwarded as a partial tree. The MAX default assumes a staging
* window at WOLFBOOT_LOAD_DTS_ADDRESS of at least 1 MiB; targets with a
* smaller window must override WOLFBOOT_DTS_MAX_SIZE (see hal/nxp_ppc.h). */
#ifndef WOLFBOOT_DTS_MAX_SIZE
#define WOLFBOOT_DTS_MAX_SIZE (1024U * 1024U)
#endif
#define WOLFBOOT_DTS_MIN_SIZE (40U)
#define FDT_FIRST_SUPPORTED_VERSION 0x10
#define FDT_LAST_SUPPORTED_VERSION 0x11

View File

@ -249,17 +249,6 @@ static void disk_decrypted_header_clear(uint8_t *hdr)
extern int wolfBoot_get_dts_size(void *dts_addr);
#if defined(MMU) || defined(WOLFBOOT_FDT)
/* Bounds for the attacker-influenced fdt_totalsize before relocating a DTB.
* MIN is the FDT v17 header size (also enforced by the signer): fdt_check_header
* validates magic/version but not totalsize, so a crafted header with a tiny
* totalsize must be rejected rather than loaded/forwarded as a partial tree. */
#ifndef WOLFBOOT_DTS_MAX_SIZE
#define WOLFBOOT_DTS_MAX_SIZE (1024U * 1024U)
#endif
#define WOLFBOOT_DTS_MIN_SIZE (40U)
#endif
#if defined(WOLFBOOT_NO_LOAD_ADDRESS) || !defined(WOLFBOOT_LOAD_ADDRESS)
/* from the linker, where wolfBoot ends */
extern uint8_t _end_wb[];
@ -650,8 +639,10 @@ void RAMFUNCTION wolfBoot_start(void)
parsed >= (int)WOLFBOOT_DTS_MIN_SIZE &&
(uint32_t)parsed <= WOLFBOOT_DTS_MAX_SIZE) {
/* Relocate to the load DTS address. The copy length is
* the parsed DTB size (bounded by the staging region),
* not the FIT-declared property length. */
* the parsed DTB size, clamped to WOLFBOOT_DTS_MAX_SIZE,
* not the FIT-declared property length. The staging window
* at WOLFBOOT_LOAD_DTS_ADDRESS must be at least that large
* (or the bound must be overridden for the target). */
dts_addr = (uint8_t*)WOLFBOOT_LOAD_DTS_ADDRESS;
dts_size = (uint32_t)parsed;
wolfBoot_printf("Loading DTS: %p -> %p (%d bytes)\n",

View File

@ -51,17 +51,6 @@ extern void hal_flash_dualbank_swap(void);
extern uint32_t kernel_load_addr;
extern uint32_t dts_load_addr;
#if defined(MMU) || defined(WOLFBOOT_FDT)
/* Bounds for the attacker-influenced fdt_totalsize before relocating a DTB.
* MIN is the FDT v17 header size (also enforced by the signer): fdt_check_header
* validates magic/version but not totalsize, so a crafted header with a tiny
* totalsize must be rejected rather than loaded/forwarded as a partial tree. */
#ifndef WOLFBOOT_DTS_MAX_SIZE
#define WOLFBOOT_DTS_MAX_SIZE (1024U * 1024U)
#endif
#define WOLFBOOT_DTS_MIN_SIZE (40U)
#endif
#if defined(__WOLFBOOT) && defined(WOLFBOOT_LOAD_ADDRESS)
extern uint8_t _end[]; /* linker symbol: end of wolfBoot BSS */
#endif
@ -662,8 +651,10 @@ backup_on_failure:
parsed >= (int)WOLFBOOT_DTS_MIN_SIZE &&
(uint32_t)parsed <= WOLFBOOT_DTS_MAX_SIZE) {
/* Relocate to the load DTS address. The copy length is
* the parsed DTB size (bounded by the staging region),
* not the FIT-declared property length. */
* the parsed DTB size, clamped to WOLFBOOT_DTS_MAX_SIZE,
* not the FIT-declared property length. The staging window
* at WOLFBOOT_LOAD_DTS_ADDRESS must be at least that large
* (or the bound must be overridden for the target). */
dts_addr = (uint8_t*)WOLFBOOT_LOAD_DTS_ADDRESS;
dts_size = (uint32_t)parsed;
wolfBoot_printf("Loading DTS: %p -> %p (%d bytes)\n",