[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Goldschmidt]

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 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 (8):
  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
  net: remove CONFIG_MCAST_TFTP
  tftp: prevent overwriting reserved memory

 README                       |   9 --
 common/bootm.c               |   8 +-
 common/image-fdt.c           |  52 ++++++-
 drivers/net/rtl8139.c        |   9 --
 drivers/net/tsec.c           |  52 -------
 drivers/usb/gadget/ether.c   |   3 -
 fs/fs.c                      |  56 ++++++-
 include/lmb.h                |   7 +-
 include/net.h                |  17 ---
 lib/lmb.c                    |  69 +++++++++
 net/eth-uclass.c             |   4 -
 net/eth_legacy.c             |  46 ------
 net/net.c                    |   9 +-
 net/tftp.c                   | 289 +++++++----------------------------
 scripts/config_whitelist.txt |   1 -
 15 files changed, 232 insertions(+), 399 deletions(-)

-- 
2.17.1

[U-Boot] [PATCH v3 1/8] lib: lmb: reserving overlapping regions should fail

[Simon Goldschmidt]

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

[U-Boot] [PATCH v3 2/8] fdt: parse "reserved-memory" for memory reservation

[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.

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

[U-Boot] [PATCH v3 3/8] lib: lmb: extend lmb for checks at load time

[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.

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

[U-Boot] [PATCH v3 4/8] fs: prevent overwriting reserved memory

[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.

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

[U-Boot] [PATCH v3 5/8] bootm: use new common function lmb_init_and_reserve

[Simon Goldschmidt]

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

[U-Boot] [PATCH v3 6/8] lmb: remove unused extern declaration

[Simon Goldschmidt]

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

[U-Boot] [PATCH v3 7/8] net: remove CONFIG_MCAST_TFTP

[Simon Goldschmidt]

This option seems unused as no mainline board enables it and it does
not compile since some years.

Additionally, it has a potential buffer underrun issue (reported as
a side node in CVE-2018-18439).

Instead of trying to fix a thing noone seems to use, let's rather
drop dead code.

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

Changes in v2:
- this patch is new in v2

 README                       |   9 --
 drivers/net/rtl8139.c        |   9 --
 drivers/net/tsec.c           |  52 --------
 drivers/usb/gadget/ether.c   |   3 -
 include/net.h                |  17 ---
 net/eth-uclass.c             |   4 -
 net/eth_legacy.c             |  46 --------
 net/net.c                    |   9 +-
 net/tftp.c                   | 223 +----------------------------------
 scripts/config_whitelist.txt |   1 -
 10 files changed, 2 insertions(+), 371 deletions(-)

diff --git a/README b/README
index a46c7c63a4..97a3e9a84b 100644
--- a/README
+++ b/README
@@ -1429,15 +1429,6 @@ The following options need to be configured:
 		forwarded through a router.
 		(Environment variable "netmask")
 
-- Multicast TFTP Mode:
-		CONFIG_MCAST_TFTP
-
-		Defines whether you want to support multicast TFTP as per
-		rfc-2090; for example to work with atftp.  Lets lots of targets
-		tftp down the same boot image concurrently.  Note: the Ethernet
-		driver in use must provide a function: mcast() to join/leave a
-		multicast group.
-
 - BOOTP Recovery Mode:
 		CONFIG_BOOTP_RANDOM_DELAY
 
diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c
index 590f8ce154..79acd64bc0 100644
--- a/drivers/net/rtl8139.c
+++ b/drivers/net/rtl8139.c
@@ -183,12 +183,6 @@ static void rtl_reset(struct eth_device *dev);
 static int rtl_transmit(struct eth_device *dev, void *packet, int length);
 static int rtl_poll(struct eth_device *dev);
 static void rtl_disable(struct eth_device *dev);
-#ifdef CONFIG_MCAST_TFTP/*  This driver already accepts all b/mcast */
-static int rtl_bcast_addr(struct eth_device *dev, const u8 *bcast_mac, u8 set)
-{
-	return (0);
-}
-#endif
 
 static struct pci_device_id supported[] = {
        {PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8139},
@@ -229,9 +223,6 @@ int rtl8139_initialize(bd_t *bis)
 		dev->halt = rtl_disable;
 		dev->send = rtl_transmit;
 		dev->recv = rtl_poll;
-#ifdef CONFIG_MCAST_TFTP
-		dev->mcast = rtl_bcast_addr;
-#endif
 
 		eth_register (dev);
 
diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index 03a46da2f8..614097c6ce 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -78,52 +78,6 @@ static void tsec_configure_serdes(struct tsec_private *priv)
 			      0, TBI_CR, CONFIG_TSEC_TBICR_SETTINGS);
 }
 
-#ifdef CONFIG_MCAST_TFTP
-
-/* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
-
-/* Set the appropriate hash bit for the given addr */
-
-/*
- * The algorithm works like so:
- * 1) Take the Destination Address (ie the multicast address), and
- * do a CRC on it (little endian), and reverse the bits of the
- * result.
- * 2) Use the 8 most significant bits as a hash into a 256-entry
- * table.  The table is controlled through 8 32-bit registers:
- * gaddr0-7.  gaddr0's MSB is entry 0, and gaddr7's LSB is entry
- * 255.  This means that the 3 most significant bits in the
- * hash index which gaddr register to use, and the 5 other bits
- * indicate which bit (assuming an IBM numbering scheme, which
- * for PowerPC (tm) is usually the case) in the register holds
- * the entry.
- */
-#ifndef CONFIG_DM_ETH
-static int tsec_mcast_addr(struct eth_device *dev, const u8 *mcast_mac, u8 set)
-#else
-static int tsec_mcast_addr(struct udevice *dev, const u8 *mcast_mac, int set)
-#endif
-{
-	struct tsec_private *priv = (struct tsec_private *)dev->priv;
-	struct tsec __iomem *regs = priv->regs;
-	u32 result, value;
-	u8 whichbit, whichreg;
-
-	result = ether_crc(MAC_ADDR_LEN, mcast_mac);
-	whichbit = (result >> 24) & 0x1f; /* the 5 LSB = which bit to set */
-	whichreg = result >> 29; /* the 3 MSB = which reg to set it in */
-
-	value = BIT(31 - whichbit);
-
-	if (set)
-		setbits_be32(&regs->hash.gaddr0 + whichreg, value);
-	else
-		clrbits_be32(&regs->hash.gaddr0 + whichreg, value);
-
-	return 0;
-}
-#endif /* Multicast TFTP ? */
-
 /*
  * Initialized required registers to appropriate values, zeroing
  * those we don't care about (unless zero is bad, in which case,
@@ -720,9 +674,6 @@ static int tsec_initialize(bd_t *bis, struct tsec_info_struct *tsec_info)
 	dev->halt = tsec_halt;
 	dev->send = tsec_send;
 	dev->recv = tsec_recv;
-#ifdef CONFIG_MCAST_TFTP
-	dev->mcast = tsec_mcast_addr;
-#endif
 
 	/* Tell U-Boot to get the addr from the env */
 	for (i = 0; i < 6; i++)
@@ -862,9 +813,6 @@ static const struct eth_ops tsec_ops = {
 	.recv = tsec_recv,
 	.free_pkt = tsec_free_pkt,
 	.stop = tsec_halt,
-#ifdef CONFIG_MCAST_TFTP
-	.mcast = tsec_mcast_addr,
-#endif
 };
 
 static const struct udevice_id tsec_ids[] = {
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 90ef1f055f..41fe41e1a6 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -2607,9 +2607,6 @@ int usb_eth_initialize(bd_t *bi)
 	netdev->halt = usb_eth_halt;
 	netdev->priv = l_priv;
 
-#ifdef CONFIG_MCAST_TFTP
-  #error not supported
-#endif
 	eth_register(netdev);
 	return 0;
 }
diff --git a/include/net.h b/include/net.h
index 51c099dae2..072b658442 100644
--- a/include/net.h
+++ b/include/net.h
@@ -123,7 +123,6 @@ enum eth_recv_flags {
  *	     called when no error was returned from recv - optional
  * stop: Stop the hardware from looking for packets - may be called even if
  *	 state == PASSIVE
- * mcast: Join or leave a multicast group (for TFTP) - optional
  * write_hwaddr: Write a MAC address to the hardware (used to pass it to Linux
  *		 on some platforms like ARM). This function expects the
  *		 eth_pdata::enetaddr field to be populated. The method can
@@ -140,9 +139,6 @@ struct eth_ops {
 	int (*recv)(struct udevice *dev, int flags, uchar **packetp);
 	int (*free_pkt)(struct udevice *dev, uchar *packet, int length);
 	void (*stop)(struct udevice *dev);
-#ifdef CONFIG_MCAST_TFTP
-	int (*mcast)(struct udevice *dev, const u8 *enetaddr, int join);
-#endif
 	int (*write_hwaddr)(struct udevice *dev);
 	int (*read_rom_hwaddr)(struct udevice *dev);
 };
@@ -175,9 +171,6 @@ struct eth_device {
 	int (*send)(struct eth_device *, void *packet, int length);
 	int (*recv)(struct eth_device *);
 	void (*halt)(struct eth_device *);
-#ifdef CONFIG_MCAST_TFTP
-	int (*mcast)(struct eth_device *, const u8 *enetaddr, u8 set);
-#endif
 	int (*write_hwaddr)(struct eth_device *);
 	struct eth_device *next;
 	int index;
@@ -287,12 +280,6 @@ int eth_rx(void);			/* Check for received packets */
 void eth_halt(void);			/* stop SCC */
 const char *eth_get_name(void);		/* get name of current device */
 
-#ifdef CONFIG_MCAST_TFTP
-int eth_mcast_join(struct in_addr mcast_addr, int join);
-u32 ether_crc(size_t len, unsigned char const *p);
-#endif
-
-
 /**********************************************************************/
 /*
  *	Protocol headers.
@@ -578,10 +565,6 @@ extern struct in_addr	net_ntp_server;		/* the ip address to NTP */
 extern int net_ntp_time_offset;			/* offset time from UTC */
 #endif
 
-#if defined(CONFIG_MCAST_TFTP)
-extern struct in_addr net_mcast_addr;
-#endif
-
 /* Initialize the network adapter */
 void net_init(void);
 int net_loop(enum proto_t);
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 91d861be41..bafa093c37 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -476,10 +476,6 @@ static int eth_post_probe(struct udevice *dev)
 			ops->free_pkt += gd->reloc_off;
 		if (ops->stop)
 			ops->stop += gd->reloc_off;
-#ifdef CONFIG_MCAST_TFTP
-		if (ops->mcast)
-			ops->mcast += gd->reloc_off;
-#endif
 		if (ops->write_hwaddr)
 			ops->write_hwaddr += gd->reloc_off;
 		if (ops->read_rom_hwaddr)
diff --git a/net/eth_legacy.c b/net/eth_legacy.c
index 2a9caa3509..7e82422b29 100644
--- a/net/eth_legacy.c
+++ b/net/eth_legacy.c
@@ -291,52 +291,6 @@ int eth_initialize(void)
 	return num_devices;
 }
 
-#ifdef CONFIG_MCAST_TFTP
-/* Multicast.
- * mcast_addr: multicast ipaddr from which multicast Mac is made
- * join: 1=join, 0=leave.
- */
-int eth_mcast_join(struct in_addr mcast_ip, int join)
-{
-	u8 mcast_mac[ARP_HLEN];
-	if (!eth_current || !eth_current->mcast)
-		return -1;
-	mcast_mac[5] = htonl(mcast_ip.s_addr) & 0xff;
-	mcast_mac[4] = (htonl(mcast_ip.s_addr)>>8) & 0xff;
-	mcast_mac[3] = (htonl(mcast_ip.s_addr)>>16) & 0x7f;
-	mcast_mac[2] = 0x5e;
-	mcast_mac[1] = 0x0;
-	mcast_mac[0] = 0x1;
-	return eth_current->mcast(eth_current, mcast_mac, join);
-}
-
-/* the 'way' for ethernet-CRC-32. Spliced in from Linux lib/crc32.c
- * and this is the ethernet-crc method needed for TSEC -- and perhaps
- * some other adapter -- hash tables
- */
-#define CRCPOLY_LE 0xedb88320
-u32 ether_crc(size_t len, unsigned char const *p)
-{
-	int i;
-	u32 crc;
-	crc = ~0;
-	while (len--) {
-		crc ^= *p++;
-		for (i = 0; i < 8; i++)
-			crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
-	}
-	/* an reverse the bits, cuz of way they arrive -- last-first */
-	crc = (crc >> 16) | (crc << 16);
-	crc = (crc >> 8 & 0x00ff00ff) | (crc << 8 & 0xff00ff00);
-	crc = (crc >> 4 & 0x0f0f0f0f) | (crc << 4 & 0xf0f0f0f0);
-	crc = (crc >> 2 & 0x33333333) | (crc << 2 & 0xcccccccc);
-	crc = (crc >> 1 & 0x55555555) | (crc << 1 & 0xaaaaaaaa);
-	return crc;
-}
-
-#endif
-
-
 int eth_init(void)
 {
 	struct eth_device *old_current;
diff --git a/net/net.c b/net/net.c
index a5a216c3ee..72286e24f4 100644
--- a/net/net.c
+++ b/net/net.c
@@ -131,10 +131,6 @@ struct in_addr net_dns_server;
 struct in_addr net_dns_server2;
 #endif
 
-#ifdef CONFIG_MCAST_TFTP	/* Multicast TFTP */
-struct in_addr net_mcast_addr;
-#endif
-
 /** END OF BOOTP EXTENTIONS **/
 
 /* Our ethernet address */
@@ -1215,10 +1211,7 @@ void net_process_received_packet(uchar *in_packet, int len)
 		dst_ip = net_read_ip(&ip->ip_dst);
 		if (net_ip.s_addr && dst_ip.s_addr != net_ip.s_addr &&
 		    dst_ip.s_addr != 0xFFFFFFFF) {
-#ifdef CONFIG_MCAST_TFTP
-			if (net_mcast_addr != dst_ip)
-#endif
-				return;
+			return;
 		}
 		/* Read source IP address for later use */
 		src_ip = net_read_ip(&ip->ip_src);
diff --git a/net/tftp.c b/net/tftp.c
index 68ffd81414..563ce3a06f 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -134,36 +134,6 @@ 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;
 
-#ifdef CONFIG_MCAST_TFTP
-#include <malloc.h>
-#define MTFTP_BITMAPSIZE	0x1000
-static unsigned *tftp_mcast_bitmap;
-static int tftp_mcast_prev_hole;
-static int tftp_mcast_bitmap_size = MTFTP_BITMAPSIZE;
-static int tftp_mcast_disabled;
-static int tftp_mcast_master_client;
-static int tftp_mcast_active;
-static int tftp_mcast_port;
-/* can get 'last' block before done..*/
-static ulong tftp_mcast_ending_block;
-
-static void parse_multicast_oack(char *pkt, int len);
-
-static void mcast_cleanup(void)
-{
-	if (net_mcast_addr)
-		eth_mcast_join(net_mcast_addr, 0);
-	if (tftp_mcast_bitmap)
-		free(tftp_mcast_bitmap);
-	tftp_mcast_bitmap = NULL;
-	net_mcast_addr.s_addr = 0;
-	tftp_mcast_active = 0;
-	tftp_mcast_port = 0;
-	tftp_mcast_ending_block = -1;
-}
-
-#endif	/* CONFIG_MCAST_TFTP */
-
 static inline void store_block(int block, uchar *src, unsigned len)
 {
 	ulong offset = block * tftp_block_size + tftp_block_wrap_offset;
@@ -196,10 +166,6 @@ static inline void store_block(int block, uchar *src, unsigned len)
 		memcpy(ptr, src, len);
 		unmap_sysmem(ptr);
 	}
-#ifdef CONFIG_MCAST_TFTP
-	if (tftp_mcast_active)
-		ext2_set_bit(block, tftp_mcast_bitmap);
-#endif
 
 	if (net_boot_file_size < newsize)
 		net_boot_file_size = newsize;
@@ -275,9 +241,6 @@ static void show_block_marker(void)
 static void restart(const char *msg)
 {
 	printf("\n%s; starting again\n", msg);
-#ifdef CONFIG_MCAST_TFTP
-	mcast_cleanup();
-#endif
 	net_start_again();
 }
 
@@ -332,12 +295,6 @@ static void tftp_send(void)
 	int len = 0;
 	ushort *s;
 
-#ifdef CONFIG_MCAST_TFTP
-	/* Multicast TFTP.. non-MasterClients do not ACK data. */
-	if (tftp_mcast_active && tftp_state == STATE_DATA &&
-	    tftp_mcast_master_client == 0)
-		return;
-#endif
 	/*
 	 *	We will always be sending some sort of packet, so
 	 *	cobble together the packet headers now.
@@ -372,31 +329,10 @@ static void tftp_send(void)
 		/* try for more effic. blk size */
 		pkt += sprintf((char *)pkt, "blksize%c%d%c",
 				0, tftp_block_size_option, 0);
-#ifdef CONFIG_MCAST_TFTP
-		/* Check all preconditions before even trying the option */
-		if (!tftp_mcast_disabled) {
-			tftp_mcast_bitmap = malloc(tftp_mcast_bitmap_size);
-			if (tftp_mcast_bitmap && eth_get_dev()->mcast) {
-				free(tftp_mcast_bitmap);
-				tftp_mcast_bitmap = NULL;
-				pkt += sprintf((char *)pkt, "multicast%c%c",
-					0, 0);
-			}
-		}
-#endif /* CONFIG_MCAST_TFTP */
 		len = pkt - xp;
 		break;
 
 	case STATE_OACK:
-#ifdef CONFIG_MCAST_TFTP
-		/* My turn!  Start at where I need blocks I missed. */
-		if (tftp_mcast_active)
-			tftp_cur_block = ext2_find_next_zero_bit(
-				tftp_mcast_bitmap,
-				tftp_mcast_bitmap_size * 8, 0);
-		/* fall through */
-#endif
-
 	case STATE_RECV_WRQ:
 	case STATE_DATA:
 		xp = pkt;
@@ -465,11 +401,7 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 	int i;
 
 	if (dest != tftp_our_port) {
-#ifdef CONFIG_MCAST_TFTP
-		if (tftp_mcast_active &&
-		    (!tftp_mcast_port || dest != tftp_mcast_port))
-#endif
-			return;
+		return;
 	}
 	if (tftp_state != STATE_SEND_RRQ && src != tftp_remote_port &&
 	    tftp_state != STATE_RECV_WRQ && tftp_state != STATE_SEND_WRQ)
@@ -549,12 +481,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 			}
 #endif
 		}
-#ifdef CONFIG_MCAST_TFTP
-		parse_multicast_oack((char *)pkt, len - 1);
-		if ((tftp_mcast_active) && (!tftp_mcast_master_client))
-			tftp_state = STATE_DATA;	/* passive.. */
-		else
-#endif
 #ifdef CONFIG_CMD_TFTPPUT
 		if (tftp_put_active) {
 			/* Get ready to send the first block */
@@ -582,11 +508,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 			tftp_remote_port = src;
 			new_transfer();
 
-#ifdef CONFIG_MCAST_TFTP
-			if (tftp_mcast_active) { /* start!=1 common if mcast */
-				tftp_prev_block = tftp_cur_block - 1;
-			} else
-#endif
 			if (tftp_cur_block != 1) {	/* Assertion */
 				puts("\nTFTP error: ");
 				printf("First block is not block 1 (%ld)\n",
@@ -612,44 +533,8 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 		 *	Acknowledge the block just received, which will prompt
 		 *	the remote for the next one.
 		 */
-#ifdef CONFIG_MCAST_TFTP
-		/* if I am the MasterClient, actively calculate what my next
-		 * needed block is; else I'm passive; not ACKING
-		 */
-		if (tftp_mcast_active) {
-			if (len < tftp_block_size)  {
-				tftp_mcast_ending_block = tftp_cur_block;
-			} else if (tftp_mcast_master_client) {
-				tftp_mcast_prev_hole = ext2_find_next_zero_bit(
-					tftp_mcast_bitmap,
-					tftp_mcast_bitmap_size * 8,
-					tftp_mcast_prev_hole);
-				tftp_cur_block = tftp_mcast_prev_hole;
-				if (tftp_cur_block >
-				    ((tftp_mcast_bitmap_size * 8) - 1)) {
-					debug("tftpfile too big\n");
-					/* try to double it and retry */
-					tftp_mcast_bitmap_size <<= 1;
-					mcast_cleanup();
-					net_start_again();
-					return;
-				}
-				tftp_prev_block = tftp_cur_block;
-			}
-		}
-#endif
 		tftp_send();
 
-#ifdef CONFIG_MCAST_TFTP
-		if (tftp_mcast_active) {
-			if (tftp_mcast_master_client &&
-			    (tftp_cur_block >= tftp_mcast_ending_block)) {
-				puts("\nMulticast tftp done\n");
-				mcast_cleanup();
-				net_set_state(NETLOOP_SUCCESS);
-			}
-		} else
-#endif
 		if (len < tftp_block_size)
 			tftp_complete();
 		break;
@@ -672,9 +557,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 		case TFTP_ERR_FILE_ALREADY_EXISTS:
 		default:
 			puts("Starting again\n\n");
-#ifdef CONFIG_MCAST_TFTP
-			mcast_cleanup();
-#endif
 			net_start_again();
 			break;
 		}
@@ -826,9 +708,6 @@ void tftp_start(enum proto_t protocol)
 	memset(net_server_ethaddr, 0, 6);
 	/* Revert tftp_block_size to dflt */
 	tftp_block_size = TFTP_BLOCK_SIZE;
-#ifdef CONFIG_MCAST_TFTP
-	mcast_cleanup();
-#endif
 #ifdef CONFIG_TFTP_TSIZE
 	tftp_tsize = 0;
 	tftp_tsize_num_hash = 0;
@@ -870,103 +749,3 @@ void tftp_start_server(void)
 	memset(net_server_ethaddr, 0, 6);
 }
 #endif /* CONFIG_CMD_TFTPSRV */
-
-#ifdef CONFIG_MCAST_TFTP
-/*
- * Credits: atftp project.
- */
-
-/*
- * Pick up BcastAddr, Port, and whether I am [now] the master-client.
- * Frame:
- *    +-------+-----------+---+-------~~-------+---+
- *    |  opc  | multicast | 0 | addr, port, mc | 0 |
- *    +-------+-----------+---+-------~~-------+---+
- * The multicast addr/port becomes what I listen to, and if 'mc' is '1' then
- * I am the new master-client so must send ACKs to DataBlocks.  If I am not
- * master-client, I'm a passive client, gathering what DataBlocks I may and
- * making note of which ones I got in my bitmask.
- * In theory, I never go from master->passive..
- * .. this comes in with pkt already pointing just past opc
- */
-static void parse_multicast_oack(char *pkt, int len)
-{
-	int i;
-	struct in_addr addr;
-	char *mc_adr;
-	char *port;
-	char *mc;
-
-	mc_adr = NULL;
-	port = NULL;
-	mc = NULL;
-	/* march along looking for 'multicast\0', which has to start at least
-	 * 14 bytes back from the end.
-	 */
-	for (i = 0; i < len - 14; i++)
-		if (strcmp(pkt + i, "multicast") == 0)
-			break;
-	if (i >= (len - 14)) /* non-Multicast OACK, ign. */
-		return;
-
-	i += 10; /* strlen multicast */
-	mc_adr = pkt + i;
-	for (; i < len; i++) {
-		if (*(pkt + i) == ',') {
-			*(pkt + i) = '\0';
-			if (port) {
-				mc = pkt + i + 1;
-				break;
-			} else {
-				port = pkt + i + 1;
-			}
-		}
-	}
-	if (!port || !mc_adr || !mc)
-		return;
-	if (tftp_mcast_active && tftp_mcast_master_client) {
-		printf("I got a OACK as master Client, WRONG!\n");
-		return;
-	}
-	/* ..I now accept packets destined for this MCAST addr, port */
-	if (!tftp_mcast_active) {
-		if (tftp_mcast_bitmap) {
-			printf("Internal failure! no mcast.\n");
-			free(tftp_mcast_bitmap);
-			tftp_mcast_bitmap = NULL;
-			tftp_mcast_disabled = 1;
-			return;
-		}
-		/* I malloc instead of pre-declare; so that if the file ends
-		 * up being too big for this bitmap I can retry
-		 */
-		tftp_mcast_bitmap = malloc(tftp_mcast_bitmap_size);
-		if (!tftp_mcast_bitmap) {
-			printf("No bitmap, no multicast. Sorry.\n");
-			tftp_mcast_disabled = 1;
-			return;
-		}
-		memset(tftp_mcast_bitmap, 0, tftp_mcast_bitmap_size);
-		tftp_mcast_prev_hole = 0;
-		tftp_mcast_active = 1;
-	}
-	addr = string_to_ip(mc_adr);
-	if (net_mcast_addr.s_addr != addr.s_addr) {
-		if (net_mcast_addr.s_addr)
-			eth_mcast_join(net_mcast_addr, 0);
-		net_mcast_addr = addr;
-		if (eth_mcast_join(net_mcast_addr, 1)) {
-			printf("Fail to set mcast, revert to TFTP\n");
-			tftp_mcast_disabled = 1;
-			mcast_cleanup();
-			net_start_again();
-		}
-	}
-	tftp_mcast_master_client = simple_strtoul((char *)mc, NULL, 10);
-	tftp_mcast_port = (unsigned short)simple_strtoul(port, NULL, 10);
-	printf("Multicast: %s:%d [%d]\n", mc_adr, tftp_mcast_port,
-	       tftp_mcast_master_client);
-	return;
-}
-
-#endif /* Multicast TFTP */
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index 0627024e71..b2691206fb 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -1210,7 +1210,6 @@ CONFIG_MAX_FPGA_DEVICES
 CONFIG_MAX_MEM_MAPPED
 CONFIG_MAX_PKT
 CONFIG_MAX_RAM_BANK_SIZE
-CONFIG_MCAST_TFTP
 CONFIG_MCF5249
 CONFIG_MCF5253
 CONFIG_MCFFEC
-- 
2.17.1

[U-Boot] [PATCH v3 8/8] tftp: prevent overwriting reserved memory

[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.

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

[U-Boot] [PATCH v3 7/8] net: remove CONFIG_MCAST_TFTP

[Joe Hershberger]

Hi Simon,

Thanks for tackling this.

On Sat, Nov 17, 2018 at 6:31 AM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> This option seems unused as no mainline board enables it and it does
> not compile since some years.
>
> Additionally, it has a potential buffer underrun issue (reported as
> a side node in CVE-2018-18439).
>
> Instead of trying to fix a thing noone seems to use, let's rather
> drop dead code.
>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
>
> Changes in v2:
> - this patch is new in v2
>
>  README                       |   9 --
>  drivers/net/rtl8139.c        |   9 --
>  drivers/net/tsec.c           |  52 --------
>  drivers/usb/gadget/ether.c   |   3 -
>  include/net.h                |  17 ---
>  net/eth-uclass.c             |   4 -
>  net/eth_legacy.c             |  46 --------
>  net/net.c                    |   9 +-
>  net/tftp.c                   | 223 +----------------------------------

I think the TFTP stuff can be removed, but multicast in general
probably not, since I believe IPv6 support requires it. I guess we
could put it back at that time, but I'm a bit concerned that we will
diverge and make that work harder. Maybe it's feasible to simply stop
guarding multicast support in the core and drivers with the TFTP
config and enable it at all times. The code size on that side is
minimal.

Thanks,
-Joe

>  scripts/config_whitelist.txt |   1 -
>  10 files changed, 2 insertions(+), 371 deletions(-)
>
> diff --git a/README b/README
> index a46c7c63a4..97a3e9a84b 100644
> --- a/README
> +++ b/README
> @@ -1429,15 +1429,6 @@ The following options need to be configured:
>                 forwarded through a router.
>                 (Environment variable "netmask")
>
> -- Multicast TFTP Mode:
> -               CONFIG_MCAST_TFTP
> -
> -               Defines whether you want to support multicast TFTP as per
> -               rfc-2090; for example to work with atftp.  Lets lots of targets
> -               tftp down the same boot image concurrently.  Note: the Ethernet
> -               driver in use must provide a function: mcast() to join/leave a
> -               multicast group.
> -
>  - BOOTP Recovery Mode:
>                 CONFIG_BOOTP_RANDOM_DELAY
>
> diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c
> index 590f8ce154..79acd64bc0 100644
> --- a/drivers/net/rtl8139.c
> +++ b/drivers/net/rtl8139.c
> @@ -183,12 +183,6 @@ static void rtl_reset(struct eth_device *dev);
>  static int rtl_transmit(struct eth_device *dev, void *packet, int length);
>  static int rtl_poll(struct eth_device *dev);
>  static void rtl_disable(struct eth_device *dev);
> -#ifdef CONFIG_MCAST_TFTP/*  This driver already accepts all b/mcast */
> -static int rtl_bcast_addr(struct eth_device *dev, const u8 *bcast_mac, u8 set)
> -{
> -       return (0);
> -}
> -#endif
>
>  static struct pci_device_id supported[] = {
>         {PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8139},
> @@ -229,9 +223,6 @@ int rtl8139_initialize(bd_t *bis)
>                 dev->halt = rtl_disable;
>                 dev->send = rtl_transmit;
>                 dev->recv = rtl_poll;
> -#ifdef CONFIG_MCAST_TFTP
> -               dev->mcast = rtl_bcast_addr;
> -#endif
>
>                 eth_register (dev);
>
> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> index 03a46da2f8..614097c6ce 100644
> --- a/drivers/net/tsec.c
> +++ b/drivers/net/tsec.c
> @@ -78,52 +78,6 @@ static void tsec_configure_serdes(struct tsec_private *priv)
>                               0, TBI_CR, CONFIG_TSEC_TBICR_SETTINGS);
>  }
>
> -#ifdef CONFIG_MCAST_TFTP
> -
> -/* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
> -
> -/* Set the appropriate hash bit for the given addr */
> -
> -/*
> - * The algorithm works like so:
> - * 1) Take the Destination Address (ie the multicast address), and
> - * do a CRC on it (little endian), and reverse the bits of the
> - * result.
> - * 2) Use the 8 most significant bits as a hash into a 256-entry
> - * table.  The table is controlled through 8 32-bit registers:
> - * gaddr0-7.  gaddr0's MSB is entry 0, and gaddr7's LSB is entry
> - * 255.  This means that the 3 most significant bits in the
> - * hash index which gaddr register to use, and the 5 other bits
> - * indicate which bit (assuming an IBM numbering scheme, which
> - * for PowerPC (tm) is usually the case) in the register holds
> - * the entry.
> - */
> -#ifndef CONFIG_DM_ETH
> -static int tsec_mcast_addr(struct eth_device *dev, const u8 *mcast_mac, u8 set)
> -#else
> -static int tsec_mcast_addr(struct udevice *dev, const u8 *mcast_mac, int set)
> -#endif
> -{
> -       struct tsec_private *priv = (struct tsec_private *)dev->priv;
> -       struct tsec __iomem *regs = priv->regs;
> -       u32 result, value;
> -       u8 whichbit, whichreg;
> -
> -       result = ether_crc(MAC_ADDR_LEN, mcast_mac);
> -       whichbit = (result >> 24) & 0x1f; /* the 5 LSB = which bit to set */
> -       whichreg = result >> 29; /* the 3 MSB = which reg to set it in */
> -
> -       value = BIT(31 - whichbit);
> -
> -       if (set)
> -               setbits_be32(&regs->hash.gaddr0 + whichreg, value);
> -       else
> -               clrbits_be32(&regs->hash.gaddr0 + whichreg, value);
> -
> -       return 0;
> -}
> -#endif /* Multicast TFTP ? */
> -
>  /*
>   * Initialized required registers to appropriate values, zeroing
>   * those we don't care about (unless zero is bad, in which case,
> @@ -720,9 +674,6 @@ static int tsec_initialize(bd_t *bis, struct tsec_info_struct *tsec_info)
>         dev->halt = tsec_halt;
>         dev->send = tsec_send;
>         dev->recv = tsec_recv;
> -#ifdef CONFIG_MCAST_TFTP
> -       dev->mcast = tsec_mcast_addr;
> -#endif
>
>         /* Tell U-Boot to get the addr from the env */
>         for (i = 0; i < 6; i++)
> @@ -862,9 +813,6 @@ static const struct eth_ops tsec_ops = {
>         .recv = tsec_recv,
>         .free_pkt = tsec_free_pkt,
>         .stop = tsec_halt,
> -#ifdef CONFIG_MCAST_TFTP
> -       .mcast = tsec_mcast_addr,
> -#endif
>  };
>
>  static const struct udevice_id tsec_ids[] = {
> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
> index 90ef1f055f..41fe41e1a6 100644
> --- a/drivers/usb/gadget/ether.c
> +++ b/drivers/usb/gadget/ether.c
> @@ -2607,9 +2607,6 @@ int usb_eth_initialize(bd_t *bi)
>         netdev->halt = usb_eth_halt;
>         netdev->priv = l_priv;
>
> -#ifdef CONFIG_MCAST_TFTP
> -  #error not supported
> -#endif
>         eth_register(netdev);
>         return 0;
>  }
> diff --git a/include/net.h b/include/net.h
> index 51c099dae2..072b658442 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -123,7 +123,6 @@ enum eth_recv_flags {
>   *          called when no error was returned from recv - optional
>   * stop: Stop the hardware from looking for packets - may be called even if
>   *      state == PASSIVE
> - * mcast: Join or leave a multicast group (for TFTP) - optional
>   * write_hwaddr: Write a MAC address to the hardware (used to pass it to Linux
>   *              on some platforms like ARM). This function expects the
>   *              eth_pdata::enetaddr field to be populated. The method can
> @@ -140,9 +139,6 @@ struct eth_ops {
>         int (*recv)(struct udevice *dev, int flags, uchar **packetp);
>         int (*free_pkt)(struct udevice *dev, uchar *packet, int length);
>         void (*stop)(struct udevice *dev);
> -#ifdef CONFIG_MCAST_TFTP
> -       int (*mcast)(struct udevice *dev, const u8 *enetaddr, int join);
> -#endif
>         int (*write_hwaddr)(struct udevice *dev);
>         int (*read_rom_hwaddr)(struct udevice *dev);
>  };
> @@ -175,9 +171,6 @@ struct eth_device {
>         int (*send)(struct eth_device *, void *packet, int length);
>         int (*recv)(struct eth_device *);
>         void (*halt)(struct eth_device *);
> -#ifdef CONFIG_MCAST_TFTP
> -       int (*mcast)(struct eth_device *, const u8 *enetaddr, u8 set);
> -#endif
>         int (*write_hwaddr)(struct eth_device *);
>         struct eth_device *next;
>         int index;
> @@ -287,12 +280,6 @@ int eth_rx(void);                  /* Check for received packets */
>  void eth_halt(void);                   /* stop SCC */
>  const char *eth_get_name(void);                /* get name of current device */
>
> -#ifdef CONFIG_MCAST_TFTP
> -int eth_mcast_join(struct in_addr mcast_addr, int join);
> -u32 ether_crc(size_t len, unsigned char const *p);
> -#endif
> -
> -
>  /**********************************************************************/
>  /*
>   *     Protocol headers.
> @@ -578,10 +565,6 @@ extern struct in_addr      net_ntp_server;         /* the ip address to NTP */
>  extern int net_ntp_time_offset;                        /* offset time from UTC */
>  #endif
>
> -#if defined(CONFIG_MCAST_TFTP)
> -extern struct in_addr net_mcast_addr;
> -#endif
> -
>  /* Initialize the network adapter */
>  void net_init(void);
>  int net_loop(enum proto_t);
> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> index 91d861be41..bafa093c37 100644
> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -476,10 +476,6 @@ static int eth_post_probe(struct udevice *dev)
>                         ops->free_pkt += gd->reloc_off;
>                 if (ops->stop)
>                         ops->stop += gd->reloc_off;
> -#ifdef CONFIG_MCAST_TFTP
> -               if (ops->mcast)
> -                       ops->mcast += gd->reloc_off;
> -#endif
>                 if (ops->write_hwaddr)
>                         ops->write_hwaddr += gd->reloc_off;
>                 if (ops->read_rom_hwaddr)
> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> index 2a9caa3509..7e82422b29 100644
> --- a/net/eth_legacy.c
> +++ b/net/eth_legacy.c
> @@ -291,52 +291,6 @@ int eth_initialize(void)
>         return num_devices;
>  }
>
> -#ifdef CONFIG_MCAST_TFTP
> -/* Multicast.
> - * mcast_addr: multicast ipaddr from which multicast Mac is made
> - * join: 1=join, 0=leave.
> - */
> -int eth_mcast_join(struct in_addr mcast_ip, int join)
> -{
> -       u8 mcast_mac[ARP_HLEN];
> -       if (!eth_current || !eth_current->mcast)
> -               return -1;
> -       mcast_mac[5] = htonl(mcast_ip.s_addr) & 0xff;
> -       mcast_mac[4] = (htonl(mcast_ip.s_addr)>>8) & 0xff;
> -       mcast_mac[3] = (htonl(mcast_ip.s_addr)>>16) & 0x7f;
> -       mcast_mac[2] = 0x5e;
> -       mcast_mac[1] = 0x0;
> -       mcast_mac[0] = 0x1;
> -       return eth_current->mcast(eth_current, mcast_mac, join);
> -}
> -
> -/* the 'way' for ethernet-CRC-32. Spliced in from Linux lib/crc32.c
> - * and this is the ethernet-crc method needed for TSEC -- and perhaps
> - * some other adapter -- hash tables
> - */
> -#define CRCPOLY_LE 0xedb88320
> -u32 ether_crc(size_t len, unsigned char const *p)
> -{
> -       int i;
> -       u32 crc;
> -       crc = ~0;
> -       while (len--) {
> -               crc ^= *p++;
> -               for (i = 0; i < 8; i++)
> -                       crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
> -       }
> -       /* an reverse the bits, cuz of way they arrive -- last-first */
> -       crc = (crc >> 16) | (crc << 16);
> -       crc = (crc >> 8 & 0x00ff00ff) | (crc << 8 & 0xff00ff00);
> -       crc = (crc >> 4 & 0x0f0f0f0f) | (crc << 4 & 0xf0f0f0f0);
> -       crc = (crc >> 2 & 0x33333333) | (crc << 2 & 0xcccccccc);
> -       crc = (crc >> 1 & 0x55555555) | (crc << 1 & 0xaaaaaaaa);
> -       return crc;
> -}
> -
> -#endif
> -
> -
>  int eth_init(void)
>  {
>         struct eth_device *old_current;
> diff --git a/net/net.c b/net/net.c
> index a5a216c3ee..72286e24f4 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -131,10 +131,6 @@ struct in_addr net_dns_server;
>  struct in_addr net_dns_server2;
>  #endif
>
> -#ifdef CONFIG_MCAST_TFTP       /* Multicast TFTP */
> -struct in_addr net_mcast_addr;
> -#endif
> -
>  /** END OF BOOTP EXTENTIONS **/
>
>  /* Our ethernet address */
> @@ -1215,10 +1211,7 @@ void net_process_received_packet(uchar *in_packet, int len)
>                 dst_ip = net_read_ip(&ip->ip_dst);
>                 if (net_ip.s_addr && dst_ip.s_addr != net_ip.s_addr &&
>                     dst_ip.s_addr != 0xFFFFFFFF) {
> -#ifdef CONFIG_MCAST_TFTP
> -                       if (net_mcast_addr != dst_ip)
> -#endif
> -                               return;
> +                       return;
>                 }
>                 /* Read source IP address for later use */
>                 src_ip = net_read_ip(&ip->ip_src);
> diff --git a/net/tftp.c b/net/tftp.c
> index 68ffd81414..563ce3a06f 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -134,36 +134,6 @@ 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;
>
> -#ifdef CONFIG_MCAST_TFTP
> -#include <malloc.h>
> -#define MTFTP_BITMAPSIZE       0x1000
> -static unsigned *tftp_mcast_bitmap;
> -static int tftp_mcast_prev_hole;
> -static int tftp_mcast_bitmap_size = MTFTP_BITMAPSIZE;
> -static int tftp_mcast_disabled;
> -static int tftp_mcast_master_client;
> -static int tftp_mcast_active;
> -static int tftp_mcast_port;
> -/* can get 'last' block before done..*/
> -static ulong tftp_mcast_ending_block;
> -
> -static void parse_multicast_oack(char *pkt, int len);
> -
> -static void mcast_cleanup(void)
> -{
> -       if (net_mcast_addr)
> -               eth_mcast_join(net_mcast_addr, 0);
> -       if (tftp_mcast_bitmap)
> -               free(tftp_mcast_bitmap);
> -       tftp_mcast_bitmap = NULL;
> -       net_mcast_addr.s_addr = 0;
> -       tftp_mcast_active = 0;
> -       tftp_mcast_port = 0;
> -       tftp_mcast_ending_block = -1;
> -}
> -
> -#endif /* CONFIG_MCAST_TFTP */
> -
>  static inline void store_block(int block, uchar *src, unsigned len)
>  {
>         ulong offset = block * tftp_block_size + tftp_block_wrap_offset;
> @@ -196,10 +166,6 @@ static inline void store_block(int block, uchar *src, unsigned len)
>                 memcpy(ptr, src, len);
>                 unmap_sysmem(ptr);
>         }
> -#ifdef CONFIG_MCAST_TFTP
> -       if (tftp_mcast_active)
> -               ext2_set_bit(block, tftp_mcast_bitmap);
> -#endif
>
>         if (net_boot_file_size < newsize)
>                 net_boot_file_size = newsize;
> @@ -275,9 +241,6 @@ static void show_block_marker(void)
>  static void restart(const char *msg)
>  {
>         printf("\n%s; starting again\n", msg);
> -#ifdef CONFIG_MCAST_TFTP
> -       mcast_cleanup();
> -#endif
>         net_start_again();
>  }
>
> @@ -332,12 +295,6 @@ static void tftp_send(void)
>         int len = 0;
>         ushort *s;
>
> -#ifdef CONFIG_MCAST_TFTP
> -       /* Multicast TFTP.. non-MasterClients do not ACK data. */
> -       if (tftp_mcast_active && tftp_state == STATE_DATA &&
> -           tftp_mcast_master_client == 0)
> -               return;
> -#endif
>         /*
>          *      We will always be sending some sort of packet, so
>          *      cobble together the packet headers now.
> @@ -372,31 +329,10 @@ static void tftp_send(void)
>                 /* try for more effic. blk size */
>                 pkt += sprintf((char *)pkt, "blksize%c%d%c",
>                                 0, tftp_block_size_option, 0);
> -#ifdef CONFIG_MCAST_TFTP
> -               /* Check all preconditions before even trying the option */
> -               if (!tftp_mcast_disabled) {
> -                       tftp_mcast_bitmap = malloc(tftp_mcast_bitmap_size);
> -                       if (tftp_mcast_bitmap && eth_get_dev()->mcast) {
> -                               free(tftp_mcast_bitmap);
> -                               tftp_mcast_bitmap = NULL;
> -                               pkt += sprintf((char *)pkt, "multicast%c%c",
> -                                       0, 0);
> -                       }
> -               }
> -#endif /* CONFIG_MCAST_TFTP */
>                 len = pkt - xp;
>                 break;
>
>         case STATE_OACK:
> -#ifdef CONFIG_MCAST_TFTP
> -               /* My turn!  Start at where I need blocks I missed. */
> -               if (tftp_mcast_active)
> -                       tftp_cur_block = ext2_find_next_zero_bit(
> -                               tftp_mcast_bitmap,
> -                               tftp_mcast_bitmap_size * 8, 0);
> -               /* fall through */
> -#endif
> -
>         case STATE_RECV_WRQ:
>         case STATE_DATA:
>                 xp = pkt;
> @@ -465,11 +401,7 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>         int i;
>
>         if (dest != tftp_our_port) {
> -#ifdef CONFIG_MCAST_TFTP
> -               if (tftp_mcast_active &&
> -                   (!tftp_mcast_port || dest != tftp_mcast_port))
> -#endif
> -                       return;
> +               return;
>         }
>         if (tftp_state != STATE_SEND_RRQ && src != tftp_remote_port &&
>             tftp_state != STATE_RECV_WRQ && tftp_state != STATE_SEND_WRQ)
> @@ -549,12 +481,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>                         }
>  #endif
>                 }
> -#ifdef CONFIG_MCAST_TFTP
> -               parse_multicast_oack((char *)pkt, len - 1);
> -               if ((tftp_mcast_active) && (!tftp_mcast_master_client))
> -                       tftp_state = STATE_DATA;        /* passive.. */
> -               else
> -#endif
>  #ifdef CONFIG_CMD_TFTPPUT
>                 if (tftp_put_active) {
>                         /* Get ready to send the first block */
> @@ -582,11 +508,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>                         tftp_remote_port = src;
>                         new_transfer();
>
> -#ifdef CONFIG_MCAST_TFTP
> -                       if (tftp_mcast_active) { /* start!=1 common if mcast */
> -                               tftp_prev_block = tftp_cur_block - 1;
> -                       } else
> -#endif
>                         if (tftp_cur_block != 1) {      /* Assertion */
>                                 puts("\nTFTP error: ");
>                                 printf("First block is not block 1 (%ld)\n",
> @@ -612,44 +533,8 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>                  *      Acknowledge the block just received, which will prompt
>                  *      the remote for the next one.
>                  */
> -#ifdef CONFIG_MCAST_TFTP
> -               /* if I am the MasterClient, actively calculate what my next
> -                * needed block is; else I'm passive; not ACKING
> -                */
> -               if (tftp_mcast_active) {
> -                       if (len < tftp_block_size)  {
> -                               tftp_mcast_ending_block = tftp_cur_block;
> -                       } else if (tftp_mcast_master_client) {
> -                               tftp_mcast_prev_hole = ext2_find_next_zero_bit(
> -                                       tftp_mcast_bitmap,
> -                                       tftp_mcast_bitmap_size * 8,
> -                                       tftp_mcast_prev_hole);
> -                               tftp_cur_block = tftp_mcast_prev_hole;
> -                               if (tftp_cur_block >
> -                                   ((tftp_mcast_bitmap_size * 8) - 1)) {
> -                                       debug("tftpfile too big\n");
> -                                       /* try to double it and retry */
> -                                       tftp_mcast_bitmap_size <<= 1;
> -                                       mcast_cleanup();
> -                                       net_start_again();
> -                                       return;
> -                               }
> -                               tftp_prev_block = tftp_cur_block;
> -                       }
> -               }
> -#endif
>                 tftp_send();
>
> -#ifdef CONFIG_MCAST_TFTP
> -               if (tftp_mcast_active) {
> -                       if (tftp_mcast_master_client &&
> -                           (tftp_cur_block >= tftp_mcast_ending_block)) {
> -                               puts("\nMulticast tftp done\n");
> -                               mcast_cleanup();
> -                               net_set_state(NETLOOP_SUCCESS);
> -                       }
> -               } else
> -#endif
>                 if (len < tftp_block_size)
>                         tftp_complete();
>                 break;
> @@ -672,9 +557,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>                 case TFTP_ERR_FILE_ALREADY_EXISTS:
>                 default:
>                         puts("Starting again\n\n");
> -#ifdef CONFIG_MCAST_TFTP
> -                       mcast_cleanup();
> -#endif
>                         net_start_again();
>                         break;
>                 }
> @@ -826,9 +708,6 @@ void tftp_start(enum proto_t protocol)
>         memset(net_server_ethaddr, 0, 6);
>         /* Revert tftp_block_size to dflt */
>         tftp_block_size = TFTP_BLOCK_SIZE;
> -#ifdef CONFIG_MCAST_TFTP
> -       mcast_cleanup();
> -#endif
>  #ifdef CONFIG_TFTP_TSIZE
>         tftp_tsize = 0;
>         tftp_tsize_num_hash = 0;
> @@ -870,103 +749,3 @@ void tftp_start_server(void)
>         memset(net_server_ethaddr, 0, 6);
>  }
>  #endif /* CONFIG_CMD_TFTPSRV */
> -
> -#ifdef CONFIG_MCAST_TFTP
> -/*
> - * Credits: atftp project.
> - */
> -
> -/*
> - * Pick up BcastAddr, Port, and whether I am [now] the master-client.
> - * Frame:
> - *    +-------+-----------+---+-------~~-------+---+
> - *    |  opc  | multicast | 0 | addr, port, mc | 0 |
> - *    +-------+-----------+---+-------~~-------+---+
> - * The multicast addr/port becomes what I listen to, and if 'mc' is '1' then
> - * I am the new master-client so must send ACKs to DataBlocks.  If I am not
> - * master-client, I'm a passive client, gathering what DataBlocks I may and
> - * making note of which ones I got in my bitmask.
> - * In theory, I never go from master->passive..
> - * .. this comes in with pkt already pointing just past opc
> - */
> -static void parse_multicast_oack(char *pkt, int len)
> -{
> -       int i;
> -       struct in_addr addr;
> -       char *mc_adr;
> -       char *port;
> -       char *mc;
> -
> -       mc_adr = NULL;
> -       port = NULL;
> -       mc = NULL;
> -       /* march along looking for 'multicast\0', which has to start at least
> -        * 14 bytes back from the end.
> -        */
> -       for (i = 0; i < len - 14; i++)
> -               if (strcmp(pkt + i, "multicast") == 0)
> -                       break;
> -       if (i >= (len - 14)) /* non-Multicast OACK, ign. */
> -               return;
> -
> -       i += 10; /* strlen multicast */
> -       mc_adr = pkt + i;
> -       for (; i < len; i++) {
> -               if (*(pkt + i) == ',') {
> -                       *(pkt + i) = '\0';
> -                       if (port) {
> -                               mc = pkt + i + 1;
> -                               break;
> -                       } else {
> -                               port = pkt + i + 1;
> -                       }
> -               }
> -       }
> -       if (!port || !mc_adr || !mc)
> -               return;
> -       if (tftp_mcast_active && tftp_mcast_master_client) {
> -               printf("I got a OACK as master Client, WRONG!\n");
> -               return;
> -       }
> -       /* ..I now accept packets destined for this MCAST addr, port */
> -       if (!tftp_mcast_active) {
> -               if (tftp_mcast_bitmap) {
> -                       printf("Internal failure! no mcast.\n");
> -                       free(tftp_mcast_bitmap);
> -                       tftp_mcast_bitmap = NULL;
> -                       tftp_mcast_disabled = 1;
> -                       return;
> -               }
> -               /* I malloc instead of pre-declare; so that if the file ends
> -                * up being too big for this bitmap I can retry
> -                */
> -               tftp_mcast_bitmap = malloc(tftp_mcast_bitmap_size);
> -               if (!tftp_mcast_bitmap) {
> -                       printf("No bitmap, no multicast. Sorry.\n");
> -                       tftp_mcast_disabled = 1;
> -                       return;
> -               }
> -               memset(tftp_mcast_bitmap, 0, tftp_mcast_bitmap_size);
> -               tftp_mcast_prev_hole = 0;
> -               tftp_mcast_active = 1;
> -       }
> -       addr = string_to_ip(mc_adr);
> -       if (net_mcast_addr.s_addr != addr.s_addr) {
> -               if (net_mcast_addr.s_addr)
> -                       eth_mcast_join(net_mcast_addr, 0);
> -               net_mcast_addr = addr;
> -               if (eth_mcast_join(net_mcast_addr, 1)) {
> -                       printf("Fail to set mcast, revert to TFTP\n");
> -                       tftp_mcast_disabled = 1;
> -                       mcast_cleanup();
> -                       net_start_again();
> -               }
> -       }
> -       tftp_mcast_master_client = simple_strtoul((char *)mc, NULL, 10);
> -       tftp_mcast_port = (unsigned short)simple_strtoul(port, NULL, 10);
> -       printf("Multicast: %s:%d [%d]\n", mc_adr, tftp_mcast_port,
> -              tftp_mcast_master_client);
> -       return;
> -}
> -
> -#endif /* Multicast TFTP */
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index 0627024e71..b2691206fb 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -1210,7 +1210,6 @@ CONFIG_MAX_FPGA_DEVICES
>  CONFIG_MAX_MEM_MAPPED
>  CONFIG_MAX_PKT
>  CONFIG_MAX_RAM_BANK_SIZE
> -CONFIG_MCAST_TFTP
>  CONFIG_MCF5249
>  CONFIG_MCF5253
>  CONFIG_MCFFEC
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 7/8] net: remove CONFIG_MCAST_TFTP

[Simon Goldschmidt]

On 17.11.2018 17:05, Joe Hershberger wrote:
> Hi Simon,
>
> Thanks for tackling this.
>
> On Sat, Nov 17, 2018 at 6:31 AM Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
>> This option seems unused as no mainline board enables it and it does
>> not compile since some years.
>>
>> Additionally, it has a potential buffer underrun issue (reported as
>> a side node in CVE-2018-18439).
>>
>> Instead of trying to fix a thing noone seems to use, let's rather
>> drop dead code.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> ---
>>
>> Changes in v2:
>> - this patch is new in v2
>>
>>   README                       |   9 --
>>   drivers/net/rtl8139.c        |   9 --
>>   drivers/net/tsec.c           |  52 --------
>>   drivers/usb/gadget/ether.c   |   3 -
>>   include/net.h                |  17 ---
>>   net/eth-uclass.c             |   4 -
>>   net/eth_legacy.c             |  46 --------
>>   net/net.c                    |   9 +-
>>   net/tftp.c                   | 223 +----------------------------------
> I think the TFTP stuff can be removed, but multicast in general
> probably not, since I believe IPv6 support requires it. I guess we
> could put it back at that time, but I'm a bit concerned that we will
> diverge and make that work harder. Maybe it's feasible to simply stop
> guarding multicast support in the core and drivers with the TFTP
> config and enable it at all times. The code size on that side is
> minimal.

Well, it was a bit hard for me to see what's general multicast and 
what's tftp only, given that it's all using one tftp-related guard.
Maybe it would be better to drop this patch from the series to discuss 
this in a separate thread?

Simon

>
> Thanks,
> -Joe
>
>>   scripts/config_whitelist.txt |   1 -
>>   10 files changed, 2 insertions(+), 371 deletions(-)
>>
>> diff --git a/README b/README
>> index a46c7c63a4..97a3e9a84b 100644
>> --- a/README
>> +++ b/README
>> @@ -1429,15 +1429,6 @@ The following options need to be configured:
>>                  forwarded through a router.
>>                  (Environment variable "netmask")
>>
>> -- Multicast TFTP Mode:
>> -               CONFIG_MCAST_TFTP
>> -
>> -               Defines whether you want to support multicast TFTP as per
>> -               rfc-2090; for example to work with atftp.  Lets lots of targets
>> -               tftp down the same boot image concurrently.  Note: the Ethernet
>> -               driver in use must provide a function: mcast() to join/leave a
>> -               multicast group.
>> -
>>   - BOOTP Recovery Mode:
>>                  CONFIG_BOOTP_RANDOM_DELAY
>>
>> diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c
>> index 590f8ce154..79acd64bc0 100644
>> --- a/drivers/net/rtl8139.c
>> +++ b/drivers/net/rtl8139.c
>> @@ -183,12 +183,6 @@ static void rtl_reset(struct eth_device *dev);
>>   static int rtl_transmit(struct eth_device *dev, void *packet, int length);
>>   static int rtl_poll(struct eth_device *dev);
>>   static void rtl_disable(struct eth_device *dev);
>> -#ifdef CONFIG_MCAST_TFTP/*  This driver already accepts all b/mcast */
>> -static int rtl_bcast_addr(struct eth_device *dev, const u8 *bcast_mac, u8 set)
>> -{
>> -       return (0);
>> -}
>> -#endif
>>
>>   static struct pci_device_id supported[] = {
>>          {PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8139},
>> @@ -229,9 +223,6 @@ int rtl8139_initialize(bd_t *bis)
>>                  dev->halt = rtl_disable;
>>                  dev->send = rtl_transmit;
>>                  dev->recv = rtl_poll;
>> -#ifdef CONFIG_MCAST_TFTP
>> -               dev->mcast = rtl_bcast_addr;
>> -#endif
>>
>>                  eth_register (dev);
>>
>> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
>> index 03a46da2f8..614097c6ce 100644
>> --- a/drivers/net/tsec.c
>> +++ b/drivers/net/tsec.c
>> @@ -78,52 +78,6 @@ static void tsec_configure_serdes(struct tsec_private *priv)
>>                                0, TBI_CR, CONFIG_TSEC_TBICR_SETTINGS);
>>   }
>>
>> -#ifdef CONFIG_MCAST_TFTP
>> -
>> -/* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
>> -
>> -/* Set the appropriate hash bit for the given addr */
>> -
>> -/*
>> - * The algorithm works like so:
>> - * 1) Take the Destination Address (ie the multicast address), and
>> - * do a CRC on it (little endian), and reverse the bits of the
>> - * result.
>> - * 2) Use the 8 most significant bits as a hash into a 256-entry
>> - * table.  The table is controlled through 8 32-bit registers:
>> - * gaddr0-7.  gaddr0's MSB is entry 0, and gaddr7's LSB is entry
>> - * 255.  This means that the 3 most significant bits in the
>> - * hash index which gaddr register to use, and the 5 other bits
>> - * indicate which bit (assuming an IBM numbering scheme, which
>> - * for PowerPC (tm) is usually the case) in the register holds
>> - * the entry.
>> - */
>> -#ifndef CONFIG_DM_ETH
>> -static int tsec_mcast_addr(struct eth_device *dev, const u8 *mcast_mac, u8 set)
>> -#else
>> -static int tsec_mcast_addr(struct udevice *dev, const u8 *mcast_mac, int set)
>> -#endif
>> -{
>> -       struct tsec_private *priv = (struct tsec_private *)dev->priv;
>> -       struct tsec __iomem *regs = priv->regs;
>> -       u32 result, value;
>> -       u8 whichbit, whichreg;
>> -
>> -       result = ether_crc(MAC_ADDR_LEN, mcast_mac);
>> -       whichbit = (result >> 24) & 0x1f; /* the 5 LSB = which bit to set */
>> -       whichreg = result >> 29; /* the 3 MSB = which reg to set it in */
>> -
>> -       value = BIT(31 - whichbit);
>> -
>> -       if (set)
>> -               setbits_be32(&regs->hash.gaddr0 + whichreg, value);
>> -       else
>> -               clrbits_be32(&regs->hash.gaddr0 + whichreg, value);
>> -
>> -       return 0;
>> -}
>> -#endif /* Multicast TFTP ? */
>> -
>>   /*
>>    * Initialized required registers to appropriate values, zeroing
>>    * those we don't care about (unless zero is bad, in which case,
>> @@ -720,9 +674,6 @@ static int tsec_initialize(bd_t *bis, struct tsec_info_struct *tsec_info)
>>          dev->halt = tsec_halt;
>>          dev->send = tsec_send;
>>          dev->recv = tsec_recv;
>> -#ifdef CONFIG_MCAST_TFTP
>> -       dev->mcast = tsec_mcast_addr;
>> -#endif
>>
>>          /* Tell U-Boot to get the addr from the env */
>>          for (i = 0; i < 6; i++)
>> @@ -862,9 +813,6 @@ static const struct eth_ops tsec_ops = {
>>          .recv = tsec_recv,
>>          .free_pkt = tsec_free_pkt,
>>          .stop = tsec_halt,
>> -#ifdef CONFIG_MCAST_TFTP
>> -       .mcast = tsec_mcast_addr,
>> -#endif
>>   };
>>
>>   static const struct udevice_id tsec_ids[] = {
>> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
>> index 90ef1f055f..41fe41e1a6 100644
>> --- a/drivers/usb/gadget/ether.c
>> +++ b/drivers/usb/gadget/ether.c
>> @@ -2607,9 +2607,6 @@ int usb_eth_initialize(bd_t *bi)
>>          netdev->halt = usb_eth_halt;
>>          netdev->priv = l_priv;
>>
>> -#ifdef CONFIG_MCAST_TFTP
>> -  #error not supported
>> -#endif
>>          eth_register(netdev);
>>          return 0;
>>   }
>> diff --git a/include/net.h b/include/net.h
>> index 51c099dae2..072b658442 100644
>> --- a/include/net.h
>> +++ b/include/net.h
>> @@ -123,7 +123,6 @@ enum eth_recv_flags {
>>    *          called when no error was returned from recv - optional
>>    * stop: Stop the hardware from looking for packets - may be called even if
>>    *      state == PASSIVE
>> - * mcast: Join or leave a multicast group (for TFTP) - optional
>>    * write_hwaddr: Write a MAC address to the hardware (used to pass it to Linux
>>    *              on some platforms like ARM). This function expects the
>>    *              eth_pdata::enetaddr field to be populated. The method can
>> @@ -140,9 +139,6 @@ struct eth_ops {
>>          int (*recv)(struct udevice *dev, int flags, uchar **packetp);
>>          int (*free_pkt)(struct udevice *dev, uchar *packet, int length);
>>          void (*stop)(struct udevice *dev);
>> -#ifdef CONFIG_MCAST_TFTP
>> -       int (*mcast)(struct udevice *dev, const u8 *enetaddr, int join);
>> -#endif
>>          int (*write_hwaddr)(struct udevice *dev);
>>          int (*read_rom_hwaddr)(struct udevice *dev);
>>   };
>> @@ -175,9 +171,6 @@ struct eth_device {
>>          int (*send)(struct eth_device *, void *packet, int length);
>>          int (*recv)(struct eth_device *);
>>          void (*halt)(struct eth_device *);
>> -#ifdef CONFIG_MCAST_TFTP
>> -       int (*mcast)(struct eth_device *, const u8 *enetaddr, u8 set);
>> -#endif
>>          int (*write_hwaddr)(struct eth_device *);
>>          struct eth_device *next;
>>          int index;
>> @@ -287,12 +280,6 @@ int eth_rx(void);                  /* Check for received packets */
>>   void eth_halt(void);                   /* stop SCC */
>>   const char *eth_get_name(void);                /* get name of current device */
>>
>> -#ifdef CONFIG_MCAST_TFTP
>> -int eth_mcast_join(struct in_addr mcast_addr, int join);
>> -u32 ether_crc(size_t len, unsigned char const *p);
>> -#endif
>> -
>> -
>>   /**********************************************************************/
>>   /*
>>    *     Protocol headers.
>> @@ -578,10 +565,6 @@ extern struct in_addr      net_ntp_server;         /* the ip address to NTP */
>>   extern int net_ntp_time_offset;                        /* offset time from UTC */
>>   #endif
>>
>> -#if defined(CONFIG_MCAST_TFTP)
>> -extern struct in_addr net_mcast_addr;
>> -#endif
>> -
>>   /* Initialize the network adapter */
>>   void net_init(void);
>>   int net_loop(enum proto_t);
>> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
>> index 91d861be41..bafa093c37 100644
>> --- a/net/eth-uclass.c
>> +++ b/net/eth-uclass.c
>> @@ -476,10 +476,6 @@ static int eth_post_probe(struct udevice *dev)
>>                          ops->free_pkt += gd->reloc_off;
>>                  if (ops->stop)
>>                          ops->stop += gd->reloc_off;
>> -#ifdef CONFIG_MCAST_TFTP
>> -               if (ops->mcast)
>> -                       ops->mcast += gd->reloc_off;
>> -#endif
>>                  if (ops->write_hwaddr)
>>                          ops->write_hwaddr += gd->reloc_off;
>>                  if (ops->read_rom_hwaddr)
>> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
>> index 2a9caa3509..7e82422b29 100644
>> --- a/net/eth_legacy.c
>> +++ b/net/eth_legacy.c
>> @@ -291,52 +291,6 @@ int eth_initialize(void)
>>          return num_devices;
>>   }
>>
>> -#ifdef CONFIG_MCAST_TFTP
>> -/* Multicast.
>> - * mcast_addr: multicast ipaddr from which multicast Mac is made
>> - * join: 1=join, 0=leave.
>> - */
>> -int eth_mcast_join(struct in_addr mcast_ip, int join)
>> -{
>> -       u8 mcast_mac[ARP_HLEN];
>> -       if (!eth_current || !eth_current->mcast)
>> -               return -1;
>> -       mcast_mac[5] = htonl(mcast_ip.s_addr) & 0xff;
>> -       mcast_mac[4] = (htonl(mcast_ip.s_addr)>>8) & 0xff;
>> -       mcast_mac[3] = (htonl(mcast_ip.s_addr)>>16) & 0x7f;
>> -       mcast_mac[2] = 0x5e;
>> -       mcast_mac[1] = 0x0;
>> -       mcast_mac[0] = 0x1;
>> -       return eth_current->mcast(eth_current, mcast_mac, join);
>> -}
>> -
>> -/* the 'way' for ethernet-CRC-32. Spliced in from Linux lib/crc32.c
>> - * and this is the ethernet-crc method needed for TSEC -- and perhaps
>> - * some other adapter -- hash tables
>> - */
>> -#define CRCPOLY_LE 0xedb88320
>> -u32 ether_crc(size_t len, unsigned char const *p)
>> -{
>> -       int i;
>> -       u32 crc;
>> -       crc = ~0;
>> -       while (len--) {
>> -               crc ^= *p++;
>> -               for (i = 0; i < 8; i++)
>> -                       crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
>> -       }
>> -       /* an reverse the bits, cuz of way they arrive -- last-first */
>> -       crc = (crc >> 16) | (crc << 16);
>> -       crc = (crc >> 8 & 0x00ff00ff) | (crc << 8 & 0xff00ff00);
>> -       crc = (crc >> 4 & 0x0f0f0f0f) | (crc << 4 & 0xf0f0f0f0);
>> -       crc = (crc >> 2 & 0x33333333) | (crc << 2 & 0xcccccccc);
>> -       crc = (crc >> 1 & 0x55555555) | (crc << 1 & 0xaaaaaaaa);
>> -       return crc;
>> -}
>> -
>> -#endif
>> -
>> -
>>   int eth_init(void)
>>   {
>>          struct eth_device *old_current;
>> diff --git a/net/net.c b/net/net.c
>> index a5a216c3ee..72286e24f4 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -131,10 +131,6 @@ struct in_addr net_dns_server;
>>   struct in_addr net_dns_server2;
>>   #endif
>>
>> -#ifdef CONFIG_MCAST_TFTP       /* Multicast TFTP */
>> -struct in_addr net_mcast_addr;
>> -#endif
>> -
>>   /** END OF BOOTP EXTENTIONS **/
>>
>>   /* Our ethernet address */
>> @@ -1215,10 +1211,7 @@ void net_process_received_packet(uchar *in_packet, int len)
>>                  dst_ip = net_read_ip(&ip->ip_dst);
>>                  if (net_ip.s_addr && dst_ip.s_addr != net_ip.s_addr &&
>>                      dst_ip.s_addr != 0xFFFFFFFF) {
>> -#ifdef CONFIG_MCAST_TFTP
>> -                       if (net_mcast_addr != dst_ip)
>> -#endif
>> -                               return;
>> +                       return;
>>                  }
>>                  /* Read source IP address for later use */
>>                  src_ip = net_read_ip(&ip->ip_src);
>> diff --git a/net/tftp.c b/net/tftp.c
>> index 68ffd81414..563ce3a06f 100644
>> --- a/net/tftp.c
>> +++ b/net/tftp.c
>> @@ -134,36 +134,6 @@ 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;
>>
>> -#ifdef CONFIG_MCAST_TFTP
>> -#include <malloc.h>
>> -#define MTFTP_BITMAPSIZE       0x1000
>> -static unsigned *tftp_mcast_bitmap;
>> -static int tftp_mcast_prev_hole;
>> -static int tftp_mcast_bitmap_size = MTFTP_BITMAPSIZE;
>> -static int tftp_mcast_disabled;
>> -static int tftp_mcast_master_client;
>> -static int tftp_mcast_active;
>> -static int tftp_mcast_port;
>> -/* can get 'last' block before done..*/
>> -static ulong tftp_mcast_ending_block;
>> -
>> -static void parse_multicast_oack(char *pkt, int len);
>> -
>> -static void mcast_cleanup(void)
>> -{
>> -       if (net_mcast_addr)
>> -               eth_mcast_join(net_mcast_addr, 0);
>> -       if (tftp_mcast_bitmap)
>> -               free(tftp_mcast_bitmap);
>> -       tftp_mcast_bitmap = NULL;
>> -       net_mcast_addr.s_addr = 0;
>> -       tftp_mcast_active = 0;
>> -       tftp_mcast_port = 0;
>> -       tftp_mcast_ending_block = -1;
>> -}
>> -
>> -#endif /* CONFIG_MCAST_TFTP */
>> -
>>   static inline void store_block(int block, uchar *src, unsigned len)
>>   {
>>          ulong offset = block * tftp_block_size + tftp_block_wrap_offset;
>> @@ -196,10 +166,6 @@ static inline void store_block(int block, uchar *src, unsigned len)
>>                  memcpy(ptr, src, len);
>>                  unmap_sysmem(ptr);
>>          }
>> -#ifdef CONFIG_MCAST_TFTP
>> -       if (tftp_mcast_active)
>> -               ext2_set_bit(block, tftp_mcast_bitmap);
>> -#endif
>>
>>          if (net_boot_file_size < newsize)
>>                  net_boot_file_size = newsize;
>> @@ -275,9 +241,6 @@ static void show_block_marker(void)
>>   static void restart(const char *msg)
>>   {
>>          printf("\n%s; starting again\n", msg);
>> -#ifdef CONFIG_MCAST_TFTP
>> -       mcast_cleanup();
>> -#endif
>>          net_start_again();
>>   }
>>
>> @@ -332,12 +295,6 @@ static void tftp_send(void)
>>          int len = 0;
>>          ushort *s;
>>
>> -#ifdef CONFIG_MCAST_TFTP
>> -       /* Multicast TFTP.. non-MasterClients do not ACK data. */
>> -       if (tftp_mcast_active && tftp_state == STATE_DATA &&
>> -           tftp_mcast_master_client == 0)
>> -               return;
>> -#endif
>>          /*
>>           *      We will always be sending some sort of packet, so
>>           *      cobble together the packet headers now.
>> @@ -372,31 +329,10 @@ static void tftp_send(void)
>>                  /* try for more effic. blk size */
>>                  pkt += sprintf((char *)pkt, "blksize%c%d%c",
>>                                  0, tftp_block_size_option, 0);
>> -#ifdef CONFIG_MCAST_TFTP
>> -               /* Check all preconditions before even trying the option */
>> -               if (!tftp_mcast_disabled) {
>> -                       tftp_mcast_bitmap = malloc(tftp_mcast_bitmap_size);
>> -                       if (tftp_mcast_bitmap && eth_get_dev()->mcast) {
>> -                               free(tftp_mcast_bitmap);
>> -                               tftp_mcast_bitmap = NULL;
>> -                               pkt += sprintf((char *)pkt, "multicast%c%c",
>> -                                       0, 0);
>> -                       }
>> -               }
>> -#endif /* CONFIG_MCAST_TFTP */
>>                  len = pkt - xp;
>>                  break;
>>
>>          case STATE_OACK:
>> -#ifdef CONFIG_MCAST_TFTP
>> -               /* My turn!  Start at where I need blocks I missed. */
>> -               if (tftp_mcast_active)
>> -                       tftp_cur_block = ext2_find_next_zero_bit(
>> -                               tftp_mcast_bitmap,
>> -                               tftp_mcast_bitmap_size * 8, 0);
>> -               /* fall through */
>> -#endif
>> -
>>          case STATE_RECV_WRQ:
>>          case STATE_DATA:
>>                  xp = pkt;
>> @@ -465,11 +401,7 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>>          int i;
>>
>>          if (dest != tftp_our_port) {
>> -#ifdef CONFIG_MCAST_TFTP
>> -               if (tftp_mcast_active &&
>> -                   (!tftp_mcast_port || dest != tftp_mcast_port))
>> -#endif
>> -                       return;
>> +               return;
>>          }
>>          if (tftp_state != STATE_SEND_RRQ && src != tftp_remote_port &&
>>              tftp_state != STATE_RECV_WRQ && tftp_state != STATE_SEND_WRQ)
>> @@ -549,12 +481,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>>                          }
>>   #endif
>>                  }
>> -#ifdef CONFIG_MCAST_TFTP
>> -               parse_multicast_oack((char *)pkt, len - 1);
>> -               if ((tftp_mcast_active) && (!tftp_mcast_master_client))
>> -                       tftp_state = STATE_DATA;        /* passive.. */
>> -               else
>> -#endif
>>   #ifdef CONFIG_CMD_TFTPPUT
>>                  if (tftp_put_active) {
>>                          /* Get ready to send the first block */
>> @@ -582,11 +508,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>>                          tftp_remote_port = src;
>>                          new_transfer();
>>
>> -#ifdef CONFIG_MCAST_TFTP
>> -                       if (tftp_mcast_active) { /* start!=1 common if mcast */
>> -                               tftp_prev_block = tftp_cur_block - 1;
>> -                       } else
>> -#endif
>>                          if (tftp_cur_block != 1) {      /* Assertion */
>>                                  puts("\nTFTP error: ");
>>                                  printf("First block is not block 1 (%ld)\n",
>> @@ -612,44 +533,8 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>>                   *      Acknowledge the block just received, which will prompt
>>                   *      the remote for the next one.
>>                   */
>> -#ifdef CONFIG_MCAST_TFTP
>> -               /* if I am the MasterClient, actively calculate what my next
>> -                * needed block is; else I'm passive; not ACKING
>> -                */
>> -               if (tftp_mcast_active) {
>> -                       if (len < tftp_block_size)  {
>> -                               tftp_mcast_ending_block = tftp_cur_block;
>> -                       } else if (tftp_mcast_master_client) {
>> -                               tftp_mcast_prev_hole = ext2_find_next_zero_bit(
>> -                                       tftp_mcast_bitmap,
>> -                                       tftp_mcast_bitmap_size * 8,
>> -                                       tftp_mcast_prev_hole);
>> -                               tftp_cur_block = tftp_mcast_prev_hole;
>> -                               if (tftp_cur_block >
>> -                                   ((tftp_mcast_bitmap_size * 8) - 1)) {
>> -                                       debug("tftpfile too big\n");
>> -                                       /* try to double it and retry */
>> -                                       tftp_mcast_bitmap_size <<= 1;
>> -                                       mcast_cleanup();
>> -                                       net_start_again();
>> -                                       return;
>> -                               }
>> -                               tftp_prev_block = tftp_cur_block;
>> -                       }
>> -               }
>> -#endif
>>                  tftp_send();
>>
>> -#ifdef CONFIG_MCAST_TFTP
>> -               if (tftp_mcast_active) {
>> -                       if (tftp_mcast_master_client &&
>> -                           (tftp_cur_block >= tftp_mcast_ending_block)) {
>> -                               puts("\nMulticast tftp done\n");
>> -                               mcast_cleanup();
>> -                               net_set_state(NETLOOP_SUCCESS);
>> -                       }
>> -               } else
>> -#endif
>>                  if (len < tftp_block_size)
>>                          tftp_complete();
>>                  break;
>> @@ -672,9 +557,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>>                  case TFTP_ERR_FILE_ALREADY_EXISTS:
>>                  default:
>>                          puts("Starting again\n\n");
>> -#ifdef CONFIG_MCAST_TFTP
>> -                       mcast_cleanup();
>> -#endif
>>                          net_start_again();
>>                          break;
>>                  }
>> @@ -826,9 +708,6 @@ void tftp_start(enum proto_t protocol)
>>          memset(net_server_ethaddr, 0, 6);
>>          /* Revert tftp_block_size to dflt */
>>          tftp_block_size = TFTP_BLOCK_SIZE;
>> -#ifdef CONFIG_MCAST_TFTP
>> -       mcast_cleanup();
>> -#endif
>>   #ifdef CONFIG_TFTP_TSIZE
>>          tftp_tsize = 0;
>>          tftp_tsize_num_hash = 0;
>> @@ -870,103 +749,3 @@ void tftp_start_server(void)
>>          memset(net_server_ethaddr, 0, 6);
>>   }
>>   #endif /* CONFIG_CMD_TFTPSRV */
>> -
>> -#ifdef CONFIG_MCAST_TFTP
>> -/*
>> - * Credits: atftp project.
>> - */
>> -
>> -/*
>> - * Pick up BcastAddr, Port, and whether I am [now] the master-client.
>> - * Frame:
>> - *    +-------+-----------+---+-------~~-------+---+
>> - *    |  opc  | multicast | 0 | addr, port, mc | 0 |
>> - *    +-------+-----------+---+-------~~-------+---+
>> - * The multicast addr/port becomes what I listen to, and if 'mc' is '1' then
>> - * I am the new master-client so must send ACKs to DataBlocks.  If I am not
>> - * master-client, I'm a passive client, gathering what DataBlocks I may and
>> - * making note of which ones I got in my bitmask.
>> - * In theory, I never go from master->passive..
>> - * .. this comes in with pkt already pointing just past opc
>> - */
>> -static void parse_multicast_oack(char *pkt, int len)
>> -{
>> -       int i;
>> -       struct in_addr addr;
>> -       char *mc_adr;
>> -       char *port;
>> -       char *mc;
>> -
>> -       mc_adr = NULL;
>> -       port = NULL;
>> -       mc = NULL;
>> -       /* march along looking for 'multicast\0', which has to start at least
>> -        * 14 bytes back from the end.
>> -        */
>> -       for (i = 0; i < len - 14; i++)
>> -               if (strcmp(pkt + i, "multicast") == 0)
>> -                       break;
>> -       if (i >= (len - 14)) /* non-Multicast OACK, ign. */
>> -               return;
>> -
>> -       i += 10; /* strlen multicast */
>> -       mc_adr = pkt + i;
>> -       for (; i < len; i++) {
>> -               if (*(pkt + i) == ',') {
>> -                       *(pkt + i) = '\0';
>> -                       if (port) {
>> -                               mc = pkt + i + 1;
>> -                               break;
>> -                       } else {
>> -                               port = pkt + i + 1;
>> -                       }
>> -               }
>> -       }
>> -       if (!port || !mc_adr || !mc)
>> -               return;
>> -       if (tftp_mcast_active && tftp_mcast_master_client) {
>> -               printf("I got a OACK as master Client, WRONG!\n");
>> -               return;
>> -       }
>> -       /* ..I now accept packets destined for this MCAST addr, port */
>> -       if (!tftp_mcast_active) {
>> -               if (tftp_mcast_bitmap) {
>> -                       printf("Internal failure! no mcast.\n");
>> -                       free(tftp_mcast_bitmap);
>> -                       tftp_mcast_bitmap = NULL;
>> -                       tftp_mcast_disabled = 1;
>> -                       return;
>> -               }
>> -               /* I malloc instead of pre-declare; so that if the file ends
>> -                * up being too big for this bitmap I can retry
>> -                */
>> -               tftp_mcast_bitmap = malloc(tftp_mcast_bitmap_size);
>> -               if (!tftp_mcast_bitmap) {
>> -                       printf("No bitmap, no multicast. Sorry.\n");
>> -                       tftp_mcast_disabled = 1;
>> -                       return;
>> -               }
>> -               memset(tftp_mcast_bitmap, 0, tftp_mcast_bitmap_size);
>> -               tftp_mcast_prev_hole = 0;
>> -               tftp_mcast_active = 1;
>> -       }
>> -       addr = string_to_ip(mc_adr);
>> -       if (net_mcast_addr.s_addr != addr.s_addr) {
>> -               if (net_mcast_addr.s_addr)
>> -                       eth_mcast_join(net_mcast_addr, 0);
>> -               net_mcast_addr = addr;
>> -               if (eth_mcast_join(net_mcast_addr, 1)) {
>> -                       printf("Fail to set mcast, revert to TFTP\n");
>> -                       tftp_mcast_disabled = 1;
>> -                       mcast_cleanup();
>> -                       net_start_again();
>> -               }
>> -       }
>> -       tftp_mcast_master_client = simple_strtoul((char *)mc, NULL, 10);
>> -       tftp_mcast_port = (unsigned short)simple_strtoul(port, NULL, 10);
>> -       printf("Multicast: %s:%d [%d]\n", mc_adr, tftp_mcast_port,
>> -              tftp_mcast_master_client);
>> -       return;
>> -}
>> -
>> -#endif /* Multicast TFTP */
>> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
>> index 0627024e71..b2691206fb 100644
>> --- a/scripts/config_whitelist.txt
>> +++ b/scripts/config_whitelist.txt
>> @@ -1210,7 +1210,6 @@ CONFIG_MAX_FPGA_DEVICES
>>   CONFIG_MAX_MEM_MAPPED
>>   CONFIG_MAX_PKT
>>   CONFIG_MAX_RAM_BANK_SIZE
>> -CONFIG_MCAST_TFTP
>>   CONFIG_MCF5249
>>   CONFIG_MCF5253
>>   CONFIG_MCFFEC
>> --
>> 2.17.1
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 7/8] net: remove CONFIG_MCAST_TFTP

[Chris Packham]

On Sun, 18 Nov 2018, 5:49 AM Simon Goldschmidt <
simon.k.r.goldschmidt@gmail.com wrote:

> On 17.11.2018 17:05, Joe Hershberger wrote:
> > Hi Simon,
> >
> > Thanks for tackling this.
> >
> > On Sat, Nov 17, 2018 at 6:31 AM Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> >> This option seems unused as no mainline board enables it and it does
> >> not compile since some years.
> >>
> >> Additionally, it has a potential buffer underrun issue (reported as
> >> a side node in CVE-2018-18439).
> >>
> >> Instead of trying to fix a thing noone seems to use, let's rather
> >> drop dead code.
> >>
> >> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >> ---
> >>
> >> Changes in v2:
> >> - this patch is new in v2
> >>
> >>   README                       |   9 --
> >>   drivers/net/rtl8139.c        |   9 --
> >>   drivers/net/tsec.c           |  52 --------
> >>   drivers/usb/gadget/ether.c   |   3 -
> >>   include/net.h                |  17 ---
> >>   net/eth-uclass.c             |   4 -
> >>   net/eth_legacy.c             |  46 --------
> >>   net/net.c                    |   9 +-
> >>   net/tftp.c                   | 223 +----------------------------------
> > I think the TFTP stuff can be removed, but multicast in general
> > probably not, since I believe IPv6 support requires it.


Thanks Joe. I swear IPv6 is still on my todo list.

I guess we
> > could put it back at that time, but I'm a bit concerned that we will
> > diverge and make that work harder. Maybe it's feasible to simply stop
> > guarding multicast support in the core and drivers with the TFTP
> > config and enable it at all times. The code size on that side is
> > minimal.
>
> Well, it was a bit hard for me to see what's general multicast and
> what's tftp only, given that it's all using one tftp-related guard.
> Maybe it would be better to drop this patch from the series to discuss
> this in a separate thread?
>

I've got a reasonable handle on the mcast requirements for IPv6. Do you
want me to have a go at separating these?

Basically the network drivers need to retain the APIs (maybe with a
CONFIG_MCAST option). The tftp parts can go and I'd need to take a closer
look at net.c.

Simon
>
> >
> > Thanks,
> > -Joe
> >
> >>   scripts/config_whitelist.txt |   1 -
> >>   10 files changed, 2 insertions(+), 371 deletions(-)
> >>
> >> diff --git a/README b/README
> >> index a46c7c63a4..97a3e9a84b 100644
> >> --- a/README
> >> +++ b/README
> >> @@ -1429,15 +1429,6 @@ The following options need to be configured:
> >>                  forwarded through a router.
> >>                  (Environment variable "netmask")
> >>
> >> -- Multicast TFTP Mode:
> >> -               CONFIG_MCAST_TFTP
> >> -
> >> -               Defines whether you want to support multicast TFTP as
> per
> >> -               rfc-2090; for example to work with atftp.  Lets lots of
> targets
> >> -               tftp down the same boot image concurrently.  Note: the
> Ethernet
> >> -               driver in use must provide a function: mcast() to
> join/leave a
> >> -               multicast group.
> >> -
> >>   - BOOTP Recovery Mode:
> >>                  CONFIG_BOOTP_RANDOM_DELAY
> >>
> >> diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c
> >> index 590f8ce154..79acd64bc0 100644
> >> --- a/drivers/net/rtl8139.c
> >> +++ b/drivers/net/rtl8139.c
> >> @@ -183,12 +183,6 @@ static void rtl_reset(struct eth_device *dev);
> >>   static int rtl_transmit(struct eth_device *dev, void *packet, int
> length);
> >>   static int rtl_poll(struct eth_device *dev);
> >>   static void rtl_disable(struct eth_device *dev);
> >> -#ifdef CONFIG_MCAST_TFTP/*  This driver already accepts all b/mcast */
> >> -static int rtl_bcast_addr(struct eth_device *dev, const u8 *bcast_mac,
> u8 set)
> >> -{
> >> -       return (0);
> >> -}
> >> -#endif
> >>
> >>   static struct pci_device_id supported[] = {
> >>          {PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8139},
> >> @@ -229,9 +223,6 @@ int rtl8139_initialize(bd_t *bis)
> >>                  dev->halt = rtl_disable;
> >>                  dev->send = rtl_transmit;
> >>                  dev->recv = rtl_poll;
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -               dev->mcast = rtl_bcast_addr;
> >> -#endif
> >>
> >>                  eth_register (dev);
> >>
> >> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> >> index 03a46da2f8..614097c6ce 100644
> >> --- a/drivers/net/tsec.c
> >> +++ b/drivers/net/tsec.c
> >> @@ -78,52 +78,6 @@ static void tsec_configure_serdes(struct
> tsec_private *priv)
> >>                                0, TBI_CR, CONFIG_TSEC_TBICR_SETTINGS);
> >>   }
> >>
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -
> >> -/* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
> >> -
> >> -/* Set the appropriate hash bit for the given addr */
> >> -
> >> -/*
> >> - * The algorithm works like so:
> >> - * 1) Take the Destination Address (ie the multicast address), and
> >> - * do a CRC on it (little endian), and reverse the bits of the
> >> - * result.
> >> - * 2) Use the 8 most significant bits as a hash into a 256-entry
> >> - * table.  The table is controlled through 8 32-bit registers:
> >> - * gaddr0-7.  gaddr0's MSB is entry 0, and gaddr7's LSB is entry
> >> - * 255.  This means that the 3 most significant bits in the
> >> - * hash index which gaddr register to use, and the 5 other bits
> >> - * indicate which bit (assuming an IBM numbering scheme, which
> >> - * for PowerPC (tm) is usually the case) in the register holds
> >> - * the entry.
> >> - */
> >> -#ifndef CONFIG_DM_ETH
> >> -static int tsec_mcast_addr(struct eth_device *dev, const u8
> *mcast_mac, u8 set)
> >> -#else
> >> -static int tsec_mcast_addr(struct udevice *dev, const u8 *mcast_mac,
> int set)
> >> -#endif
> >> -{
> >> -       struct tsec_private *priv = (struct tsec_private *)dev->priv;
> >> -       struct tsec __iomem *regs = priv->regs;
> >> -       u32 result, value;
> >> -       u8 whichbit, whichreg;
> >> -
> >> -       result = ether_crc(MAC_ADDR_LEN, mcast_mac);
> >> -       whichbit = (result >> 24) & 0x1f; /* the 5 LSB = which bit to
> set */
> >> -       whichreg = result >> 29; /* the 3 MSB = which reg to set it in
> */
> >> -
> >> -       value = BIT(31 - whichbit);
> >> -
> >> -       if (set)
> >> -               setbits_be32(&regs->hash.gaddr0 + whichreg, value);
> >> -       else
> >> -               clrbits_be32(&regs->hash.gaddr0 + whichreg, value);
> >> -
> >> -       return 0;
> >> -}
> >> -#endif /* Multicast TFTP ? */
> >> -
> >>   /*
> >>    * Initialized required registers to appropriate values, zeroing
> >>    * those we don't care about (unless zero is bad, in which case,
> >> @@ -720,9 +674,6 @@ static int tsec_initialize(bd_t *bis, struct
> tsec_info_struct *tsec_info)
> >>          dev->halt = tsec_halt;
> >>          dev->send = tsec_send;
> >>          dev->recv = tsec_recv;
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -       dev->mcast = tsec_mcast_addr;
> >> -#endif
> >>
> >>          /* Tell U-Boot to get the addr from the env */
> >>          for (i = 0; i < 6; i++)
> >> @@ -862,9 +813,6 @@ static const struct eth_ops tsec_ops = {
> >>          .recv = tsec_recv,
> >>          .free_pkt = tsec_free_pkt,
> >>          .stop = tsec_halt,
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -       .mcast = tsec_mcast_addr,
> >> -#endif
> >>   };
> >>
> >>   static const struct udevice_id tsec_ids[] = {
> >> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
> >> index 90ef1f055f..41fe41e1a6 100644
> >> --- a/drivers/usb/gadget/ether.c
> >> +++ b/drivers/usb/gadget/ether.c
> >> @@ -2607,9 +2607,6 @@ int usb_eth_initialize(bd_t *bi)
> >>          netdev->halt = usb_eth_halt;
> >>          netdev->priv = l_priv;
> >>
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -  #error not supported
> >> -#endif
> >>          eth_register(netdev);
> >>          return 0;
> >>   }
> >> diff --git a/include/net.h b/include/net.h
> >> index 51c099dae2..072b658442 100644
> >> --- a/include/net.h
> >> +++ b/include/net.h
> >> @@ -123,7 +123,6 @@ enum eth_recv_flags {
> >>    *          called when no error was returned from recv - optional
> >>    * stop: Stop the hardware from looking for packets - may be called
> even if
> >>    *      state == PASSIVE
> >> - * mcast: Join or leave a multicast group (for TFTP) - optional
> >>    * write_hwaddr: Write a MAC address to the hardware (used to pass it
> to Linux
> >>    *              on some platforms like ARM). This function expects the
> >>    *              eth_pdata::enetaddr field to be populated. The method
> can
> >> @@ -140,9 +139,6 @@ struct eth_ops {
> >>          int (*recv)(struct udevice *dev, int flags, uchar **packetp);
> >>          int (*free_pkt)(struct udevice *dev, uchar *packet, int
> length);
> >>          void (*stop)(struct udevice *dev);
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -       int (*mcast)(struct udevice *dev, const u8 *enetaddr, int join);
> >> -#endif
> >>          int (*write_hwaddr)(struct udevice *dev);
> >>          int (*read_rom_hwaddr)(struct udevice *dev);
> >>   };
> >> @@ -175,9 +171,6 @@ struct eth_device {
> >>          int (*send)(struct eth_device *, void *packet, int length);
> >>          int (*recv)(struct eth_device *);
> >>          void (*halt)(struct eth_device *);
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -       int (*mcast)(struct eth_device *, const u8 *enetaddr, u8 set);
> >> -#endif
> >>          int (*write_hwaddr)(struct eth_device *);
> >>          struct eth_device *next;
> >>          int index;
> >> @@ -287,12 +280,6 @@ int eth_rx(void);                  /* Check for
> received packets */
> >>   void eth_halt(void);                   /* stop SCC */
> >>   const char *eth_get_name(void);                /* get name of current
> device */
> >>
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -int eth_mcast_join(struct in_addr mcast_addr, int join);
> >> -u32 ether_crc(size_t len, unsigned char const *p);
> >> -#endif
> >> -
> >> -
> >>
>  /**********************************************************************/
> >>   /*
> >>    *     Protocol headers.
> >> @@ -578,10 +565,6 @@ extern struct in_addr      net_ntp_server;
>  /* the ip address to NTP */
> >>   extern int net_ntp_time_offset;                        /* offset time
> from UTC */
> >>   #endif
> >>
> >> -#if defined(CONFIG_MCAST_TFTP)
> >> -extern struct in_addr net_mcast_addr;
> >> -#endif
> >> -
> >>   /* Initialize the network adapter */
> >>   void net_init(void);
> >>   int net_loop(enum proto_t);
> >> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> >> index 91d861be41..bafa093c37 100644
> >> --- a/net/eth-uclass.c
> >> +++ b/net/eth-uclass.c
> >> @@ -476,10 +476,6 @@ static int eth_post_probe(struct udevice *dev)
> >>                          ops->free_pkt += gd->reloc_off;
> >>                  if (ops->stop)
> >>                          ops->stop += gd->reloc_off;
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -               if (ops->mcast)
> >> -                       ops->mcast += gd->reloc_off;
> >> -#endif
> >>                  if (ops->write_hwaddr)
> >>                          ops->write_hwaddr += gd->reloc_off;
> >>                  if (ops->read_rom_hwaddr)
> >> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> >> index 2a9caa3509..7e82422b29 100644
> >> --- a/net/eth_legacy.c
> >> +++ b/net/eth_legacy.c
> >> @@ -291,52 +291,6 @@ int eth_initialize(void)
> >>          return num_devices;
> >>   }
> >>
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -/* Multicast.
> >> - * mcast_addr: multicast ipaddr from which multicast Mac is made
> >> - * join: 1=join, 0=leave.
> >> - */
> >> -int eth_mcast_join(struct in_addr mcast_ip, int join)
> >> -{
> >> -       u8 mcast_mac[ARP_HLEN];
> >> -       if (!eth_current || !eth_current->mcast)
> >> -               return -1;
> >> -       mcast_mac[5] = htonl(mcast_ip.s_addr) & 0xff;
> >> -       mcast_mac[4] = (htonl(mcast_ip.s_addr)>>8) & 0xff;
> >> -       mcast_mac[3] = (htonl(mcast_ip.s_addr)>>16) & 0x7f;
> >> -       mcast_mac[2] = 0x5e;
> >> -       mcast_mac[1] = 0x0;
> >> -       mcast_mac[0] = 0x1;
> >> -       return eth_current->mcast(eth_current, mcast_mac, join);
> >> -}
> >> -
> >> -/* the 'way' for ethernet-CRC-32. Spliced in from Linux lib/crc32.c
> >> - * and this is the ethernet-crc method needed for TSEC -- and perhaps
> >> - * some other adapter -- hash tables
> >> - */
> >> -#define CRCPOLY_LE 0xedb88320
> >> -u32 ether_crc(size_t len, unsigned char const *p)
> >> -{
> >> -       int i;
> >> -       u32 crc;
> >> -       crc = ~0;
> >> -       while (len--) {
> >> -               crc ^= *p++;
> >> -               for (i = 0; i < 8; i++)
> >> -                       crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
> >> -       }
> >> -       /* an reverse the bits, cuz of way they arrive -- last-first */
> >> -       crc = (crc >> 16) | (crc << 16);
> >> -       crc = (crc >> 8 & 0x00ff00ff) | (crc << 8 & 0xff00ff00);
> >> -       crc = (crc >> 4 & 0x0f0f0f0f) | (crc << 4 & 0xf0f0f0f0);
> >> -       crc = (crc >> 2 & 0x33333333) | (crc << 2 & 0xcccccccc);
> >> -       crc = (crc >> 1 & 0x55555555) | (crc << 1 & 0xaaaaaaaa);
> >> -       return crc;
> >> -}
> >> -
> >> -#endif
> >> -
> >> -
> >>   int eth_init(void)
> >>   {
> >>          struct eth_device *old_current;
> >> diff --git a/net/net.c b/net/net.c
> >> index a5a216c3ee..72286e24f4 100644
> >> --- a/net/net.c
> >> +++ b/net/net.c
> >> @@ -131,10 +131,6 @@ struct in_addr net_dns_server;
> >>   struct in_addr net_dns_server2;
> >>   #endif
> >>
> >> -#ifdef CONFIG_MCAST_TFTP       /* Multicast TFTP */
> >> -struct in_addr net_mcast_addr;
> >> -#endif
> >> -
> >>   /** END OF BOOTP EXTENTIONS **/
> >>
> >>   /* Our ethernet address */
> >> @@ -1215,10 +1211,7 @@ void net_process_received_packet(uchar
> *in_packet, int len)
> >>                  dst_ip = net_read_ip(&ip->ip_dst);
> >>                  if (net_ip.s_addr && dst_ip.s_addr != net_ip.s_addr &&
> >>                      dst_ip.s_addr != 0xFFFFFFFF) {
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -                       if (net_mcast_addr != dst_ip)
> >> -#endif
> >> -                               return;
> >> +                       return;
> >>                  }
> >>                  /* Read source IP address for later use */
> >>                  src_ip = net_read_ip(&ip->ip_src);
> >> diff --git a/net/tftp.c b/net/tftp.c
> >> index 68ffd81414..563ce3a06f 100644
> >> --- a/net/tftp.c
> >> +++ b/net/tftp.c
> >> @@ -134,36 +134,6 @@ 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;
> >>
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -#include <malloc.h>
> >> -#define MTFTP_BITMAPSIZE       0x1000
> >> -static unsigned *tftp_mcast_bitmap;
> >> -static int tftp_mcast_prev_hole;
> >> -static int tftp_mcast_bitmap_size = MTFTP_BITMAPSIZE;
> >> -static int tftp_mcast_disabled;
> >> -static int tftp_mcast_master_client;
> >> -static int tftp_mcast_active;
> >> -static int tftp_mcast_port;
> >> -/* can get 'last' block before done..*/
> >> -static ulong tftp_mcast_ending_block;
> >> -
> >> -static void parse_multicast_oack(char *pkt, int len);
> >> -
> >> -static void mcast_cleanup(void)
> >> -{
> >> -       if (net_mcast_addr)
> >> -               eth_mcast_join(net_mcast_addr, 0);
> >> -       if (tftp_mcast_bitmap)
> >> -               free(tftp_mcast_bitmap);
> >> -       tftp_mcast_bitmap = NULL;
> >> -       net_mcast_addr.s_addr = 0;
> >> -       tftp_mcast_active = 0;
> >> -       tftp_mcast_port = 0;
> >> -       tftp_mcast_ending_block = -1;
> >> -}
> >> -
> >> -#endif /* CONFIG_MCAST_TFTP */
> >> -
> >>   static inline void store_block(int block, uchar *src, unsigned len)
> >>   {
> >>          ulong offset = block * tftp_block_size +
> tftp_block_wrap_offset;
> >> @@ -196,10 +166,6 @@ static inline void store_block(int block, uchar
> *src, unsigned len)
> >>                  memcpy(ptr, src, len);
> >>                  unmap_sysmem(ptr);
> >>          }
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -       if (tftp_mcast_active)
> >> -               ext2_set_bit(block, tftp_mcast_bitmap);
> >> -#endif
> >>
> >>          if (net_boot_file_size < newsize)
> >>                  net_boot_file_size = newsize;
> >> @@ -275,9 +241,6 @@ static void show_block_marker(void)
> >>   static void restart(const char *msg)
> >>   {
> >>          printf("\n%s; starting again\n", msg);
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -       mcast_cleanup();
> >> -#endif
> >>          net_start_again();
> >>   }
> >>
> >> @@ -332,12 +295,6 @@ static void tftp_send(void)
> >>          int len = 0;
> >>          ushort *s;
> >>
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -       /* Multicast TFTP.. non-MasterClients do not ACK data. */
> >> -       if (tftp_mcast_active && tftp_state == STATE_DATA &&
> >> -           tftp_mcast_master_client == 0)
> >> -               return;
> >> -#endif
> >>          /*
> >>           *      We will always be sending some sort of packet, so
> >>           *      cobble together the packet headers now.
> >> @@ -372,31 +329,10 @@ static void tftp_send(void)
> >>                  /* try for more effic. blk size */
> >>                  pkt += sprintf((char *)pkt, "blksize%c%d%c",
> >>                                  0, tftp_block_size_option, 0);
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -               /* Check all preconditions before even trying the
> option */
> >> -               if (!tftp_mcast_disabled) {
> >> -                       tftp_mcast_bitmap =
> malloc(tftp_mcast_bitmap_size);
> >> -                       if (tftp_mcast_bitmap && eth_get_dev()->mcast) {
> >> -                               free(tftp_mcast_bitmap);
> >> -                               tftp_mcast_bitmap = NULL;
> >> -                               pkt += sprintf((char *)pkt,
> "multicast%c%c",
> >> -                                       0, 0);
> >> -                       }
> >> -               }
> >> -#endif /* CONFIG_MCAST_TFTP */
> >>                  len = pkt - xp;
> >>                  break;
> >>
> >>          case STATE_OACK:
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -               /* My turn!  Start at where I need blocks I missed. */
> >> -               if (tftp_mcast_active)
> >> -                       tftp_cur_block = ext2_find_next_zero_bit(
> >> -                               tftp_mcast_bitmap,
> >> -                               tftp_mcast_bitmap_size * 8, 0);
> >> -               /* fall through */
> >> -#endif
> >> -
> >>          case STATE_RECV_WRQ:
> >>          case STATE_DATA:
> >>                  xp = pkt;
> >> @@ -465,11 +401,7 @@ static void tftp_handler(uchar *pkt, unsigned
> dest, struct in_addr sip,
> >>          int i;
> >>
> >>          if (dest != tftp_our_port) {
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -               if (tftp_mcast_active &&
> >> -                   (!tftp_mcast_port || dest != tftp_mcast_port))
> >> -#endif
> >> -                       return;
> >> +               return;
> >>          }
> >>          if (tftp_state != STATE_SEND_RRQ && src != tftp_remote_port &&
> >>              tftp_state != STATE_RECV_WRQ && tftp_state !=
> STATE_SEND_WRQ)
> >> @@ -549,12 +481,6 @@ static void tftp_handler(uchar *pkt, unsigned
> dest, struct in_addr sip,
> >>                          }
> >>   #endif
> >>                  }
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -               parse_multicast_oack((char *)pkt, len - 1);
> >> -               if ((tftp_mcast_active) && (!tftp_mcast_master_client))
> >> -                       tftp_state = STATE_DATA;        /* passive.. */
> >> -               else
> >> -#endif
> >>   #ifdef CONFIG_CMD_TFTPPUT
> >>                  if (tftp_put_active) {
> >>                          /* Get ready to send the first block */
> >> @@ -582,11 +508,6 @@ static void tftp_handler(uchar *pkt, unsigned
> dest, struct in_addr sip,
> >>                          tftp_remote_port = src;
> >>                          new_transfer();
> >>
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -                       if (tftp_mcast_active) { /* start!=1 common if
> mcast */
> >> -                               tftp_prev_block = tftp_cur_block - 1;
> >> -                       } else
> >> -#endif
> >>                          if (tftp_cur_block != 1) {      /* Assertion */
> >>                                  puts("\nTFTP error: ");
> >>                                  printf("First block is not block 1
> (%ld)\n",
> >> @@ -612,44 +533,8 @@ static void tftp_handler(uchar *pkt, unsigned
> dest, struct in_addr sip,
> >>                   *      Acknowledge the block just received, which
> will prompt
> >>                   *      the remote for the next one.
> >>                   */
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -               /* if I am the MasterClient, actively calculate what my
> next
> >> -                * needed block is; else I'm passive; not ACKING
> >> -                */
> >> -               if (tftp_mcast_active) {
> >> -                       if (len < tftp_block_size)  {
> >> -                               tftp_mcast_ending_block =
> tftp_cur_block;
> >> -                       } else if (tftp_mcast_master_client) {
> >> -                               tftp_mcast_prev_hole =
> ext2_find_next_zero_bit(
> >> -                                       tftp_mcast_bitmap,
> >> -                                       tftp_mcast_bitmap_size * 8,
> >> -                                       tftp_mcast_prev_hole);
> >> -                               tftp_cur_block = tftp_mcast_prev_hole;
> >> -                               if (tftp_cur_block >
> >> -                                   ((tftp_mcast_bitmap_size * 8) - 1))
> {
> >> -                                       debug("tftpfile too big\n");
> >> -                                       /* try to double it and retry */
> >> -                                       tftp_mcast_bitmap_size <<= 1;
> >> -                                       mcast_cleanup();
> >> -                                       net_start_again();
> >> -                                       return;
> >> -                               }
> >> -                               tftp_prev_block = tftp_cur_block;
> >> -                       }
> >> -               }
> >> -#endif
> >>                  tftp_send();
> >>
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -               if (tftp_mcast_active) {
> >> -                       if (tftp_mcast_master_client &&
> >> -                           (tftp_cur_block >=
> tftp_mcast_ending_block)) {
> >> -                               puts("\nMulticast tftp done\n");
> >> -                               mcast_cleanup();
> >> -                               net_set_state(NETLOOP_SUCCESS);
> >> -                       }
> >> -               } else
> >> -#endif
> >>                  if (len < tftp_block_size)
> >>                          tftp_complete();
> >>                  break;
> >> @@ -672,9 +557,6 @@ static void tftp_handler(uchar *pkt, unsigned dest,
> struct in_addr sip,
> >>                  case TFTP_ERR_FILE_ALREADY_EXISTS:
> >>                  default:
> >>                          puts("Starting again\n\n");
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -                       mcast_cleanup();
> >> -#endif
> >>                          net_start_again();
> >>                          break;
> >>                  }
> >> @@ -826,9 +708,6 @@ void tftp_start(enum proto_t protocol)
> >>          memset(net_server_ethaddr, 0, 6);
> >>          /* Revert tftp_block_size to dflt */
> >>          tftp_block_size = TFTP_BLOCK_SIZE;
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -       mcast_cleanup();
> >> -#endif
> >>   #ifdef CONFIG_TFTP_TSIZE
> >>          tftp_tsize = 0;
> >>          tftp_tsize_num_hash = 0;
> >> @@ -870,103 +749,3 @@ void tftp_start_server(void)
> >>          memset(net_server_ethaddr, 0, 6);
> >>   }
> >>   #endif /* CONFIG_CMD_TFTPSRV */
> >> -
> >> -#ifdef CONFIG_MCAST_TFTP
> >> -/*
> >> - * Credits: atftp project.
> >> - */
> >> -
> >> -/*
> >> - * Pick up BcastAddr, Port, and whether I am [now] the master-client.
> >> - * Frame:
> >> - *    +-------+-----------+---+-------~~-------+---+
> >> - *    |  opc  | multicast | 0 | addr, port, mc | 0 |
> >> - *    +-------+-----------+---+-------~~-------+---+
> >> - * The multicast addr/port becomes what I listen to, and if 'mc' is
> '1' then
> >> - * I am the new master-client so must send ACKs to DataBlocks.  If I
> am not
> >> - * master-client, I'm a passive client, gathering what DataBlocks I
> may and
> >> - * making note of which ones I got in my bitmask.
> >> - * In theory, I never go from master->passive..
> >> - * .. this comes in with pkt already pointing just past opc
> >> - */
> >> -static void parse_multicast_oack(char *pkt, int len)
> >> -{
> >> -       int i;
> >> -       struct in_addr addr;
> >> -       char *mc_adr;
> >> -       char *port;
> >> -       char *mc;
> >> -
> >> -       mc_adr = NULL;
> >> -       port = NULL;
> >> -       mc = NULL;
> >> -       /* march along looking for 'multicast\0', which has to start at
> least
> >> -        * 14 bytes back from the end.
> >> -        */
> >> -       for (i = 0; i < len - 14; i++)
> >> -               if (strcmp(pkt + i, "multicast") == 0)
> >> -                       break;
> >> -       if (i >= (len - 14)) /* non-Multicast OACK, ign. */
> >> -               return;
> >> -
> >> -       i += 10; /* strlen multicast */
> >> -       mc_adr = pkt + i;
> >> -       for (; i < len; i++) {
> >> -               if (*(pkt + i) == ',') {
> >> -                       *(pkt + i) = '\0';
> >> -                       if (port) {
> >> -                               mc = pkt + i + 1;
> >> -                               break;
> >> -                       } else {
> >> -                               port = pkt + i + 1;
> >> -                       }
> >> -               }
> >> -       }
> >> -       if (!port || !mc_adr || !mc)
> >> -               return;
> >> -       if (tftp_mcast_active && tftp_mcast_master_client) {
> >> -               printf("I got a OACK as master Client, WRONG!\n");
> >> -               return;
> >> -       }
> >> -       /* ..I now accept packets destined for this MCAST addr, port */
> >> -       if (!tftp_mcast_active) {
> >> -               if (tftp_mcast_bitmap) {
> >> -                       printf("Internal failure! no mcast.\n");
> >> -                       free(tftp_mcast_bitmap);
> >> -                       tftp_mcast_bitmap = NULL;
> >> -                       tftp_mcast_disabled = 1;
> >> -                       return;
> >> -               }
> >> -               /* I malloc instead of pre-declare; so that if the file
> ends
> >> -                * up being too big for this bitmap I can retry
> >> -                */
> >> -               tftp_mcast_bitmap = malloc(tftp_mcast_bitmap_size);
> >> -               if (!tftp_mcast_bitmap) {
> >> -                       printf("No bitmap, no multicast. Sorry.\n");
> >> -                       tftp_mcast_disabled = 1;
> >> -                       return;
> >> -               }
> >> -               memset(tftp_mcast_bitmap, 0, tftp_mcast_bitmap_size);
> >> -               tftp_mcast_prev_hole = 0;
> >> -               tftp_mcast_active = 1;
> >> -       }
> >> -       addr = string_to_ip(mc_adr);
> >> -       if (net_mcast_addr.s_addr != addr.s_addr) {
> >> -               if (net_mcast_addr.s_addr)
> >> -                       eth_mcast_join(net_mcast_addr, 0);
> >> -               net_mcast_addr = addr;
> >> -               if (eth_mcast_join(net_mcast_addr, 1)) {
> >> -                       printf("Fail to set mcast, revert to TFTP\n");
> >> -                       tftp_mcast_disabled = 1;
> >> -                       mcast_cleanup();
> >> -                       net_start_again();
> >> -               }
> >> -       }
> >> -       tftp_mcast_master_client = simple_strtoul((char *)mc, NULL, 10);
> >> -       tftp_mcast_port = (unsigned short)simple_strtoul(port, NULL,
> 10);
> >> -       printf("Multicast: %s:%d [%d]\n", mc_adr, tftp_mcast_port,
> >> -              tftp_mcast_master_client);
> >> -       return;
> >> -}
> >> -
> >> -#endif /* Multicast TFTP */
> >> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> >> index 0627024e71..b2691206fb 100644
> >> --- a/scripts/config_whitelist.txt
> >> +++ b/scripts/config_whitelist.txt
> >> @@ -1210,7 +1210,6 @@ CONFIG_MAX_FPGA_DEVICES
> >>   CONFIG_MAX_MEM_MAPPED
> >>   CONFIG_MAX_PKT
> >>   CONFIG_MAX_RAM_BANK_SIZE
> >> -CONFIG_MCAST_TFTP
> >>   CONFIG_MCF5249
> >>   CONFIG_MCF5253
> >>   CONFIG_MCFFEC
> >> --
> >> 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
>

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Glass]

Hi Simon,

On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> 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 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 (8):
>   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
>   net: remove CONFIG_MCAST_TFTP
>   tftp: prevent overwriting reserved memory
>
>  README                       |   9 --
>  common/bootm.c               |   8 +-
>  common/image-fdt.c           |  52 ++++++-
>  drivers/net/rtl8139.c        |   9 --
>  drivers/net/tsec.c           |  52 -------
>  drivers/usb/gadget/ether.c   |   3 -
>  fs/fs.c                      |  56 ++++++-
>  include/lmb.h                |   7 +-
>  include/net.h                |  17 ---
>  lib/lmb.c                    |  69 +++++++++
>  net/eth-uclass.c             |   4 -
>  net/eth_legacy.c             |  46 ------
>  net/net.c                    |   9 +-
>  net/tftp.c                   | 289 +++++++----------------------------
>  scripts/config_whitelist.txt |   1 -
>  15 files changed, 232 insertions(+), 399 deletions(-)

This is great work, but what is missing is a test for lmb.

Regards,
Simon

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Goldschmidt]

On Tue, Nov 27, 2018 at 2:02 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Simon,
>
> On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > 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 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 (8):
> >   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
> >   net: remove CONFIG_MCAST_TFTP
> >   tftp: prevent overwriting reserved memory
> >
> >  README                       |   9 --
> >  common/bootm.c               |   8 +-
> >  common/image-fdt.c           |  52 ++++++-
> >  drivers/net/rtl8139.c        |   9 --
> >  drivers/net/tsec.c           |  52 -------
> >  drivers/usb/gadget/ether.c   |   3 -
> >  fs/fs.c                      |  56 ++++++-
> >  include/lmb.h                |   7 +-
> >  include/net.h                |  17 ---
> >  lib/lmb.c                    |  69 +++++++++
> >  net/eth-uclass.c             |   4 -
> >  net/eth_legacy.c             |  46 ------
> >  net/net.c                    |   9 +-
> >  net/tftp.c                   | 289 +++++++----------------------------
> >  scripts/config_whitelist.txt |   1 -
> >  15 files changed, 232 insertions(+), 399 deletions(-)
>
> This is great work, but what is missing is a test for lmb.

Yeah, well, the tests didn't work on my system and I figured it's
better to get the code fixed than to use my time on trying to get the
tests running.

However, after searching for the required packages and fiddling around
some more, I guess I made them work so I could add tests now...

I also have work-in-progress for compressing fit image contents (we
currently only support uncompressing the kernel). It will switch some
'lmb_reserve' calls to the new 'lmb_alloc_addr' as this is more safe.
Maybe I can combine the tests in that series?

Regards,
Simon

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Goldschmidt]

Simon,

On Tue, Nov 27, 2018 at 6:45 AM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On Tue, Nov 27, 2018 at 2:02 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Simon,
> >
> > On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >
> > > 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 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 (8):
> > >   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
> > >   net: remove CONFIG_MCAST_TFTP
> > >   tftp: prevent overwriting reserved memory
> > >
> > >  README                       |   9 --
> > >  common/bootm.c               |   8 +-
> > >  common/image-fdt.c           |  52 ++++++-
> > >  drivers/net/rtl8139.c        |   9 --
> > >  drivers/net/tsec.c           |  52 -------
> > >  drivers/usb/gadget/ether.c   |   3 -
> > >  fs/fs.c                      |  56 ++++++-
> > >  include/lmb.h                |   7 +-
> > >  include/net.h                |  17 ---
> > >  lib/lmb.c                    |  69 +++++++++
> > >  net/eth-uclass.c             |   4 -
> > >  net/eth_legacy.c             |  46 ------
> > >  net/net.c                    |   9 +-
> > >  net/tftp.c                   | 289 +++++++----------------------------
> > >  scripts/config_whitelist.txt |   1 -
> > >  15 files changed, 232 insertions(+), 399 deletions(-)
> >
> > This is great work, but what is missing is a test for lmb.
>
> Yeah, well, the tests didn't work on my system and I figured it's
> better to get the code fixed than to use my time on trying to get the
> tests running.
>
> However, after searching for the required packages and fiddling around
> some more, I guess I made them work so I could add tests now...
>
> I also have work-in-progress for compressing fit image contents (we
> currently only support uncompressing the kernel). It will switch some
> 'lmb_reserve' calls to the new 'lmb_alloc_addr' as this is more safe.
> Maybe I can combine the tests in that series?

After managing to get the tests to run via 'make qcheck' (and 'make
tests'; had to install much more than listed in 'test/py/README.md'),
I tried to add tests to 'test/lib/' (next to hexdump.c), but I failed
to get them run. Even chaning 'test/lib/hexdump.c' to fail did not
produce errors. Are these tests not included in 'make qcheck'?

Regards,
Simon

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Glass]

Hi Simon,

On Mon, 3 Dec 2018 at 00:50, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> Simon,
>
> On Tue, Nov 27, 2018 at 6:45 AM Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > On Tue, Nov 27, 2018 at 2:02 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
> > > <simon.k.r.goldschmidt@gmail.com> wrote:
> > > >
> > > > 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 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 (8):
> > > >   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
> > > >   net: remove CONFIG_MCAST_TFTP
> > > >   tftp: prevent overwriting reserved memory
> > > >
> > > >  README                       |   9 --
> > > >  common/bootm.c               |   8 +-
> > > >  common/image-fdt.c           |  52 ++++++-
> > > >  drivers/net/rtl8139.c        |   9 --
> > > >  drivers/net/tsec.c           |  52 -------
> > > >  drivers/usb/gadget/ether.c   |   3 -
> > > >  fs/fs.c                      |  56 ++++++-
> > > >  include/lmb.h                |   7 +-
> > > >  include/net.h                |  17 ---
> > > >  lib/lmb.c                    |  69 +++++++++
> > > >  net/eth-uclass.c             |   4 -
> > > >  net/eth_legacy.c             |  46 ------
> > > >  net/net.c                    |   9 +-
> > > >  net/tftp.c                   | 289 +++++++----------------------------
> > > >  scripts/config_whitelist.txt |   1 -
> > > >  15 files changed, 232 insertions(+), 399 deletions(-)
> > >
> > > This is great work, but what is missing is a test for lmb.
> >
> > Yeah, well, the tests didn't work on my system and I figured it's
> > better to get the code fixed than to use my time on trying to get the
> > tests running.
> >
> > However, after searching for the required packages and fiddling around
> > some more, I guess I made them work so I could add tests now...
> >
> > I also have work-in-progress for compressing fit image contents (we
> > currently only support uncompressing the kernel). It will switch some
> > 'lmb_reserve' calls to the new 'lmb_alloc_addr' as this is more safe.
> > Maybe I can combine the tests in that series?
>
> After managing to get the tests to run via 'make qcheck' (and 'make
> tests'; had to install much more than listed in 'test/py/README.md'),
> I tried to add tests to 'test/lib/' (next to hexdump.c), but I failed
> to get them run. Even chaning 'test/lib/hexdump.c' to fail did not
> produce errors. Are these tests not included in 'make qcheck'?

That runs the Python tests which are in test/py/tests. Some of those
tests run compiled-in code (e.g. log_test.c and cmd_ut.c). Is your
test intended to be Python or C?
Regards,
Simon

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Goldschmidt]

Am Mo., 3. Dez. 2018, 19:20 hat Simon Glass <sjg@chromium.org> geschrieben:

> Hi Simon,
>
> On Mon, 3 Dec 2018 at 00:50, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > Simon,
> >
> > On Tue, Nov 27, 2018 at 6:45 AM Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >
> > > On Tue, Nov 27, 2018 at 2:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
> > > > <simon.k.r.goldschmidt@gmail.com> wrote:
> > > > >
> > > > > 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 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 (8):
> > > > >   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
> > > > >   net: remove CONFIG_MCAST_TFTP
> > > > >   tftp: prevent overwriting reserved memory
> > > > >
> > > > >  README                       |   9 --
> > > > >  common/bootm.c               |   8 +-
> > > > >  common/image-fdt.c           |  52 ++++++-
> > > > >  drivers/net/rtl8139.c        |   9 --
> > > > >  drivers/net/tsec.c           |  52 -------
> > > > >  drivers/usb/gadget/ether.c   |   3 -
> > > > >  fs/fs.c                      |  56 ++++++-
> > > > >  include/lmb.h                |   7 +-
> > > > >  include/net.h                |  17 ---
> > > > >  lib/lmb.c                    |  69 +++++++++
> > > > >  net/eth-uclass.c             |   4 -
> > > > >  net/eth_legacy.c             |  46 ------
> > > > >  net/net.c                    |   9 +-
> > > > >  net/tftp.c                   | 289
> +++++++----------------------------
> > > > >  scripts/config_whitelist.txt |   1 -
> > > > >  15 files changed, 232 insertions(+), 399 deletions(-)
> > > >
> > > > This is great work, but what is missing is a test for lmb.
> > >
> > > Yeah, well, the tests didn't work on my system and I figured it's
> > > better to get the code fixed than to use my time on trying to get the
> > > tests running.
> > >
> > > However, after searching for the required packages and fiddling around
> > > some more, I guess I made them work so I could add tests now...
> > >
> > > I also have work-in-progress for compressing fit image contents (we
> > > currently only support uncompressing the kernel). It will switch some
> > > 'lmb_reserve' calls to the new 'lmb_alloc_addr' as this is more safe.
> > > Maybe I can combine the tests in that series?
> >
> > After managing to get the tests to run via 'make qcheck' (and 'make
> > tests'; had to install much more than listed in 'test/py/README.md'),
> > I tried to add tests to 'test/lib/' (next to hexdump.c), but I failed
> > to get them run. Even chaning 'test/lib/hexdump.c' to fail did not
> > produce errors. Are these tests not included in 'make qcheck'?
>
> That runs the Python tests which are in test/py/tests. Some of those
> tests run compiled-in code (e.g. log_test.c and cmd_ut.c). Is your
> test intended to be Python or C?
>

I thought I'd create a unit test under test/lib that calls functions from
lmb.c, so that would be C code. Python would not work without adding extra
commands to call from Python.

There are tests in test/lib, how do I run them?

Regards,
Simon

>

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Glass]

Hi Simon,

On Mon, 3 Dec 2018 at 12:05, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
>
>
> Am Mo., 3. Dez. 2018, 19:20 hat Simon Glass <sjg@chromium.org> geschrieben:
>>
>> Hi Simon,
>>
>> On Mon, 3 Dec 2018 at 00:50, Simon Goldschmidt
>> <simon.k.r.goldschmidt@gmail.com> wrote:
>> >
>> > Simon,
>> >
>> > On Tue, Nov 27, 2018 at 6:45 AM Simon Goldschmidt
>> > <simon.k.r.goldschmidt@gmail.com> wrote:
>> > >
>> > > On Tue, Nov 27, 2018 at 2:02 AM Simon Glass <sjg@chromium.org> wrote:
>> > > >
>> > > > Hi Simon,
>> > > >
>> > > > On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
>> > > > <simon.k.r.goldschmidt@gmail.com> wrote:
>> > > > >
>> > > > > 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 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 (8):
>> > > > >   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
>> > > > >   net: remove CONFIG_MCAST_TFTP
>> > > > >   tftp: prevent overwriting reserved memory
>> > > > >
>> > > > >  README                       |   9 --
>> > > > >  common/bootm.c               |   8 +-
>> > > > >  common/image-fdt.c           |  52 ++++++-
>> > > > >  drivers/net/rtl8139.c        |   9 --
>> > > > >  drivers/net/tsec.c           |  52 -------
>> > > > >  drivers/usb/gadget/ether.c   |   3 -
>> > > > >  fs/fs.c                      |  56 ++++++-
>> > > > >  include/lmb.h                |   7 +-
>> > > > >  include/net.h                |  17 ---
>> > > > >  lib/lmb.c                    |  69 +++++++++
>> > > > >  net/eth-uclass.c             |   4 -
>> > > > >  net/eth_legacy.c             |  46 ------
>> > > > >  net/net.c                    |   9 +-
>> > > > >  net/tftp.c                   | 289 +++++++----------------------------
>> > > > >  scripts/config_whitelist.txt |   1 -
>> > > > >  15 files changed, 232 insertions(+), 399 deletions(-)
>> > > >
>> > > > This is great work, but what is missing is a test for lmb.
>> > >
>> > > Yeah, well, the tests didn't work on my system and I figured it's
>> > > better to get the code fixed than to use my time on trying to get the
>> > > tests running.
>> > >
>> > > However, after searching for the required packages and fiddling around
>> > > some more, I guess I made them work so I could add tests now...
>> > >
>> > > I also have work-in-progress for compressing fit image contents (we
>> > > currently only support uncompressing the kernel). It will switch some
>> > > 'lmb_reserve' calls to the new 'lmb_alloc_addr' as this is more safe.
>> > > Maybe I can combine the tests in that series?
>> >
>> > After managing to get the tests to run via 'make qcheck' (and 'make
>> > tests'; had to install much more than listed in 'test/py/README.md'),
>> > I tried to add tests to 'test/lib/' (next to hexdump.c), but I failed
>> > to get them run. Even chaning 'test/lib/hexdump.c' to fail did not
>> > produce errors. Are these tests not included in 'make qcheck'?
>>
>> That runs the Python tests which are in test/py/tests. Some of those
>> tests run compiled-in code (e.g. log_test.c and cmd_ut.c). Is your
>> test intended to be Python or C?
>
>
> I thought I'd create a unit test under test/lib that calls functions from lmb.c, so that would be C code. Python would not work without adding extra commands to call from Python.
>
> There are tests in test/lib, how do I run them?

I suspect it is lib/ since it is holds tests for library functions,
although hex_to_bin() is an inline function.

Better to put tests in another dir. Maybe test/image ?

You can run an individual test with something like:

/tmp/b/sandbox/u-boot  -d /tmp/b/sandbox/arch/sandbox/dts/test.dtb  -c
"ut dm lib_test_bin2hex"

where /tmp/b/sandbox is the build directory for sandbox.

Regards,
Simon

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Goldschmidt]

On Tue, Dec 4, 2018 at 12:45 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Simon,
>
> On Mon, 3 Dec 2018 at 12:05, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> >
> >
> > Am Mo., 3. Dez. 2018, 19:20 hat Simon Glass <sjg@chromium.org> geschrieben:
> >>
> >> Hi Simon,
> >>
> >> On Mon, 3 Dec 2018 at 00:50, Simon Goldschmidt
> >> <simon.k.r.goldschmidt@gmail.com> wrote:
> >> >
> >> > Simon,
> >> >
> >> > On Tue, Nov 27, 2018 at 6:45 AM Simon Goldschmidt
> >> > <simon.k.r.goldschmidt@gmail.com> wrote:
> >> > >
> >> > > On Tue, Nov 27, 2018 at 2:02 AM Simon Glass <sjg@chromium.org> wrote:
> >> > > >
> >> > > > Hi Simon,
> >> > > >
> >> > > > On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
> >> > > > <simon.k.r.goldschmidt@gmail.com> wrote:
> >> > > > >
> >> > > > > 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 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 (8):
> >> > > > >   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
> >> > > > >   net: remove CONFIG_MCAST_TFTP
> >> > > > >   tftp: prevent overwriting reserved memory
> >> > > > >
> >> > > > >  README                       |   9 --
> >> > > > >  common/bootm.c               |   8 +-
> >> > > > >  common/image-fdt.c           |  52 ++++++-
> >> > > > >  drivers/net/rtl8139.c        |   9 --
> >> > > > >  drivers/net/tsec.c           |  52 -------
> >> > > > >  drivers/usb/gadget/ether.c   |   3 -
> >> > > > >  fs/fs.c                      |  56 ++++++-
> >> > > > >  include/lmb.h                |   7 +-
> >> > > > >  include/net.h                |  17 ---
> >> > > > >  lib/lmb.c                    |  69 +++++++++
> >> > > > >  net/eth-uclass.c             |   4 -
> >> > > > >  net/eth_legacy.c             |  46 ------
> >> > > > >  net/net.c                    |   9 +-
> >> > > > >  net/tftp.c                   | 289 +++++++----------------------------
> >> > > > >  scripts/config_whitelist.txt |   1 -
> >> > > > >  15 files changed, 232 insertions(+), 399 deletions(-)
> >> > > >
> >> > > > This is great work, but what is missing is a test for lmb.
> >> > >
> >> > > Yeah, well, the tests didn't work on my system and I figured it's
> >> > > better to get the code fixed than to use my time on trying to get the
> >> > > tests running.
> >> > >
> >> > > However, after searching for the required packages and fiddling around
> >> > > some more, I guess I made them work so I could add tests now...
> >> > >
> >> > > I also have work-in-progress for compressing fit image contents (we
> >> > > currently only support uncompressing the kernel). It will switch some
> >> > > 'lmb_reserve' calls to the new 'lmb_alloc_addr' as this is more safe.
> >> > > Maybe I can combine the tests in that series?
> >> >
> >> > After managing to get the tests to run via 'make qcheck' (and 'make
> >> > tests'; had to install much more than listed in 'test/py/README.md'),
> >> > I tried to add tests to 'test/lib/' (next to hexdump.c), but I failed
> >> > to get them run. Even chaning 'test/lib/hexdump.c' to fail did not
> >> > produce errors. Are these tests not included in 'make qcheck'?
> >>
> >> That runs the Python tests which are in test/py/tests. Some of those
> >> tests run compiled-in code (e.g. log_test.c and cmd_ut.c). Is your
> >> test intended to be Python or C?
> >
> >
> > I thought I'd create a unit test under test/lib that calls functions from lmb.c, so that would be C code. Python would not work without adding extra commands to call from Python.
> >
> > There are tests in test/lib, how do I run them?
>
> I suspect it is lib/ since it is holds tests for library functions,
> although hex_to_bin() is an inline function.
>
> Better to put tests in another dir. Maybe test/image ?

Well, my tests would ensure lib/lmb.c works as expected (especially
for the corner case reported by Frank), so I though test/lib/ would be
good?

> You can run an individual test with something like:
>
> /tmp/b/sandbox/u-boot  -d /tmp/b/sandbox/arch/sandbox/dts/test.dtb  -c
> "ut dm lib_test_bin2hex"
>
> where /tmp/b/sandbox is the build directory for sandbox.

OK, that worked, thanks for the hint. Did I miss this in the
documentation somewhere?

And are these tests executed in a standard test run (e.g. on travis)?
If not, how would they be integrated?

Thanks for your patience with me :-)

Regards,
Simon

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Glass]

Hi Simon,

On Tue, 4 Dec 2018 at 04:54, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On Tue, Dec 4, 2018 at 12:45 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Simon,
> >
> > On Mon, 3 Dec 2018 at 12:05, Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >
> > >
> > >
> > > Am Mo., 3. Dez. 2018, 19:20 hat Simon Glass <sjg@chromium.org> geschrieben:
> > >>
> > >> Hi Simon,
> > >>
> > >> On Mon, 3 Dec 2018 at 00:50, Simon Goldschmidt
> > >> <simon.k.r.goldschmidt@gmail.com> wrote:
> > >> >
> > >> > Simon,
> > >> >
> > >> > On Tue, Nov 27, 2018 at 6:45 AM Simon Goldschmidt
> > >> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >> > >
> > >> > > On Tue, Nov 27, 2018 at 2:02 AM Simon Glass <sjg@chromium.org> wrote:
> > >> > > >
> > >> > > > Hi Simon,
> > >> > > >
> > >> > > > On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
> > >> > > > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >> > > > >
> > >> > > > > 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 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 (8):
> > >> > > > >   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
> > >> > > > >   net: remove CONFIG_MCAST_TFTP
> > >> > > > >   tftp: prevent overwriting reserved memory
> > >> > > > >
> > >> > > > >  README                       |   9 --
> > >> > > > >  common/bootm.c               |   8 +-
> > >> > > > >  common/image-fdt.c           |  52 ++++++-
> > >> > > > >  drivers/net/rtl8139.c        |   9 --
> > >> > > > >  drivers/net/tsec.c           |  52 -------
> > >> > > > >  drivers/usb/gadget/ether.c   |   3 -
> > >> > > > >  fs/fs.c                      |  56 ++++++-
> > >> > > > >  include/lmb.h                |   7 +-
> > >> > > > >  include/net.h                |  17 ---
> > >> > > > >  lib/lmb.c                    |  69 +++++++++
> > >> > > > >  net/eth-uclass.c             |   4 -
> > >> > > > >  net/eth_legacy.c             |  46 ------
> > >> > > > >  net/net.c                    |   9 +-
> > >> > > > >  net/tftp.c                   | 289 +++++++----------------------------
> > >> > > > >  scripts/config_whitelist.txt |   1 -
> > >> > > > >  15 files changed, 232 insertions(+), 399 deletions(-)
> > >> > > >
> > >> > > > This is great work, but what is missing is a test for lmb.
> > >> > >
> > >> > > Yeah, well, the tests didn't work on my system and I figured it's
> > >> > > better to get the code fixed than to use my time on trying to get the
> > >> > > tests running.
> > >> > >
> > >> > > However, after searching for the required packages and fiddling around
> > >> > > some more, I guess I made them work so I could add tests now...
> > >> > >
> > >> > > I also have work-in-progress for compressing fit image contents (we
> > >> > > currently only support uncompressing the kernel). It will switch some
> > >> > > 'lmb_reserve' calls to the new 'lmb_alloc_addr' as this is more safe.
> > >> > > Maybe I can combine the tests in that series?
> > >> >
> > >> > After managing to get the tests to run via 'make qcheck' (and 'make
> > >> > tests'; had to install much more than listed in 'test/py/README.md'),
> > >> > I tried to add tests to 'test/lib/' (next to hexdump.c), but I failed
> > >> > to get them run. Even chaning 'test/lib/hexdump.c' to fail did not
> > >> > produce errors. Are these tests not included in 'make qcheck'?
> > >>
> > >> That runs the Python tests which are in test/py/tests. Some of those
> > >> tests run compiled-in code (e.g. log_test.c and cmd_ut.c). Is your
> > >> test intended to be Python or C?
> > >
> > >
> > > I thought I'd create a unit test under test/lib that calls functions from lmb.c, so that would be C code. Python would not work without adding extra commands to call from Python.
> > >
> > > There are tests in test/lib, how do I run them?
> >
> > I suspect it is lib/ since it is holds tests for library functions,
> > although hex_to_bin() is an inline function.
> >
> > Better to put tests in another dir. Maybe test/image ?
>
> Well, my tests would ensure lib/lmb.c works as expected (especially
> for the corner case reported by Frank), so I though test/lib/ would be
> good?

I suppose so.

>
> > You can run an individual test with something like:
> >
> > /tmp/b/sandbox/u-boot  -d /tmp/b/sandbox/arch/sandbox/dts/test.dtb  -c
> > "ut dm lib_test_bin2hex"
> >
> > where /tmp/b/sandbox is the build directory for sandbox.
>
> OK, that worked, thanks for the hint. Did I miss this in the
> documentation somewhere?

No, only the help from the 'ut' command. But you could add a patch to
test/README perhaps?

>
> And are these tests executed in a standard test run (e.g. on travis)?
> If not, how would they be integrated?

So long as they run with 'make check' (or 'make qcheck') then you should be OK.

>
> Thanks for your patience with me :-)
>
> Regards,
> Simon

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Goldschmidt]

On Wed, Dec 5, 2018 at 2:13 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Simon,
>
> On Tue, 4 Dec 2018 at 04:54, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > On Tue, Dec 4, 2018 at 12:45 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Mon, 3 Dec 2018 at 12:05, Simon Goldschmidt
> > > <simon.k.r.goldschmidt@gmail.com> wrote:
> > > >
> > > >
> > > >
> > > > Am Mo., 3. Dez. 2018, 19:20 hat Simon Glass <sjg@chromium.org> geschrieben:
> > > >>
> > > >> Hi Simon,
> > > >>
> > > >> On Mon, 3 Dec 2018 at 00:50, Simon Goldschmidt
> > > >> <simon.k.r.goldschmidt@gmail.com> wrote:
> > > >> >
> > > >> > Simon,
> > > >> >
> > > >> > On Tue, Nov 27, 2018 at 6:45 AM Simon Goldschmidt
> > > >> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > > >> > >
> > > >> > > On Tue, Nov 27, 2018 at 2:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > >> > > >
> > > >> > > > Hi Simon,
> > > >> > > >
> > > >> > > > On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
> > > >> > > > <simon.k.r.goldschmidt@gmail.com> wrote:
> > > >> > > > >
> > > >> > > > > 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 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 (8):
> > > >> > > > >   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
> > > >> > > > >   net: remove CONFIG_MCAST_TFTP
> > > >> > > > >   tftp: prevent overwriting reserved memory
> > > >> > > > >
> > > >> > > > >  README                       |   9 --
> > > >> > > > >  common/bootm.c               |   8 +-
> > > >> > > > >  common/image-fdt.c           |  52 ++++++-
> > > >> > > > >  drivers/net/rtl8139.c        |   9 --
> > > >> > > > >  drivers/net/tsec.c           |  52 -------
> > > >> > > > >  drivers/usb/gadget/ether.c   |   3 -
> > > >> > > > >  fs/fs.c                      |  56 ++++++-
> > > >> > > > >  include/lmb.h                |   7 +-
> > > >> > > > >  include/net.h                |  17 ---
> > > >> > > > >  lib/lmb.c                    |  69 +++++++++
> > > >> > > > >  net/eth-uclass.c             |   4 -
> > > >> > > > >  net/eth_legacy.c             |  46 ------
> > > >> > > > >  net/net.c                    |   9 +-
> > > >> > > > >  net/tftp.c                   | 289 +++++++----------------------------
> > > >> > > > >  scripts/config_whitelist.txt |   1 -
> > > >> > > > >  15 files changed, 232 insertions(+), 399 deletions(-)
> > > >> > > >
> > > >> > > > This is great work, but what is missing is a test for lmb.
> > > >> > >
> > > >> > > Yeah, well, the tests didn't work on my system and I figured it's
> > > >> > > better to get the code fixed than to use my time on trying to get the
> > > >> > > tests running.
> > > >> > >
> > > >> > > However, after searching for the required packages and fiddling around
> > > >> > > some more, I guess I made them work so I could add tests now...
> > > >> > >
> > > >> > > I also have work-in-progress for compressing fit image contents (we
> > > >> > > currently only support uncompressing the kernel). It will switch some
> > > >> > > 'lmb_reserve' calls to the new 'lmb_alloc_addr' as this is more safe.
> > > >> > > Maybe I can combine the tests in that series?
> > > >> >
> > > >> > After managing to get the tests to run via 'make qcheck' (and 'make
> > > >> > tests'; had to install much more than listed in 'test/py/README.md'),
> > > >> > I tried to add tests to 'test/lib/' (next to hexdump.c), but I failed
> > > >> > to get them run. Even chaning 'test/lib/hexdump.c' to fail did not
> > > >> > produce errors. Are these tests not included in 'make qcheck'?
> > > >>
> > > >> That runs the Python tests which are in test/py/tests. Some of those
> > > >> tests run compiled-in code (e.g. log_test.c and cmd_ut.c). Is your
> > > >> test intended to be Python or C?
> > > >
> > > >
> > > > I thought I'd create a unit test under test/lib that calls functions from lmb.c, so that would be C code. Python would not work without adding extra commands to call from Python.
> > > >
> > > > There are tests in test/lib, how do I run them?
> > >
> > > I suspect it is lib/ since it is holds tests for library functions,
> > > although hex_to_bin() is an inline function.
> > >
> > > Better to put tests in another dir. Maybe test/image ?
> >
> > Well, my tests would ensure lib/lmb.c works as expected (especially
> > for the corner case reported by Frank), so I though test/lib/ would be
> > good?
>
> I suppose so.
>
> >
> > > You can run an individual test with something like:
> > >
> > > /tmp/b/sandbox/u-boot  -d /tmp/b/sandbox/arch/sandbox/dts/test.dtb  -c
> > > "ut dm lib_test_bin2hex"
> > >
> > > where /tmp/b/sandbox is the build directory for sandbox.
> >
> > OK, that worked, thanks for the hint. Did I miss this in the
> > documentation somewhere?
>
> No, only the help from the 'ut' command. But you could add a patch to
> test/README perhaps?

Once I have understood what's missing, I could do that ;-)

> > And are these tests executed in a standard test run (e.g. on travis)?
> > If not, how would they be integrated?
>
> So long as they run with 'make check' (or 'make qcheck') then you should be OK.

Since I did not get 'make check' to report a failure when changing
test/lib/hexdump.c to fail, I don't think these tests are run on 'make
check'. Where exactly is it defined which tests are run?

Regards,
Simon

[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439

[Simon Glass]

Hi Simon,

On Wed, 5 Dec 2018 at 06:16, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On Wed, Dec 5, 2018 at 2:13 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Simon,
> >
> > On Tue, 4 Dec 2018 at 04:54, Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >
> > > On Tue, Dec 4, 2018 at 12:45 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Mon, 3 Dec 2018 at 12:05, Simon Goldschmidt
> > > > <simon.k.r.goldschmidt@gmail.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > Am Mo., 3. Dez. 2018, 19:20 hat Simon Glass <sjg@chromium.org> geschrieben:
> > > > >>
> > > > >> Hi Simon,
> > > > >>
> > > > >> On Mon, 3 Dec 2018 at 00:50, Simon Goldschmidt
> > > > >> <simon.k.r.goldschmidt@gmail.com> wrote:
> > > > >> >
> > > > >> > Simon,
> > > > >> >
> > > > >> > On Tue, Nov 27, 2018 at 6:45 AM Simon Goldschmidt
> > > > >> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > > > >> > >
> > > > >> > > On Tue, Nov 27, 2018 at 2:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > > >> > > >
> > > > >> > > > Hi Simon,
> > > > >> > > >
> > > > >> > > > On Sat, 17 Nov 2018 at 05:25, Simon Goldschmidt
> > > > >> > > > <simon.k.r.goldschmidt@gmail.com> wrote:
> > > > >> > > > >
> > > > >> > > > > 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 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 (8):
> > > > >> > > > >   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
> > > > >> > > > >   net: remove CONFIG_MCAST_TFTP
> > > > >> > > > >   tftp: prevent overwriting reserved memory
> > > > >> > > > >
> > > > >> > > > >  README                       |   9 --
> > > > >> > > > >  common/bootm.c               |   8 +-
> > > > >> > > > >  common/image-fdt.c           |  52 ++++++-
> > > > >> > > > >  drivers/net/rtl8139.c        |   9 --
> > > > >> > > > >  drivers/net/tsec.c           |  52 -------
> > > > >> > > > >  drivers/usb/gadget/ether.c   |   3 -
> > > > >> > > > >  fs/fs.c                      |  56 ++++++-
> > > > >> > > > >  include/lmb.h                |   7 +-
> > > > >> > > > >  include/net.h                |  17 ---
> > > > >> > > > >  lib/lmb.c                    |  69 +++++++++
> > > > >> > > > >  net/eth-uclass.c             |   4 -
> > > > >> > > > >  net/eth_legacy.c             |  46 ------
> > > > >> > > > >  net/net.c                    |   9 +-
> > > > >> > > > >  net/tftp.c                   | 289 +++++++----------------------------
> > > > >> > > > >  scripts/config_whitelist.txt |   1 -
> > > > >> > > > >  15 files changed, 232 insertions(+), 399 deletions(-)
> > > > >> > > >
> > > > >> > > > This is great work, but what is missing is a test for lmb.
> > > > >> > >
> > > > >> > > Yeah, well, the tests didn't work on my system and I figured it's
> > > > >> > > better to get the code fixed than to use my time on trying to get the
> > > > >> > > tests running.
> > > > >> > >
> > > > >> > > However, after searching for the required packages and fiddling around
> > > > >> > > some more, I guess I made them work so I could add tests now...
> > > > >> > >
> > > > >> > > I also have work-in-progress for compressing fit image contents (we
> > > > >> > > currently only support uncompressing the kernel). It will switch some
> > > > >> > > 'lmb_reserve' calls to the new 'lmb_alloc_addr' as this is more safe.
> > > > >> > > Maybe I can combine the tests in that series?
> > > > >> >
> > > > >> > After managing to get the tests to run via 'make qcheck' (and 'make
> > > > >> > tests'; had to install much more than listed in 'test/py/README.md'),
> > > > >> > I tried to add tests to 'test/lib/' (next to hexdump.c), but I failed
> > > > >> > to get them run. Even chaning 'test/lib/hexdump.c' to fail did not
> > > > >> > produce errors. Are these tests not included in 'make qcheck'?
> > > > >>
> > > > >> That runs the Python tests which are in test/py/tests. Some of those
> > > > >> tests run compiled-in code (e.g. log_test.c and cmd_ut.c). Is your
> > > > >> test intended to be Python or C?
> > > > >
> > > > >
> > > > > I thought I'd create a unit test under test/lib that calls functions from lmb.c, so that would be C code. Python would not work without adding extra commands to call from Python.
> > > > >
> > > > > There are tests in test/lib, how do I run them?
> > > >
> > > > I suspect it is lib/ since it is holds tests for library functions,
> > > > although hex_to_bin() is an inline function.
> > > >
> > > > Better to put tests in another dir. Maybe test/image ?
> > >
> > > Well, my tests would ensure lib/lmb.c works as expected (especially
> > > for the corner case reported by Frank), so I though test/lib/ would be
> > > good?
> >
> > I suppose so.
> >
> > >
> > > > You can run an individual test with something like:
> > > >
> > > > /tmp/b/sandbox/u-boot  -d /tmp/b/sandbox/arch/sandbox/dts/test.dtb  -c
> > > > "ut dm lib_test_bin2hex"
> > > >
> > > > where /tmp/b/sandbox is the build directory for sandbox.
> > >
> > > OK, that worked, thanks for the hint. Did I miss this in the
> > > documentation somewhere?
> >
> > No, only the help from the 'ut' command. But you could add a patch to
> > test/README perhaps?
>
> Once I have understood what's missing, I could do that ;-)
>
> > > And are these tests executed in a standard test run (e.g. on travis)?
> > > If not, how would they be integrated?
> >
> > So long as they run with 'make check' (or 'make qcheck') then you should be OK.
>
> Since I did not get 'make check' to report a failure when changing
> test/lib/hexdump.c to fail, I don't think these tests are run on 'make
> check'. Where exactly is it defined which tests are run?

See generate_ut_subtest() for the logic.

I actually think this is a bit of a widespread problem. E.g. the
bloblist tests don't run.

Regards,
Simon