On Mon, Nov 26, 2018 at 9:00 AM Chris Packham <judge.pack...@gmail.com> wrote: > > No mainline board enables CONFIG_MCAST_TFTP and there have been > compilation issues with the code for some time. Additionally, it has a > potential buffer underrun issue (reported as a side note in > CVE-2018-18439). > > Remove the multicast TFTP code but keep the driver API for the future > addition of IPv6.
Thanks for taking this. I expect that there will be merge conflicts between this patch any my patch fixing CVE-2018-18439. Joe, how should we proceed? I can rebase my series on top of this if you want. Regards, Simon > > Cc: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com> > Signed-off-by: Chris Packham <judge.pack...@gmail.com> > --- > As suggested by Joe[1] I've make the mcast API unconditional. So there > is a small increase in binary size with this patch. > > Currently there are only two drivers that implement the API but > eventually that'll need to grow when IPv6 is implemented. > > -- > [1] - http://patchwork.ozlabs.org/patch/999307/#2031941 > > README | 9 -- > drivers/net/rtl8139.c | 6 +- > drivers/net/tsec.c | 14 +-- > drivers/usb/gadget/ether.c | 3 - > include/net.h | 14 +-- > net/eth-uclass.c | 2 - > net/eth_legacy.c | 4 - > net/net.c | 7 -- > net/tftp.c | 218 ----------------------------------- > scripts/config_whitelist.txt | 1 - > 10 files changed, 6 insertions(+), 272 deletions(-) > > diff --git a/README b/README > index a46c7c63a4fe..97a3e9a84b3f 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 590f8ce15426..13309970e2c4 100644 > --- a/drivers/net/rtl8139.c > +++ b/drivers/net/rtl8139.c > @@ -183,12 +183,10 @@ 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) > +static int rtl_bcast_addr(struct eth_device *dev, const u8 *bcast_mac, int > join) > { > return (0); > } > -#endif > > static struct pci_device_id supported[] = { > {PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8139}, > @@ -229,9 +227,7 @@ 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 9a4fab85e928..06a9b4fb03ce 100644 > --- a/drivers/net/tsec.c > +++ b/drivers/net/tsec.c > @@ -78,8 +78,6 @@ static void tsec_configure_serdes(struct tsec_private *priv) > 0, TBI_CR, CONFIG_TSEC_TBICR_SETTINGS); > } > > -#ifdef CONFIG_MCAST_TFTP > - > /* 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 > @@ -124,9 +122,10 @@ static u32 ether_crc(size_t len, unsigned char const *p) > * the entry. > */ > #ifndef CONFIG_DM_ETH > -static int tsec_mcast_addr(struct eth_device *dev, const u8 *mcast_mac, u8 > set) > +static int tsec_mcast_addr(struct eth_device *dev, const u8 *mcast_mac, > + int join) > #else > -static int tsec_mcast_addr(struct udevice *dev, const u8 *mcast_mac, int set) > +static int tsec_mcast_addr(struct udevice *dev, const u8 *mcast_mac, int > join) > #endif > { > struct tsec_private *priv = (struct tsec_private *)dev->priv; > @@ -140,14 +139,13 @@ static int tsec_mcast_addr(struct udevice *dev, const > u8 *mcast_mac, int set) > > value = BIT(31 - whichbit); > > - if (set) > + if (join) > 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 > @@ -745,9 +743,7 @@ 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++) > @@ -887,9 +883,7 @@ 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 90ef1f055f76..41fe41e1a605 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 359bfb5ef69f..dd52ed3f476c 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -140,9 +140,7 @@ 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 +173,7 @@ 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 (*mcast)(struct eth_device *, const u8 *enetaddr, int join); > int (*write_hwaddr)(struct eth_device *); > struct eth_device *next; > int index; > @@ -286,11 +282,7 @@ extern void (*push_packet)(void *packet, int length); > 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); > -#endif > - > > /**********************************************************************/ > /* > @@ -577,10 +569,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 91d861be4136..2ef20df19203 100644 > --- a/net/eth-uclass.c > +++ b/net/eth-uclass.c > @@ -476,10 +476,8 @@ 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 d2e16b8fa3da..e250a430f333 100644 > --- a/net/eth_legacy.c > +++ b/net/eth_legacy.c > @@ -291,7 +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. > @@ -310,9 +309,6 @@ int eth_mcast_join(struct in_addr mcast_ip, int join) > return eth_current->mcast(eth_current, mcast_mac, join); > } > > -#endif > - > - > int eth_init(void) > { > struct eth_device *old_current; > diff --git a/net/net.c b/net/net.c > index a5a216c3eeec..a84fbdc7993b 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,9 +1211,6 @@ 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; > } > /* Read source IP address for later use */ > diff --git a/net/tftp.c b/net/tftp.c > index 68ffd814146c..cf744c513494 100644 > --- a/net/tftp.c > +++ b/net/tftp.c > @@ -134,35 +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) > { > @@ -196,10 +167,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 +242,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 +296,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,30 +330,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: > @@ -465,10 +403,6 @@ 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; > } > if (tftp_state != STATE_SEND_RRQ && src != tftp_remote_port && > @@ -549,12 +483,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 +510,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 +535,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 +559,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 +710,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; > @@ -871,102 +752,3 @@ void tftp_start_server(void) > } > #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 abfb0ff89fa9..95ab388cae21 100644 > --- a/scripts/config_whitelist.txt > +++ b/scripts/config_whitelist.txt > @@ -1209,7 +1209,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.19.2 > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
