* [U-Boot] [PATCH v2 1/8] lib: lmb: reserving overlapping regions should fail
[not found] <20181117093430.15827-1-simon.k.r.goldschmidt@gmail.com>
@ 2018-11-17 9:34 ` Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 2/8] fdt: parse "reserved-memory" for memory reservation Simon Goldschmidt
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Simon Goldschmidt @ 2018-11-17 9:34 UTC (permalink / raw)
To: u-boot
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 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;
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2 2/8] fdt: parse "reserved-memory" for memory reservation
[not found] <20181117093430.15827-1-simon.k.r.goldschmidt@gmail.com>
2018-11-17 9:34 ` [U-Boot] [PATCH v2 1/8] lib: lmb: reserving overlapping regions should fail Simon Goldschmidt
@ 2018-11-17 9:34 ` Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 3/8] lib: lmb: extend lmb for checks at load time Simon Goldschmidt
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Simon Goldschmidt @ 2018-11-17 9:34 UTC (permalink / raw)
To: u-boot
boot_fdt_add_mem_rsv_regions() adds reserved memory sections to an lmb
struct. Currently, it only parses regions described by /memreserve/
entries.
Extend this to the more commonly used scheme of the "reserved-memory"
node.
Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
Changes in v2:
- this patch is new in v2
common/image-fdt.c | 52 +++++++++++++++++++++++++++++++++++++++-------
1 file changed, 44 insertions(+), 8 deletions(-)
diff --git a/common/image-fdt.c b/common/image-fdt.c
index 95748f0ae1..65984c3c98 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -10,6 +10,7 @@
#include <common.h>
#include <fdt_support.h>
+#include <fdtdec.h>
#include <errno.h>
#include <image.h>
#include <linux/libfdt.h>
@@ -67,30 +68,65 @@ static const image_header_t *image_get_fdt(ulong fdt_addr)
}
#endif
+static void boot_fdt_reserve_region(struct lmb *lmb, uint64_t addr,
+ uint64_t size)
+{
+ int ret;
+
+ ret = lmb_reserve(lmb, addr, size);
+ if (!ret)
+ debug(" reserving fdt memory region: addr=%llx size=%llx\n",
+ (unsigned long long)addr, (unsigned long long)size);
+ else
+ puts("ERROR: reserving fdt memory region failed ");
+ printf("(addr=%llx size=%llx)\n",
+ (unsigned long long)addr, (unsigned long long)size);
+}
+
/**
- * boot_fdt_add_mem_rsv_regions - Mark the memreserve sections as unusable
+ * boot_fdt_add_mem_rsv_regions - Mark the memreserve and reserved-memory
+ * sections as unusable
* @lmb: pointer to lmb handle, will be used for memory mgmt
* @fdt_blob: pointer to fdt blob base address
*
- * Adds the memreserve regions in the dtb to the lmb block. Adding the
- * memreserve regions prevents u-boot from using them to store the initrd
- * or the fdt blob.
+ * Adds the and reserved-memorymemreserve regions in the dtb to the lmb block.
+ * Adding the memreserve regions prevents u-boot from using them to store the
+ * initrd or the fdt blob.
*/
void boot_fdt_add_mem_rsv_regions(struct lmb *lmb, void *fdt_blob)
{
uint64_t addr, size;
- int i, total;
+ int i, total, ret;
+ int nodeoffset, subnode;
+ struct fdt_resource res;
if (fdt_check_header(fdt_blob) != 0)
return;
+ /* process memreserve sections */
total = fdt_num_mem_rsv(fdt_blob);
for (i = 0; i < total; i++) {
if (fdt_get_mem_rsv(fdt_blob, i, &addr, &size) != 0)
continue;
- printf(" reserving fdt memory region: addr=%llx size=%llx\n",
- (unsigned long long)addr, (unsigned long long)size);
- lmb_reserve(lmb, addr, size);
+ boot_fdt_reserve_region(lmb, addr, size);
+ }
+
+ /* process reserved-memory */
+ nodeoffset = fdt_subnode_offset(fdt_blob, 0, "reserved-memory");
+ if (nodeoffset >= 0) {
+ subnode = fdt_first_subnode(fdt_blob, nodeoffset);
+ while (subnode >= 0) {
+ /* check if this subnode has a reg property */
+ ret = fdt_get_resource(fdt_blob, subnode, "reg", 0,
+ &res);
+ if (!ret) {
+ addr = res.start;
+ size = res.end - res.start + 1;
+ boot_fdt_reserve_region(lmb, addr, size);
+ }
+
+ subnode = fdt_next_subnode(fdt_blob, subnode);
+ }
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2 3/8] lib: lmb: extend lmb for checks at load time
[not found] <20181117093430.15827-1-simon.k.r.goldschmidt@gmail.com>
2018-11-17 9:34 ` [U-Boot] [PATCH v2 1/8] lib: lmb: reserving overlapping regions should fail Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 2/8] fdt: parse "reserved-memory" for memory reservation Simon Goldschmidt
@ 2018-11-17 9:34 ` Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 4/8] fs: prevent overwriting reserved memory Simon Goldschmidt
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Simon Goldschmidt @ 2018-11-17 9:34 UTC (permalink / raw)
To: u-boot
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 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;
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2 4/8] fs: prevent overwriting reserved memory
[not found] <20181117093430.15827-1-simon.k.r.goldschmidt@gmail.com>
` (2 preceding siblings ...)
2018-11-17 9:34 ` [U-Boot] [PATCH v2 3/8] lib: lmb: extend lmb for checks at load time Simon Goldschmidt
@ 2018-11-17 9:34 ` Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 5/8] bootm: use new common function lmb_init_and_reserve Simon Goldschmidt
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Simon Goldschmidt @ 2018-11-17 9:34 UTC (permalink / raw)
To: u-boot
This fixes CVE-2018-18440 ("insufficient boundary checks in filesystem
image load") by using lmb to check the load size of a file against
reserved memory addresses.
Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
Changes in v2: None
fs/fs.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++---
include/lmb.h | 2 ++
lib/lmb.c | 13 ++++++++++++
3 files changed, 68 insertions(+), 3 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c
index adae98d021..4baf6b1c39 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -428,13 +428,57 @@ int fs_size(const char *filename, loff_t *size)
return ret;
}
-int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
- loff_t *actread)
+#ifdef CONFIG_LMB
+/* Check if a file may be read to the given address */
+static int fs_read_lmb_check(const char *filename, ulong addr, loff_t offset,
+ loff_t len, struct fstype_info *info)
+{
+ struct lmb lmb;
+ int ret;
+ loff_t size;
+ loff_t read_len;
+
+ /* get the actual size of the file */
+ ret = info->size(filename, &size);
+ if (ret)
+ return ret;
+ if (offset >= size) {
+ /* offset >= EOF, no bytes will be written */
+ return 0;
+ }
+ read_len = size - offset;
+
+ /* limit to 'len' if it is smaller */
+ if (len && len < read_len)
+ read_len = len;
+
+ lmb_init_and_reserve(&lmb, gd->bd->bi_dram[0].start,
+ gd->bd->bi_dram[0].size, (void *)gd->fdt_blob);
+ lmb_dump_all(&lmb);
+
+ if (lmb_alloc_addr(&lmb, addr, read_len) == addr)
+ return 0;
+
+ printf("** Reading file would overwrite reserved memory **\n");
+ return -1;
+}
+#endif
+
+static int _fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
+ int do_lmb_check, loff_t *actread)
{
struct fstype_info *info = fs_get_info(fs_type);
void *buf;
int ret;
+#ifdef CONFIG_LMB
+ if (do_lmb_check) {
+ ret = fs_read_lmb_check(filename, addr, offset, len, info);
+ if (ret)
+ return ret;
+ }
+#endif
+
/*
* We don't actually know how many bytes are being read, since len==0
* means read the whole file.
@@ -451,6 +495,12 @@ int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
return ret;
}
+int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
+ loff_t *actread)
+{
+ return _fs_read(filename, addr, offset, len, 0, actread);
+}
+
int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
loff_t *actwrite)
{
@@ -621,7 +671,7 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
pos = 0;
time = get_timer(0);
- ret = fs_read(filename, addr, pos, bytes, &len_read);
+ ret = _fs_read(filename, addr, pos, bytes, 1, &len_read);
time = get_timer(time);
if (ret < 0)
return 1;
diff --git a/include/lmb.h b/include/lmb.h
index 7d7e2a78dc..62da85e716 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -31,6 +31,8 @@ struct lmb {
extern struct lmb lmb;
extern void lmb_init(struct lmb *lmb);
+extern void lmb_init_and_reserve(struct lmb *lmb, phys_addr_t base,
+ phys_size_t size, void *fdt_blob);
extern long lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size);
extern long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size);
extern phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align);
diff --git a/lib/lmb.c b/lib/lmb.c
index 9de1581972..776aa7a8b7 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -104,6 +104,19 @@ void lmb_init(struct lmb *lmb)
lmb->reserved.size = 0;
}
+/* Initialize the struct, add memory and call arch/board reserve functions */
+void lmb_init_and_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+ void *fdt_blob)
+{
+ lmb_init(lmb);
+ lmb_add(lmb, base, size);
+ arch_lmb_reserve(lmb);
+ board_lmb_reserve(lmb);
+
+ if (IMAGE_ENABLE_OF_LIBFDT)
+ boot_fdt_add_mem_rsv_regions(lmb, fdt_blob);
+}
+
/* This routine called with relocation disabled. */
static long lmb_add_region(struct lmb_region *rgn, phys_addr_t base, phys_size_t size)
{
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2 5/8] bootm: use new common function lmb_init_and_reserve
[not found] <20181117093430.15827-1-simon.k.r.goldschmidt@gmail.com>
` (3 preceding siblings ...)
2018-11-17 9:34 ` [U-Boot] [PATCH v2 4/8] fs: prevent overwriting reserved memory Simon Goldschmidt
@ 2018-11-17 9:34 ` Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 6/8] lmb: remove unused extern declaration Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 8/8] tftp: prevent overwriting reserved memory Simon Goldschmidt
6 siblings, 0 replies; 8+ messages in thread
From: Simon Goldschmidt @ 2018-11-17 9:34 UTC (permalink / raw)
To: u-boot
This reduces duplicate code only.
Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
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)
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2 6/8] lmb: remove unused extern declaration
[not found] <20181117093430.15827-1-simon.k.r.goldschmidt@gmail.com>
` (4 preceding siblings ...)
2018-11-17 9:34 ` [U-Boot] [PATCH v2 5/8] bootm: use new common function lmb_init_and_reserve Simon Goldschmidt
@ 2018-11-17 9:34 ` Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 8/8] tftp: prevent overwriting reserved memory Simon Goldschmidt
6 siblings, 0 replies; 8+ messages in thread
From: Simon Goldschmidt @ 2018-11-17 9:34 UTC (permalink / raw)
To: u-boot
lmb.h includes an extern declaration of "struct lmb lmb;" which
is not used anywhere, so remove it.
Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
Changes in v2:
- this patch is new in v2
include/lmb.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h
index 62da85e716..1bb003e35e 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -28,8 +28,6 @@ struct lmb {
struct lmb_region reserved;
};
-extern struct lmb lmb;
-
extern void lmb_init(struct lmb *lmb);
extern void lmb_init_and_reserve(struct lmb *lmb, phys_addr_t base,
phys_size_t size, void *fdt_blob);
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2 8/8] tftp: prevent overwriting reserved memory
[not found] <20181117093430.15827-1-simon.k.r.goldschmidt@gmail.com>
` (5 preceding siblings ...)
2018-11-17 9:34 ` [U-Boot] [PATCH v2 6/8] lmb: remove unused extern declaration Simon Goldschmidt
@ 2018-11-17 9:34 ` Simon Goldschmidt
6 siblings, 0 replies; 8+ messages in thread
From: Simon Goldschmidt @ 2018-11-17 9:34 UTC (permalink / raw)
To: u-boot
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 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 563ce3a06f..390394199d 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;
@@ -134,10 +138,11 @@ static char tftp_filename[MAX_LEN];
static unsigned short tftp_block_size = TFTP_BLOCK_SIZE;
static unsigned short tftp_block_size_option = TFTP_MTU_BLOCKSIZE;
-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;
@@ -145,30 +150,38 @@ 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);
}
if (net_boot_file_size < newsize)
net_boot_file_size = newsize;
+
+ return 0;
}
/* Clear our state ready for a new transfer */
@@ -527,7 +540,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
@@ -577,6 +594,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)
{
@@ -673,7 +708,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
@@ -721,9 +763,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");
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2 8/8] tftp: prevent overwriting reserved memory
[not found] <20181117091818.15393-1-simon.k.r.goldschmidt@gmail.com>
@ 2018-11-17 9:18 ` Simon Goldschmidt
0 siblings, 0 replies; 8+ messages in thread
From: Simon Goldschmidt @ 2018-11-17 9:18 UTC (permalink / raw)
To: u-boot
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 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 563ce3a06f..390394199d 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;
@@ -134,10 +138,11 @@ static char tftp_filename[MAX_LEN];
static unsigned short tftp_block_size = TFTP_BLOCK_SIZE;
static unsigned short tftp_block_size_option = TFTP_MTU_BLOCKSIZE;
-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;
@@ -145,30 +150,38 @@ 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);
}
if (net_boot_file_size < newsize)
net_boot_file_size = newsize;
+
+ return 0;
}
/* Clear our state ready for a new transfer */
@@ -527,7 +540,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
@@ -577,6 +594,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)
{
@@ -673,7 +708,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
@@ -721,9 +763,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");
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-17 9:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20181117093430.15827-1-simon.k.r.goldschmidt@gmail.com>
2018-11-17 9:34 ` [U-Boot] [PATCH v2 1/8] lib: lmb: reserving overlapping regions should fail Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 2/8] fdt: parse "reserved-memory" for memory reservation Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 3/8] lib: lmb: extend lmb for checks at load time Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 4/8] fs: prevent overwriting reserved memory Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 5/8] bootm: use new common function lmb_init_and_reserve Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 6/8] lmb: remove unused extern declaration Simon Goldschmidt
2018-11-17 9:34 ` [U-Boot] [PATCH v2 8/8] tftp: prevent overwriting reserved memory Simon Goldschmidt
[not found] <20181117091818.15393-1-simon.k.r.goldschmidt@gmail.com>
2018-11-17 9:18 ` Simon Goldschmidt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.