Skip to contentSkip to conversation

Fix CVE-2018-18440 and CVE-2018-18439

PATCH Superseded Simon Goldschmidt 2 participants 18 messages Nov 24, 2018 – Dec 5, 2018 2 messages not in the archive

Subsystems (3)

v4 of 10 Superseded Latest is v10 →

lore
Share & tools
lore
#0 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

This series fixes CVE-2018-18440 ("insufficient boundary checks in filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed and reserved memory. Then, the file size to load is checked against these addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in boot_fdt_reserve_region
- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted patch 8

Changes in v3:
- No patch changes, but needed to resend since patman added too many cc addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

 common/bootm.c     |  8 ++----
 common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
 fs/fs.c            | 56 +++++++++++++++++++++++++++++++++++--
 include/lmb.h      |  7 +++--
 lib/lmb.c          | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 net/tftp.c         | 66 ++++++++++++++++++++++++++++++++++++++------
 6 files changed, 231 insertions(+), 28 deletions(-)
Signature · 1 line
2.17.1
#1 lore raw

PATCH v4 1/7 lib: lmb: reserving overlapping regions should fail

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.

---

Changes in v4: None
Changes in v2: None

 lib/lmb.c | 3 +++
 1 file changed, 3 insertions(+)
lib/lmb.c +3 -0
index 1705417348..8dc703d996 100644
@@ -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;
 		}
 	}
 
Signature · 1 line
2.17.1
#2 lore raw

PATCH v4 2/7 fdt: parse "reserved-memory" for memory reservation

In reply to Simon Goldschmidt

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.

---

Changes in v4:
- fixed invalid 'if' statement without braces in boot_fdt_reserve_region

Changes in v2:
- this patch is new in v2

 common/image-fdt.c | 53 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 8 deletions(-)
common/image-fdt.c +45 -8
index 95748f0ae1..5c0d6db3fe 100644
@@ -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,66 @@ 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);
+		}
 	}
 }
 
Signature · 1 line
2.17.1
#3 lore raw

PATCH v4 3/7 lib: lmb: extend lmb for checks at load time

In reply to Simon Goldschmidt

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.

---

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(+)
include/lmb.h +3 -0
index f04d058093..7d7e2a78dc 100644
@@ -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);
 
lib/lmb.c +53 -0

59 lines not on this page · raw message

Signature · 1 line
2.17.1
#4 lore raw

PATCH v4 4/7 fs: prevent overwriting reserved memory

In reply to Simon Goldschmidt

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.

---

Changes in v4: None
Changes in v2: None

 fs/fs.c       | 56 ++++++++++++++++++++++++++++++++++++++++++++++++---
 include/lmb.h |  2 ++
 lib/lmb.c     | 13 ++++++++++++
 3 files changed, 68 insertions(+), 3 deletions(-)
fs/fs.c +53 -3
index adae98d021..4baf6b1c39 100644
@@ -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;
include/lmb.h +2 -0
index 7d7e2a78dc..62da85e716 100644
@@ -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);
lib/lmb.c +13 -0
index 9de1581972..776aa7a8b7 100644
@@ -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)
 {
Signature · 1 line
2.17.1
#5 lore raw

PATCH v4 5/7 bootm: use new common function lmb_init_and_reserve

In reply to Simon Goldschmidt

This reduces duplicate code only.

---

Changes in v4: None
Changes in v2: None

 common/bootm.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
common/bootm.c +2 -6
index 8bf84ebcb7..31e4f0f794 100644
@@ -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)
Signature · 1 line
2.17.1
#6 lore raw

PATCH v4 6/7 lmb: remove unused extern declaration

In reply to Simon Goldschmidt

lmb.h includes an extern declaration of "struct lmb lmb;" which is not used anywhere, so remove it.

---

Changes in v4: None
Changes in v2:
- this patch is new in v2

 include/lmb.h | 2 --
 1 file changed, 2 deletions(-)
include/lmb.h +0 -2
index 62da85e716..1bb003e35e 100644
@@ -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);
Signature · 1 line
2.17.1
#7 lore raw

PATCH v4 7/7 tftp: prevent overwriting reserved memory

In reply to Simon Goldschmidt

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.

---

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(-)
net/tftp.c +57 -9
index 68ffd81414..d31364166e 100644
@@ -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");
 
Signature · 1 line
2.17.1
#8 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

In reply to Simon Goldschmidt

Hi Simon,

after applying these Patch-series i cannot load to any address (fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

regards Frank

Quoted 55 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe Hershberger" <[email protected]>
Cc: "Alexey Brodkin" <[email protected]>, "Heinrich Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>
Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

This series fixes CVE-2018-18440 ("insufficient boundary checks in filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed and reserved memory. Then, the file size to load is checked against these addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in boot_fdt_reserve_region
- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted patch 8

Changes in v3:
- No patch changes, but needed to resend since patman added too many cc addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

 common/bootm.c     |  8 ++----
 common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
 fs/fs.c            | 56 +++++++++++++++++++++++++++++++++++--
 include/lmb.h      |  7 +++--
 lib/lmb.c          | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 net/tftp.c         | 66 ++++++++++++++++++++++++++++++++++++++------
 6 files changed, 231 insertions(+), 28 deletions(-)
Signature · 6 lines
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot
#9 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

Quoted 4 lines · Frank Wunderlich

Hi Simon,

after applying these Patch-series i cannot load to any address (fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

No, you should not need additional patches. The code makes use of "lmb" memory allocation just like the "bootm" code does. The "memory reservation" patch you cited only ensures that memory which is marked as reserved in the fdt cannot be overwritten by load.

If it doesn't work for you at all, the available memory is probably not described correctly. Could you check the values of the following defines (or if they are defined at all):
- CONFIG_SYS_SDRAM_BASE
- CONFIG_ARM
- CONFIG_NR_DRAM_BANKS

I might need to improve the DRAM detection code in v5 (which is still pending as I am working on lmb tests).

Regards,
Simon

Quoted 58 lines

regards Frank

Quoted 55 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe Hershberger" <[email protected]>
Cc: "Alexey Brodkin" <[email protected]>, "Heinrich Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>
Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

This series fixes CVE-2018-18440 ("insufficient boundary checks in filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed and reserved memory. Then, the file size to load is checked against these addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in boot_fdt_reserve_region
- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted patch 8

Changes in v3:
- No patch changes, but needed to resend since patman added too many cc addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

 common/bootm.c     |  8 ++----
 common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
 fs/fs.c            | 56 +++++++++++++++++++++++++++++++++++--
 include/lmb.h      |  7 +++--
 lib/lmb.c          | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 net/tftp.c         | 66 ++++++++++++++++++++++++++++++++++++++------
 6 files changed, 231 insertions(+), 28 deletions(-)

--
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

#10 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

Hi Simon

#define CONFIG_SYS_SDRAM_BASE 0x80000000

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/include/configs/mt7623.h#L47

CONFIG_ARM=y
CONFIG_NR_DRAM_BANKS=1
https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/configs/mt7623n_bpir2_defconfig#L7

i applied the patch-series on top of my 2018-11 final (currently removed from github)

https://github.com/frank-w/u-boot/tree/bpi-r2_v5

tried ${scriptaddr}=0x83000000

here the fatload-command:

https://github.com/frank-w/u-boot/blob/60bc4075c7744e36058fcba76cd6e6c3a4002265/uEnv.txt#L22

working before, 0x81000000 and some higher values

HTH

regards Frank

Quoted 90 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:25 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: "U-Boot Mailing List" <[email protected]>
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Quoted 4 lines · Frank Wunderlich

Hi Simon,

after applying these Patch-series i cannot load to any address (fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

No, you should not need additional patches. The code makes use of "lmb" memory allocation just like the "bootm" code does. The "memory reservation" patch you cited only ensures that memory which is marked as reserved in the fdt cannot be overwritten by load.

If it doesn't work for you at all, the available memory is probably not described correctly. Could you check the values of the following defines (or if they are defined at all):
- CONFIG_SYS_SDRAM_BASE
- CONFIG_ARM
- CONFIG_NR_DRAM_BANKS

I might need to improve the DRAM detection code in v5 (which is still pending as I am working on lmb tests).

Regards,
Simon

Quoted 58 lines

regards Frank

Quoted 55 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe Hershberger" <[email protected]>
Cc: "Alexey Brodkin" <[email protected]>, "Heinrich Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>
Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

This series fixes CVE-2018-18440 ("insufficient boundary checks in filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed and reserved memory. Then, the file size to load is checked against these addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in boot_fdt_reserve_region
- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted patch 8

Changes in v3:
- No patch changes, but needed to resend since patman added too many cc addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

 common/bootm.c     |  8 ++----
 common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
 fs/fs.c            | 56 +++++++++++++++++++++++++++++++++++--
 include/lmb.h      |  7 +++--
 lib/lmb.c          | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 net/tftp.c         | 66 ++++++++++++++++++++++++++++++++++++++------
 6 files changed, 231 insertions(+), 28 deletions(-)

--
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

#11 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

forgot error-message and detailed command:

fatload ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv} ** Reading file would overwrite reserved memory **
echo ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv} mmc 1:1 0x83000000 bananapi/bpi-r2/linux/uEnv.txt

file exists i checked with test, but fatload failed, after reverting the Patches same command works

regards Frank

Quoted 127 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:46 Uhr
Von: "Frank Wunderlich" <[email protected]>
An: "Simon Goldschmidt" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Hi Simon

#define CONFIG_SYS_SDRAM_BASE 0x80000000

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/include/configs/mt7623.h#L47

CONFIG_ARM=y
CONFIG_NR_DRAM_BANKS=1
https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/configs/mt7623n_bpir2_defconfig#L7

i applied the patch-series on top of my 2018-11 final (currently removed from github)

https://github.com/frank-w/u-boot/tree/bpi-r2_v5

tried ${scriptaddr}=0x83000000

here the fatload-command:

https://github.com/frank-w/u-boot/blob/60bc4075c7744e36058fcba76cd6e6c3a4002265/uEnv.txt#L22

working before, 0x81000000 and some higher values

HTH

regards Frank

Quoted 25 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:25 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: "U-Boot Mailing List" <[email protected]>
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Quoted 4 lines · Frank Wunderlich

Hi Simon,

after applying these Patch-series i cannot load to any address (fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

No, you should not need additional patches. The code makes use of "lmb" memory allocation just like the "bootm" code does. The "memory reservation" patch you cited only ensures that memory which is marked as reserved in the fdt cannot be overwritten by load.

If it doesn't work for you at all, the available memory is probably not described correctly. Could you check the values of the following defines (or if they are defined at all):
- CONFIG_SYS_SDRAM_BASE
- CONFIG_ARM
- CONFIG_NR_DRAM_BANKS

Quoted 65 lines

I might need to improve the DRAM detection code in v5 (which is still pending as I am working on lmb tests).

Regards,
Simon

Quoted 58 lines

regards Frank

Quoted 55 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe Hershberger" <[email protected]>
Cc: "Alexey Brodkin" <[email protected]>, "Heinrich Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>
Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

This series fixes CVE-2018-18440 ("insufficient boundary checks in filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed and reserved memory. Then, the file size to load is checked against these addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in boot_fdt_reserve_region
- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted patch 8

Changes in v3:
- No patch changes, but needed to resend since patman added too many cc addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

 common/bootm.c     |  8 ++----
 common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
 fs/fs.c            | 56 +++++++++++++++++++++++++++++++++++--
 include/lmb.h      |  7 +++--
 lib/lmb.c          | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 net/tftp.c         | 66 ++++++++++++++++++++++++++++++++++++++------
 6 files changed, 231 insertions(+), 28 deletions(-)

--
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

#12 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

Am 01.12.2018 um 12:07 schrieb Frank Wunderlich:

Quoted 8 lines

forgot error-message and detailed command:

fatload ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv} ** Reading file would overwrite reserved memory **
echo ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv} mmc 1:1 0x83000000 bananapi/bpi-r2/linux/uEnv.txt

file exists i checked with test, but fatload failed, after reverting the Patches same command works

Hmm, ok. With your configuration, I thought 'gd->bd->bi_dram[0].start' and 'gd->bd->bi_dram[0].size' should be populated and correctly describe your DRAM.

Could you try adding this printf code to the function 'lmb_init_and_reserve':

	printf("lmb_init: base: 0x%x, size: 0x%x\n", base, size);

and check if this correctly describes your memory?

Thanks,
Simon

Quoted 130 lines

regards Frank

Quoted 127 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:46 Uhr
Von: "Frank Wunderlich" <[email protected]>
An: "Simon Goldschmidt" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Hi Simon

#define CONFIG_SYS_SDRAM_BASE 0x80000000

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/include/configs/mt7623.h#L47

CONFIG_ARM=y
CONFIG_NR_DRAM_BANKS=1
https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/configs/mt7623n_bpir2_defconfig#L7

i applied the patch-series on top of my 2018-11 final (currently removed from github)

https://github.com/frank-w/u-boot/tree/bpi-r2_v5

tried ${scriptaddr}=0x83000000

here the fatload-command:

https://github.com/frank-w/u-boot/blob/60bc4075c7744e36058fcba76cd6e6c3a4002265/uEnv.txt#L22

working before, 0x81000000 and some higher values

HTH

regards Frank

Quoted 25 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:25 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: "U-Boot Mailing List" <[email protected]>
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Quoted 4 lines · Frank Wunderlich

Hi Simon,

after applying these Patch-series i cannot load to any address (fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

No, you should not need additional patches. The code makes use of "lmb" memory allocation just like the "bootm" code does. The "memory reservation" patch you cited only ensures that memory which is marked as reserved in the fdt cannot be overwritten by load.

If it doesn't work for you at all, the available memory is probably not described correctly. Could you check the values of the following defines (or if they are defined at all):
- CONFIG_SYS_SDRAM_BASE
- CONFIG_ARM
- CONFIG_NR_DRAM_BANKS

Quoted 65 lines

I might need to improve the DRAM detection code in v5 (which is still pending as I am working on lmb tests).

Regards,
Simon

Quoted 58 lines

regards Frank

Quoted 55 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe Hershberger" <[email protected]>
Cc: "Alexey Brodkin" <[email protected]>, "Heinrich Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>
Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

This series fixes CVE-2018-18440 ("insufficient boundary checks in filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed and reserved memory. Then, the file size to load is checked against these addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in boot_fdt_reserve_region
- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted patch 8

Changes in v3:
- No patch changes, but needed to resend since patman added too many cc addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

  common/bootm.c     |  8 ++----
  common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
  fs/fs.c            | 56 +++++++++++++++++++++++++++++++++++--
  include/lmb.h      |  7 +++--
  lib/lmb.c          | 69 ++++++++++++++++++++++++++++++++++++++++++++++
  net/tftp.c         | 66 ++++++++++++++++++++++++++++++++++++++------
  6 files changed, 231 insertions(+), 28 deletions(-)

--
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

A message that is not in the archive (sent off-list or not yet received): trinity-ea7ab694-86d5-44f3-8d7c-4ee89f4952f4-1543765723523@3c-app-gmx-bs16

#14 lore raw
Quoted 175 lines

Gesendet: Sonntag, 02. Dezember 2018 um 16:48 Uhr
Von: "Frank Wunderlich" <[email protected]>
An: "Simon Goldschmidt" <[email protected]>
Betreff: Aw: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

lmb_init: base: 0x80000000, size: 0x80000000

https://github.com/frank-w/u-boot/commit/e0763252a8e135f00b996fbda7bb1067a192ca53

Regards Frank

Quoted 162 lines

Gesendet: Sonntag, 02. Dezember 2018 um 10:24 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Am 01.12.2018 um 12:07 schrieb Frank Wunderlich:

Quoted 8 lines

forgot error-message and detailed command:

fatload ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv} ** Reading file would overwrite reserved memory **
echo ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv} mmc 1:1 0x83000000 bananapi/bpi-r2/linux/uEnv.txt

file exists i checked with test, but fatload failed, after reverting the Patches same command works

Hmm, ok. With your configuration, I thought 'gd->bd->bi_dram[0].start' and 'gd->bd->bi_dram[0].size' should be populated and correctly describe your DRAM.

Could you try adding this printf code to the function 'lmb_init_and_reserve':

	printf("lmb_init: base: 0x%x, size: 0x%x\n", base, size);

and check if this correctly describes your memory?

Thanks,
Simon

Quoted 130 lines

regards Frank

Quoted 127 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:46 Uhr
Von: "Frank Wunderlich" <[email protected]>
An: "Simon Goldschmidt" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Hi Simon

#define CONFIG_SYS_SDRAM_BASE 0x80000000

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/include/configs/mt7623.h#L47

CONFIG_ARM=y
CONFIG_NR_DRAM_BANKS=1
https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/configs/mt7623n_bpir2_defconfig#L7

i applied the patch-series on top of my 2018-11 final (currently removed from github)

https://github.com/frank-w/u-boot/tree/bpi-r2_v5

tried ${scriptaddr}=0x83000000

here the fatload-command:

https://github.com/frank-w/u-boot/blob/60bc4075c7744e36058fcba76cd6e6c3a4002265/uEnv.txt#L22

working before, 0x81000000 and some higher values

HTH

regards Frank

Quoted 25 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:25 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: "U-Boot Mailing List" <[email protected]>
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Quoted 4 lines · Frank Wunderlich

Hi Simon,

after applying these Patch-series i cannot load to any address (fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

No, you should not need additional patches. The code makes use of "lmb" memory allocation just like the "bootm" code does. The "memory reservation" patch you cited only ensures that memory which is marked as reserved in the fdt cannot be overwritten by load.

If it doesn't work for you at all, the available memory is probably not described correctly. Could you check the values of the following defines (or if they are defined at all):
- CONFIG_SYS_SDRAM_BASE
- CONFIG_ARM
- CONFIG_NR_DRAM_BANKS

Quoted 65 lines

I might need to improve the DRAM detection code in v5 (which is still pending as I am working on lmb tests).

Regards,
Simon

Quoted 58 lines

regards Frank

Quoted 55 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe Hershberger" <[email protected]>
Cc: "Alexey Brodkin" <[email protected]>, "Heinrich Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>
Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

This series fixes CVE-2018-18440 ("insufficient boundary checks in filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed and reserved memory. Then, the file size to load is checked against these addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in boot_fdt_reserve_region
- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted patch 8

Changes in v3:
- No patch changes, but needed to resend since patman added too many cc addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

  common/bootm.c     |  8 ++----
  common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
  fs/fs.c            | 56 +++++++++++++++++++++++++++++++++++--
  include/lmb.h      |  7 +++--
  lib/lmb.c          | 69 ++++++++++++++++++++++++++++++++++++++++++++++
  net/tftp.c         | 66 ++++++++++++++++++++++++++++++++++++++------
  6 files changed, 231 insertions(+), 28 deletions(-)

--
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

#15 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

In reply to a message not in the archive

Am 02.12.2018 um 16:48 schrieb Frank Wunderlich:

Quoted 1 line

lmb_init: base: 0x80000000, size: 0x80000000

Ok, so I don't know your board, is that correct? Do you have 2 GByte starting at 0x80000000?

If so, that would result in an overflow to 0 on a 32-bit platform and would explain why it doesn't work.

This is a perfect input for a test case :-)

Thanks,
Simon

Quoted 167 lines

https://github.com/frank-w/u-boot/commit/e0763252a8e135f00b996fbda7bb1067a192ca53

Regards Frank

Quoted 162 lines

Gesendet: Sonntag, 02. Dezember 2018 um 10:24 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Am 01.12.2018 um 12:07 schrieb Frank Wunderlich:

Quoted 8 lines

forgot error-message and detailed command:

fatload ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv} ** Reading file would overwrite reserved memory **
echo ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv} mmc 1:1 0x83000000 bananapi/bpi-r2/linux/uEnv.txt

file exists i checked with test, but fatload failed, after reverting the Patches same command works

Hmm, ok. With your configuration, I thought 'gd->bd->bi_dram[0].start' and 'gd->bd->bi_dram[0].size' should be populated and correctly describe your DRAM.

Could you try adding this printf code to the function 'lmb_init_and_reserve':

	printf("lmb_init: base: 0x%x, size: 0x%x\n", base, size);

and check if this correctly describes your memory?

Thanks,
Simon

Quoted 130 lines

regards Frank

Quoted 127 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:46 Uhr
Von: "Frank Wunderlich" <[email protected]>
An: "Simon Goldschmidt" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Hi Simon

#define CONFIG_SYS_SDRAM_BASE 0x80000000

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/include/configs/mt7623.h#L47

CONFIG_ARM=y
CONFIG_NR_DRAM_BANKS=1
https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/configs/mt7623n_bpir2_defconfig#L7

i applied the patch-series on top of my 2018-11 final (currently removed from github)

https://github.com/frank-w/u-boot/tree/bpi-r2_v5

tried ${scriptaddr}=0x83000000

here the fatload-command:

https://github.com/frank-w/u-boot/blob/60bc4075c7744e36058fcba76cd6e6c3a4002265/uEnv.txt#L22

working before, 0x81000000 and some higher values

HTH

regards Frank

Quoted 25 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:25 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: "U-Boot Mailing List" <[email protected]>
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Quoted 4 lines · Frank Wunderlich

Hi Simon,

after applying these Patch-series i cannot load to any address (fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

No, you should not need additional patches. The code makes use of "lmb" memory allocation just like the "bootm" code does. The "memory reservation" patch you cited only ensures that memory which is marked as reserved in the fdt cannot be overwritten by load.

If it doesn't work for you at all, the available memory is probably not described correctly. Could you check the values of the following defines (or if they are defined at all):
- CONFIG_SYS_SDRAM_BASE
- CONFIG_ARM
- CONFIG_NR_DRAM_BANKS

Quoted 65 lines

I might need to improve the DRAM detection code in v5 (which is still pending as I am working on lmb tests).

Regards,
Simon

Quoted 58 lines

regards Frank

Quoted 55 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe Hershberger" <[email protected]>
Cc: "Alexey Brodkin" <[email protected]>, "Heinrich Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>
Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

This series fixes CVE-2018-18440 ("insufficient boundary checks in filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed and reserved memory. Then, the file size to load is checked against these addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in boot_fdt_reserve_region
- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted patch 8

Changes in v3:
- No patch changes, but needed to resend since patman added too many cc

    addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

   common/bootm.c     |  8 ++----
   common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
   fs/fs.c            | 56 +++++++++++++++++++++++++++++++++++--
   include/lmb.h      |  7 +++--
   lib/lmb.c          | 69 ++++++++++++++++++++++++++++++++++++++++++++++
   net/tftp.c         | 66 ++++++++++++++++++++++++++++++++++++++------
   6 files changed, 231 insertions(+), 28 deletions(-)

--
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

#16 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

Right, i test on bananapi-r2. 2gb ram with 0x80000000 base-adress

Seems you need (unsigned) int64 for calculations

Am 2. Dezember 2018 18:14:19 MEZ schrieb Simon Goldschmidt <[email protected]>:

Quoted 217 lines

Am 02.12.2018 um 16:48 schrieb Frank Wunderlich:

Quoted 1 line

lmb_init: base: 0x80000000, size: 0x80000000

Ok, so I don't know your board, is that correct? Do you have 2 GByte starting at 0x80000000?

If so, that would result in an overflow to 0 on a 32-bit platform and would explain why it doesn't work.

This is a perfect input for a test case :-)

Thanks,
Simon

Quoted 2 lines

https://github.com/frank-w/u-boot/commit/e0763252a8e135f00b996fbda7bb1067a192ca53

Quoted 8 lines

Regards Frank

Quoted 5 lines

Gesendet: Sonntag, 02. Dezember 2018 um 10:24 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 5 lines
Quoted 5 lines

Am 01.12.2018 um 12:07 schrieb Frank Wunderlich:

Quoted 3 lines

forgot error-message and detailed command:

fatload ${device} ${partition} ${scriptaddr}

${bpi}/${board}/${service}/${bootenv}

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

** Reading file would overwrite reserved memory ** echo ${device} ${partition} ${scriptaddr}

${bpi}/${board}/${service}/${bootenv}

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

mmc 1:1 0x83000000 bananapi/bpi-r2/linux/uEnv.txt

file exists i checked with test, but fatload failed, after

reverting the Patches same command works

Quoted 2 lines
Quoted 2 lines

Hmm, ok. With your configuration, I thought

'gd->bd->bi_dram[0].start'

Quoted 1 line
Quoted 1 line

and 'gd->bd->bi_dram[0].size' should be populated and correctly

describe

Quoted 13 lines
Quoted 13 lines

your DRAM.

Could you try adding this printf code to the function 'lmb_init_and_reserve':

	printf("lmb_init: base: 0x%x, size: 0x%x\n", base, size);

and check if this correctly describes your memory?

Thanks,
Simon

Quoted 1 line
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

regards Frank

Quoted 5 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:46 Uhr
Von: "Frank Wunderlich" <[email protected]>
An: "Simon Goldschmidt" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Hi Simon

#define CONFIG_SYS_SDRAM_BASE 0x80000000

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/include/configs/mt7623.h#L47

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

CONFIG_ARM=y
CONFIG_NR_DRAM_BANKS=1

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/configs/mt7623n_bpir2_defconfig#L7

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

i applied the patch-series on top of my 2018-11 final (currently

removed from github)

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

https://github.com/frank-w/u-boot/tree/bpi-r2_v5

tried ${scriptaddr}=0x83000000

here the fatload-command:

https://github.com/frank-w/u-boot/blob/60bc4075c7744e36058fcba76cd6e6c3a4002265/uEnv.txt#L22

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

working before, 0x81000000 and some higher values

HTH

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

regards Frank

Quoted 5 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:25 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: "U-Boot Mailing List" <[email protected]>
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines · Frank Wunderlich

Hi Simon,

after applying these Patch-series i cannot load to any address

(fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

No, you should not need additional patches. The code makes use of "lmb" memory allocation just like the "bootm" code does. The

"memory

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

reservation" patch you cited only ensures that memory which is

marked

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

as reserved in the fdt cannot be overwritten by load.

If it doesn't work for you at all, the available memory is

probably

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

not described correctly. Could you check the values of the

following

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

defines (or if they are defined at all):
- CONFIG_SYS_SDRAM_BASE
- CONFIG_ARM
- CONFIG_NR_DRAM_BANKS

I might need to improve the DRAM detection code in v5 (which is

still

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

pending as I am working on lmb tests).

Regards,
Simon

Quoted 6 lines

regards Frank

Quoted 3 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe

Hershberger" <[email protected]>

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Cc "Alexey Brodkin" <[email protected]>, "Heinrich

Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

This series fixes CVE-2018-18440 ("insufficient boundary checks

in

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks

in

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed

and

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

reserved memory. Then, the file size to load is checked against

these

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in

boot_fdt_reserve_region

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted

patch 8

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

Changes in v3:
- No patch changes, but needed to resend since patman added too

many cc

Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
    addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
   common/bootm.c     |  8 ++----
   common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
   fs/fs.c            | 56

+++++++++++++++++++++++++++++++++++--

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
   include/lmb.h      |  7 +++--
   lib/lmb.c          | 69

++++++++++++++++++++++++++++++++++++++++++++++

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
   net/tftp.c         | 66

++++++++++++++++++++++++++++++++++++++------

Quoted 18 lines
Quoted 18 lines
Quoted 16 lines
Quoted 16 lines
Quoted 11 lines
Quoted 10 lines
Quoted 10 lines
   6 files changed, 231 insertions(+), 28 deletions(-)

--
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

#17 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

Quoted 4 lines · Frank Wunderlich

Right, i test on bananapi-r2. 2gb ram with 0x80000000 base-adress

Seems you need (unsigned) int64 for calculations

Hmm, I don't know if that's enough. Of course using uint64_t would fix it on 32 bit systems, but I don't like the idea that it's then broken for 64-bit systems allocating something to the end of the address space...

The existing lmb functions to check for overlap cannot handle "size + length " overflowing to 0. I might be able to fix that without promoting to the next higher type.

Regards,
Simon

Quoted 219 lines

Am 2. Dezember 2018 18:14:19 MEZ schrieb Simon Goldschmidt <[email protected]>:

Quoted 217 lines

Am 02.12.2018 um 16:48 schrieb Frank Wunderlich:

Quoted 1 line

lmb_init: base: 0x80000000, size: 0x80000000

Ok, so I don't know your board, is that correct? Do you have 2 GByte starting at 0x80000000?

If so, that would result in an overflow to 0 on a 32-bit platform and would explain why it doesn't work.

This is a perfect input for a test case :-)

Thanks,
Simon

Quoted 2 lines

https://github.com/frank-w/u-boot/commit/e0763252a8e135f00b996fbda7bb1067a192ca53

Quoted 8 lines

Regards Frank

Quoted 5 lines

Gesendet: Sonntag, 02. Dezember 2018 um 10:24 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 5 lines
Quoted 5 lines

Am 01.12.2018 um 12:07 schrieb Frank Wunderlich:

Quoted 3 lines

forgot error-message and detailed command:

fatload ${device} ${partition} ${scriptaddr}

${bpi}/${board}/${service}/${bootenv}

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

** Reading file would overwrite reserved memory ** echo ${device} ${partition} ${scriptaddr}

${bpi}/${board}/${service}/${bootenv}

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

mmc 1:1 0x83000000 bananapi/bpi-r2/linux/uEnv.txt

file exists i checked with test, but fatload failed, after

reverting the Patches same command works

Quoted 2 lines
Quoted 2 lines

Hmm, ok. With your configuration, I thought

'gd->bd->bi_dram[0].start'

Quoted 1 line
Quoted 1 line

and 'gd->bd->bi_dram[0].size' should be populated and correctly

describe

Quoted 13 lines
Quoted 13 lines

your DRAM.

Could you try adding this printf code to the function 'lmb_init_and_reserve':

    printf("lmb_init: base: 0x%x, size: 0x%x\n", base, size);

and check if this correctly describes your memory?

Thanks,
Simon

Quoted 1 line
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

regards Frank

Quoted 5 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:46 Uhr
Von: "Frank Wunderlich" <[email protected]>
An: "Simon Goldschmidt" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Hi Simon

#define CONFIG_SYS_SDRAM_BASE 0x80000000

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/include/configs/mt7623.h#L47

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

CONFIG_ARM=y
CONFIG_NR_DRAM_BANKS=1

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/configs/mt7623n_bpir2_defconfig#L7

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

i applied the patch-series on top of my 2018-11 final (currently

removed from github)

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

https://github.com/frank-w/u-boot/tree/bpi-r2_v5

tried ${scriptaddr}=0x83000000

here the fatload-command:

https://github.com/frank-w/u-boot/blob/60bc4075c7744e36058fcba76cd6e6c3a4002265/uEnv.txt#L22

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

working before, 0x81000000 and some higher values

HTH

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

regards Frank

Quoted 5 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:25 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: "U-Boot Mailing List" <[email protected]>
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines · Frank Wunderlich

Hi Simon,

after applying these Patch-series i cannot load to any address

(fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

No, you should not need additional patches. The code makes use of "lmb" memory allocation just like the "bootm" code does. The

"memory

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

reservation" patch you cited only ensures that memory which is

marked

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

as reserved in the fdt cannot be overwritten by load.

If it doesn't work for you at all, the available memory is

probably

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

not described correctly. Could you check the values of the

following

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

defines (or if they are defined at all):
- CONFIG_SYS_SDRAM_BASE
- CONFIG_ARM
- CONFIG_NR_DRAM_BANKS

I might need to improve the DRAM detection code in v5 (which is

still

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

pending as I am working on lmb tests).

Regards,
Simon

Quoted 6 lines

regards Frank

Quoted 3 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe

Hershberger" <[email protected]>

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Cc "Alexey Brodkin" <[email protected]>, "Heinrich

Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

This series fixes CVE-2018-18440 ("insufficient boundary checks

in

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks

in

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed

and

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

reserved memory. Then, the file size to load is checked against

these

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in

boot_fdt_reserve_region

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted

patch 8

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

Changes in v3:
- No patch changes, but needed to resend since patman added too

many cc

Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
    addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
   common/bootm.c     |  8 ++----
   common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
   fs/fs.c            | 56

+++++++++++++++++++++++++++++++++++--

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
   include/lmb.h      |  7 +++--
   lib/lmb.c          | 69

++++++++++++++++++++++++++++++++++++++++++++++

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
   net/tftp.c         | 66

++++++++++++++++++++++++++++++++++++++------

Quoted 18 lines
Quoted 18 lines
Quoted 16 lines
Quoted 16 lines
Quoted 11 lines
Quoted 10 lines
Quoted 10 lines
   6 files changed, 231 insertions(+), 28 deletions(-)

--
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

A message that is not in the archive (sent off-list or not yet received): trinity-2a951f0c-95cd-4665-a427-292f48386f4a-1544019621849@3c-app-gmx-bs38

#19 lore raw

v4 cover Fix CVE-2018-18440 and CVE-2018-18439

Am 05.12.2018 um 15:20 schrieb Frank Wunderlich:

Quoted 3 lines

Hi Simon,

have you any new infos here? last discussion is for v3...

I'm working on it. The bug you get is in lmb.c (and has been there for a long time). It is rather hidden by the way the lmb functions are used when booting, but it shows in my new usage.

I'll post v4 soon, hopefully.

Regards,
Simon

Quoted 232 lines

regards Frank

Quoted 229 lines

Gesendet: Sonntag, 02. Dezember 2018 um 18:44 Uhr
Von: "Frank Wunderlich" <[email protected]>
An: "Simon Goldschmidt" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and CVE-2018-18439

Right, i test on bananapi-r2. 2gb ram with 0x80000000 base-adress

Seems you need (unsigned) int64 for calculations

Am 2. Dezember 2018 18:14:19 MEZ schrieb Simon Goldschmidt <[email protected]>:

Quoted 217 lines

Am 02.12.2018 um 16:48 schrieb Frank Wunderlich:

Quoted 1 line

lmb_init: base: 0x80000000, size: 0x80000000

Ok, so I don't know your board, is that correct? Do you have 2 GByte starting at 0x80000000?

If so, that would result in an overflow to 0 on a 32-bit platform and would explain why it doesn't work.

This is a perfect input for a test case :-)

Thanks,
Simon

Quoted 2 lines

https://github.com/frank-w/u-boot/commit/e0763252a8e135f00b996fbda7bb1067a192ca53

Quoted 8 lines

Regards Frank

Quoted 5 lines

Gesendet: Sonntag, 02. Dezember 2018 um 10:24 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 5 lines
Quoted 5 lines

Am 01.12.2018 um 12:07 schrieb Frank Wunderlich:

Quoted 3 lines

forgot error-message and detailed command:

fatload ${device} ${partition} ${scriptaddr}

${bpi}/${board}/${service}/${bootenv}

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

** Reading file would overwrite reserved memory ** echo ${device} ${partition} ${scriptaddr}

${bpi}/${board}/${service}/${bootenv}

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

mmc 1:1 0x83000000 bananapi/bpi-r2/linux/uEnv.txt

file exists i checked with test, but fatload failed, after

reverting the Patches same command works

Quoted 2 lines
Quoted 2 lines

Hmm, ok. With your configuration, I thought

'gd->bd->bi_dram[0].start'

Quoted 1 line
Quoted 1 line

and 'gd->bd->bi_dram[0].size' should be populated and correctly

describe

Quoted 13 lines
Quoted 13 lines

your DRAM.

Could you try adding this printf code to the function 'lmb_init_and_reserve':

	printf("lmb_init: base: 0x%x, size: 0x%x\n", base, size);

and check if this correctly describes your memory?

Thanks,
Simon

Quoted 1 line
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

regards Frank

Quoted 5 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:46 Uhr
Von: "Frank Wunderlich" <[email protected]>
An: "Simon Goldschmidt" <[email protected]>
Cc: u-boot at lists.denx.de
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Hi Simon

#define CONFIG_SYS_SDRAM_BASE 0x80000000

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/include/configs/mt7623.h#L47

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

CONFIG_ARM=y
CONFIG_NR_DRAM_BANKS=1

https://github.com/frank-w/u-boot/blob/a6d0c3f8e992a2e428f05443647fe9f5b13f8634/configs/mt7623n_bpir2_defconfig#L7

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

i applied the patch-series on top of my 2018-11 final (currently

removed from github)

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

https://github.com/frank-w/u-boot/tree/bpi-r2_v5

tried ${scriptaddr}=0x83000000

here the fatload-command:

https://github.com/frank-w/u-boot/blob/60bc4075c7744e36058fcba76cd6e6c3a4002265/uEnv.txt#L22

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

working before, 0x81000000 and some higher values

HTH

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

regards Frank

Quoted 5 lines

Gesendet: Samstag, 01. Dezember 2018 um 10:25 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: "U-Boot Mailing List" <[email protected]>
Betreff: Re: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines · Frank Wunderlich

Hi Simon,

after applying these Patch-series i cannot load to any address

(fatload). Do i need any additional Patch ("fdt: parse "reserved-memory" for memory reservation" sounds like that). Maybe there should be a fallback if no reservation is defined.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

No, you should not need additional patches. The code makes use of "lmb" memory allocation just like the "bootm" code does. The

"memory

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

reservation" patch you cited only ensures that memory which is

marked

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

as reserved in the fdt cannot be overwritten by load.

If it doesn't work for you at all, the available memory is

probably

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

not described correctly. Could you check the values of the

following

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

defines (or if they are defined at all):
- CONFIG_SYS_SDRAM_BASE
- CONFIG_ARM
- CONFIG_NR_DRAM_BANKS

I might need to improve the DRAM detection code in v5 (which is

still

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

pending as I am working on lmb tests).

Regards,
Simon

Quoted 6 lines

regards Frank

Quoted 3 lines

Gesendet: Samstag, 24. November 2018 um 15:11 Uhr
Von: "Simon Goldschmidt" <[email protected]>
An: "Tom Rini" <[email protected]>, u-boot at lists.denx.de, "Joe

Hershberger" <[email protected]>

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Cc "Alexey Brodkin" <[email protected]>, "Heinrich

Schuchardt" <[email protected]>, "Michal Simek" <[email protected]>, "Alexander Graf" <[email protected]>, "Andrea Barisani" <[email protected]>

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Betreff: [U-Boot] [PATCH v4 0/7] Fix CVE-2018-18440 and

CVE-2018-18439

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

This series fixes CVE-2018-18440 ("insufficient boundary checks

in

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks

in

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

network image boot") by adding restrictions to the tftp code. The functions from lmb.c are used to setup regions of allowed

and

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

reserved memory. Then, the file size to load is checked against

these

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

addresses and loading the file is aborted if it would overwrite reserved memory.

The memory reservation code is reused from bootm/image.

Changes in v4:
- fixed invalid 'if' statement without braces in

boot_fdt_reserve_region

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

- removed patch 7 ("net: remove CONFIG_MCAST_TFTP), adapted

patch 8

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

Changes in v3:
- No patch changes, but needed to resend since patman added too

many cc

Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
     addresses that gmail seemed to detect as spam :-(

Changes in v2:
- added code to reserve devicetree reserved-memory in lmb
- added tftp fixes (patches 7 and 8)
- fixed a bug in new function lmb_alloc_addr

Simon Goldschmidt (7):
lib: lmb: reserving overlapping regions should fail
fdt: parse "reserved-memory" for memory reservation
lib: lmb: extend lmb for checks at load time
fs: prevent overwriting reserved memory
bootm: use new common function lmb_init_and_reserve
lmb: remove unused extern declaration
tftp: prevent overwriting reserved memory

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
    common/bootm.c     |  8 ++----
    common/image-fdt.c | 53 +++++++++++++++++++++++++++++------
    fs/fs.c            | 56

+++++++++++++++++++++++++++++++++++--

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
    include/lmb.h      |  7 +++--
    lib/lmb.c          | 69

++++++++++++++++++++++++++++++++++++++++++++++

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
    net/tftp.c         | 66

++++++++++++++++++++++++++++++++++++++------

Quoted 18 lines
Quoted 18 lines
Quoted 16 lines
Quoted 16 lines
Quoted 11 lines
Quoted 10 lines
Quoted 10 lines
    6 files changed, 231 insertions(+), 28 deletions(-)

--
2.17.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot