Re: [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP

On Mon, Nov 26, 2018 at 9:00 AM Chris Packham judge.packham@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@gmail.com Signed-off-by: Chris Packham judge.packham@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(&regs->hash.gaddr0 + whichreg, value);
    else
            clrbits_be32(&regs->hash.gaddr0 + whichreg, value);
  
    return 0;
  

} -#endif /* Multicast TFTP ? */

/*

• Initialized required registers to appropriate values, zeroing

@@ -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