lmb_add_region handles overlapping regions wrong: instead of merging or rejecting to add a new reserved region that overlaps an existing one, it just adds the new region.

Since internally the same function is used for lmb_alloc, change lmb_add_region to reject overlapping regions.

Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---

Changes in v4: None Changes in v2: None

lib/lmb.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/lib/lmb.c b/lib/lmb.c index 1705417348..8dc703d996 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -136,6 +136,9 @@ static long lmb_add_region(struct lmb_region *rgn, phys_addr_t base, phys_size_t rgn->region[i].size += size; coalesced++; break; + } else if (lmb_addrs_overlap(base, size, rgnbase, rgnsize)) { + /* regions overlap */ + return -1; } }

This adds two new functions, lmb_alloc_addr and lmb_get_unreserved_size.

lmb_alloc_addr behaves like lmb_alloc, but it tries to allocate a pre-specified address range. Unlike lmb_reserve, this address range must be inside one of the memory ranges that has been set up with lmb_add.

lmb_get_unreserved_size returns the number of bytes that can be used up to the next reserved region or the end of valid ram. This can be 0 if the address passed is reserved.

Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---

Changes in v4: None Changes in v2: - added lmb_get_unreserved_size() for tftp

include/lmb.h | 3 +++ lib/lmb.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+)

diff --git a/include/lmb.h b/include/lmb.h index f04d058093..7d7e2a78dc 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -38,6 +38,9 @@ extern phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align phys_addr_t max_addr); extern phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align, phys_addr_t max_addr); +extern phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, + phys_size_t size); +extern phys_size_t lmb_get_unreserved_size(struct lmb *lmb, phys_addr_t addr); extern int lmb_is_reserved(struct lmb *lmb, phys_addr_t addr); extern long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size);

diff --git a/lib/lmb.c b/lib/lmb.c index 8dc703d996..9de1581972 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -324,6 +324,59 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align, phy return 0; }

+/* + * Try to allocate a specific address range: must be in defined memory but not + * reserved + */ +phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size) +{ + long j; + + /* Check if the requested address is in one of the memory regions */ + j = lmb_overlaps_region(&lmb->memory, base, size); + if (j >= 0) { + /* + * Check if the requested end address is in the same memory + * region we found. + */ + if (lmb_addrs_overlap(lmb->memory.region[j].base, + lmb->memory.region[j].size, base + size - + 1, 1)) { + /* ok, reserve the memory */ + if (!lmb_reserve(lmb, base, size)) + return base; + } + } + return 0; +} + +/* Return number of bytes from a given address that are free */ +phys_size_t lmb_get_unreserved_size(struct lmb *lmb, phys_addr_t addr) +{ + int i; + long j; + + /* check if the requested address is in the memory regions */ + j = lmb_overlaps_region(&lmb->memory, addr, 1); + if (j >= 0) { + for (i = 0; i < lmb->reserved.cnt; i++) { + if (addr < lmb->reserved.region[i].base) { + /* first reserved range > requested address */ + return lmb->reserved.region[i].base - addr; + } + if (lmb->reserved.region[i].base + + lmb->reserved.region[i].size > addr) { + /* requested addr is in this reserved range */ + return 0; + } + } + /* if we come here: no reserved ranges above requested addr */ + return lmb->memory.region[lmb->memory.cnt - 1].base + + lmb->memory.region[lmb->memory.cnt - 1].size - addr; + } + return 0; +} + int lmb_is_reserved(struct lmb *lmb, phys_addr_t addr) { int i;

This reduces duplicate code only.

Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---

Changes in v4: None Changes in v2: None

common/bootm.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/common/bootm.c b/common/bootm.c index 8bf84ebcb7..31e4f0f794 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -56,15 +56,11 @@ static void boot_start_lmb(bootm_headers_t *images) ulong mem_start; phys_size_t mem_size;

- lmb_init(&images->lmb); - mem_start = env_get_bootm_low(); mem_size = env_get_bootm_size();

- lmb_add(&images->lmb, (phys_addr_t)mem_start, mem_size); - - arch_lmb_reserve(&images->lmb); - board_lmb_reserve(&images->lmb); + lmb_init_and_reserve(&images->lmb, (phys_addr_t)mem_start, mem_size, + NULL); } #else #define lmb_reserve(lmb, base, size)

This fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by using lmb to check for a valid range to store received blocks.

Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---

Changes in v4: - this was patch 8, is now patch 7 - lines changed because v3 patch 7 got removed and MCAST_TFTP still exists

Changes in v2: - this patch is new in v2

net/tftp.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 9 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c index 68ffd81414..d31364166e 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -17,6 +17,8 @@ #include <flash.h> #endif

+DECLARE_GLOBAL_DATA_PTR; + /* Well known TFTP port # */ #define WELL_KNOWN_PORT 69 /* Millisecs to timeout for lost pkt */ @@ -81,6 +83,8 @@ static ulong tftp_block_wrap; /* memory offset due to wrapping */ static ulong tftp_block_wrap_offset; static int tftp_state; +static ulong tftp_load_addr; +static ulong tftp_load_size; #ifdef CONFIG_TFTP_TSIZE /* The file size reported by the server */ static int tftp_tsize; @@ -164,10 +168,11 @@ static void mcast_cleanup(void)

#endif /* CONFIG_MCAST_TFTP */

-static inline void store_block(int block, uchar *src, unsigned len) +static inline int store_block(int block, uchar *src, unsigned int len) { ulong offset = block * tftp_block_size + tftp_block_wrap_offset; ulong newsize = offset + len; + ulong store_addr = tftp_load_addr + offset; #ifdef CONFIG_SYS_DIRECT_FLASH_TFTP int i, rc = 0;

@@ -175,24 +180,30 @@ static inline void store_block(int block, uchar *src, unsigned len) /* start address in flash? */ if (flash_info[i].flash_id == FLASH_UNKNOWN) continue; - if (load_addr + offset >= flash_info[i].start[0]) { + if (store_addr >= flash_info[i].start[0]) { rc = 1; break; } }

if (rc) { /* Flash is destination for this packet */ - rc = flash_write((char *)src, (ulong)(load_addr+offset), len); + rc = flash_write((char *)src, store_addr, len); if (rc) { flash_perror(rc); - net_set_state(NETLOOP_FAIL); - return; + return rc; } } else #endif /* CONFIG_SYS_DIRECT_FLASH_TFTP */ { - void *ptr = map_sysmem(load_addr + offset, len); + void *ptr;

+ if (store_addr < tftp_load_addr || + store_addr + len > tftp_load_addr + tftp_load_size) { + puts("\nTFTP error: "); + puts("trying to overwrite reserved memory...\n"); + return -1; + } + ptr = map_sysmem(store_addr, len); memcpy(ptr, src, len); unmap_sysmem(ptr); } @@ -203,6 +214,8 @@ static inline void store_block(int block, uchar *src, unsigned len)

if (net_boot_file_size < newsize) net_boot_file_size = newsize; + + return 0; }

/* Clear our state ready for a new transfer */ @@ -606,7 +619,11 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip, timeout_count_max = tftp_timeout_count_max; net_set_timeout_handler(timeout_ms, tftp_timeout_handler);

- store_block(tftp_cur_block - 1, pkt + 2, len); + if (store_block(tftp_cur_block - 1, pkt + 2, len)) { + eth_halt(); + net_set_state(NETLOOP_FAIL); + break; + }

/* * Acknowledge the block just received, which will prompt @@ -695,6 +712,24 @@ static void tftp_timeout_handler(void) } }

+/* Initialize tftp_load_addr and tftp_load_size from load_addr and lmb */ +static int tftp_init_load_addr(void) +{ + struct lmb lmb; + phys_size_t max_size; + + tftp_load_addr = load_addr; + + lmb_init_and_reserve(&lmb, gd->bd->bi_dram[0].start, + gd->bd->bi_dram[0].size, (void *)gd->fdt_blob); + + max_size = lmb_get_unreserved_size(&lmb, tftp_load_addr); + if (!max_size) + return -1; + + tftp_load_size = max_size; + return 0; +}

void tftp_start(enum proto_t protocol) { @@ -791,7 +826,14 @@ void tftp_start(enum proto_t protocol) } else #endif { - printf("Load address: 0x%lx\n", load_addr); + if (tftp_init_load_addr()) { + eth_halt(); + net_set_state(NETLOOP_FAIL); + puts("\nTFTP error: "); + puts("trying to overwrite reserved memory...\n"); + return; + } + printf("Load address: 0x%lx\n", tftp_load_addr); puts("Loading: *\b"); tftp_state = STATE_SEND_RRQ; #ifdef CONFIG_CMD_BOOTEFI @@ -842,9 +884,15 @@ void tftp_start_server(void) { tftp_filename[0] = 0;

+ if (tftp_init_load_addr()) { + eth_halt(); + net_set_state(NETLOOP_FAIL); + puts("\nTFTP error: trying to overwrite reserved memory...\n"); + return; + } printf("Using %s device\n", eth_get_name()); printf("Listening for TFTP transfer on %pI4\n", &net_ip); - printf("Load address: 0x%lx\n", load_addr); + printf("Load address: 0x%lx\n", tftp_load_addr);

puts("Loading: *\b");