[U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP
Simon Goldschmidt
simon.k.r.goldschmidt at gmail.com
Mon Nov 26 08:15:14 UTC 2018
On Mon, Nov 26, 2018 at 9:00 AM Chris Packham <judge.packham at 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.goldschmidt at gmail.com>
> Signed-off-by: Chris Packham <judge.packham at 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
>
More information about the U-Boot
mailing list