[U-Boot] [PATCH v3 0/8] Fix CVE-2018-18440 and CVE-2018-18439
This series fixes CVE-2018-18440 ("insufficient boundary checks in filesystem image load") by adding restrictions to the 'load' command and fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by adding restrictions to the tftp code.
The functions from lmb.c are used to setup regions of allowed and reserved memory. Then, the file size to load is checked against these addresses and loading the file is aborted if it would overwrite reserved memory.
The memory reservation code is reused from bootm/image.
Changes in 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(-)
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; } }
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); + } } }
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;
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) {
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)
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);
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(®s->hash.gaddr0 + whichreg, value); - else - clrbits_be32(®s->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
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_TFTPDefines whether you want to support multicast TFTP as perrfc-2090; for example to work with atftp. Lets lots of targetstftp down the same boot image concurrently. Note: the Ethernetdriver in use must provide a function: mcast() to join/leave amulticast 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:
- Take the Destination Address (ie the multicast address), and
- do a CRC on it (little endian), and reverse the bits of the
- result.
- 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
- 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(®s->hash.gaddr0 + whichreg, value);elseclrbits_be32(®s->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 theeth_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@lists.denx.de https://lists.denx.de/listinfo/u-boot
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_TFTPDefines whether you want to support multicast TFTP as perrfc-2090; for example to work with atftp. Lets lots of targetstftp down the same boot image concurrently. Note: the Ethernetdriver in use must provide a function: mcast() to join/leave amulticast 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:
- Take the Destination Address (ie the multicast address), and
- do a CRC on it (little endian), and reverse the bits of the
- result.
- 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
- 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(®s->hash.gaddr0 + whichreg, value);elseclrbits_be32(®s->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 theeth_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@lists.denx.de https://lists.denx.de/listinfo/u-boot
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_TFTPDefines whether you want to support multicast TFTP asper
rfc-2090; for example to work with atftp. Lets lots oftargets
tftp down the same boot image concurrently. Note: theEthernet
driver in use must provide a function: mcast() tojoin/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:
- Take the Destination Address (ie the multicast address), and
- do a CRC on it (little endian), and reverse the bits of the
- result.
- 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
- 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 toset */
whichreg = result >> 29; /* the 3 MSB = which reg to set it in*/
value = BIT(31 - whichbit);if (set)setbits_be32(®s->hash.gaddr0 + whichreg, value);elseclrbits_be32(®s->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 theeth_pdata::enetaddr field to be populated. The methodcan
@@ -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 theoption */
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 ifmcast */
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, whichwill prompt
* the remote for the next one. */-#ifdef CONFIG_MCAST_TFTP
/* if I am the MasterClient, actively calculate what mynext
* 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 atleast
* 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 fileends
* 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@lists.denx.de https://lists.denx.de/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
This fixes CVE-2018-18439 ("insufficient boundary checks in network image boot") by using lmb to check for a valid range to store received blocks.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
Changes in v2: - this patch is new in v2
net/tftp.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 9 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c index 563ce3a06f..390394199d 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -17,6 +17,8 @@ #include <flash.h> #endif
+DECLARE_GLOBAL_DATA_PTR; + /* Well known TFTP port # */ #define WELL_KNOWN_PORT 69 /* Millisecs to timeout for lost pkt */ @@ -81,6 +83,8 @@ static ulong tftp_block_wrap; /* memory offset due to wrapping */ static ulong tftp_block_wrap_offset; static int tftp_state; +static ulong tftp_load_addr; +static ulong tftp_load_size; #ifdef CONFIG_TFTP_TSIZE /* The file size reported by the server */ static int tftp_tsize; @@ -134,10 +138,11 @@ static char tftp_filename[MAX_LEN]; static unsigned short tftp_block_size = TFTP_BLOCK_SIZE; static unsigned short tftp_block_size_option = TFTP_MTU_BLOCKSIZE;
-static inline void store_block(int block, uchar *src, unsigned len) +static inline int store_block(int block, uchar *src, unsigned int len) { ulong offset = block * tftp_block_size + tftp_block_wrap_offset; ulong newsize = offset + len; + ulong store_addr = tftp_load_addr + offset; #ifdef CONFIG_SYS_DIRECT_FLASH_TFTP int i, rc = 0;
@@ -145,30 +150,38 @@ static inline void store_block(int block, uchar *src, unsigned len) /* start address in flash? */ if (flash_info[i].flash_id == FLASH_UNKNOWN) continue; - if (load_addr + offset >= flash_info[i].start[0]) { + if (store_addr >= flash_info[i].start[0]) { rc = 1; break; } }
if (rc) { /* Flash is destination for this packet */ - rc = flash_write((char *)src, (ulong)(load_addr+offset), len); + rc = flash_write((char *)src, store_addr, len); if (rc) { flash_perror(rc); - net_set_state(NETLOOP_FAIL); - return; + return rc; } } else #endif /* CONFIG_SYS_DIRECT_FLASH_TFTP */ { - void *ptr = map_sysmem(load_addr + offset, len); + void *ptr;
+ if (store_addr < tftp_load_addr || + store_addr + len > tftp_load_addr + tftp_load_size) { + puts("\nTFTP error: "); + puts("trying to overwrite reserved memory...\n"); + return -1; + } + ptr = map_sysmem(store_addr, len); memcpy(ptr, src, len); unmap_sysmem(ptr); }
if (net_boot_file_size < newsize) net_boot_file_size = newsize; + + return 0; }
/* Clear our state ready for a new transfer */ @@ -527,7 +540,11 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip, timeout_count_max = tftp_timeout_count_max; net_set_timeout_handler(timeout_ms, tftp_timeout_handler);
- store_block(tftp_cur_block - 1, pkt + 2, len); + if (store_block(tftp_cur_block - 1, pkt + 2, len)) { + eth_halt(); + net_set_state(NETLOOP_FAIL); + break; + }
/* * Acknowledge the block just received, which will prompt @@ -577,6 +594,24 @@ static void tftp_timeout_handler(void) } }
+/* Initialize tftp_load_addr and tftp_load_size from load_addr and lmb */ +static int tftp_init_load_addr(void) +{ + struct lmb lmb; + phys_size_t max_size; + + tftp_load_addr = load_addr; + + lmb_init_and_reserve(&lmb, gd->bd->bi_dram[0].start, + gd->bd->bi_dram[0].size, (void *)gd->fdt_blob); + + max_size = lmb_get_unreserved_size(&lmb, tftp_load_addr); + if (!max_size) + return -1; + + tftp_load_size = max_size; + return 0; +}
void tftp_start(enum proto_t protocol) { @@ -673,7 +708,14 @@ void tftp_start(enum proto_t protocol) } else #endif { - printf("Load address: 0x%lx\n", load_addr); + if (tftp_init_load_addr()) { + eth_halt(); + net_set_state(NETLOOP_FAIL); + puts("\nTFTP error: "); + puts("trying to overwrite reserved memory...\n"); + return; + } + printf("Load address: 0x%lx\n", tftp_load_addr); puts("Loading: *\b"); tftp_state = STATE_SEND_RRQ; #ifdef CONFIG_CMD_BOOTEFI @@ -721,9 +763,15 @@ void tftp_start_server(void) { tftp_filename[0] = 0;
+ if (tftp_init_load_addr()) { + eth_halt(); + net_set_state(NETLOOP_FAIL); + puts("\nTFTP error: trying to overwrite reserved memory...\n"); + return; + } printf("Using %s device\n", eth_get_name()); printf("Listening for TFTP transfer on %pI4\n", &net_ip); - printf("Load address: 0x%lx\n", load_addr); + printf("Load address: 0x%lx\n", tftp_load_addr);
puts("Loading: *\b");
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
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
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
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
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
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
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
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
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
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
participants (4)
-
Chris Packham -
Joe Hershberger -
Simon Glass -
Simon Goldschmidt