All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver
@ 2018-11-26  8:00 Chris Packham
  2018-11-26  8:00 ` [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP Chris Packham
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Chris Packham @ 2018-11-26  8:00 UTC (permalink / raw)
  To: u-boot

ether_crc was added to the core net code in commit 53a5c424bf86
("multicast tftp: RFC2090") so that other drivers could use it. However
the only current user of it is tsec.c so move it there.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---

 drivers/net/tsec.c | 25 +++++++++++++++++++++++++
 include/net.h      |  1 -
 net/eth_legacy.c   | 24 ------------------------
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index 03a46da2f8a1..9a4fab85e928 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -80,6 +80,31 @@ static void tsec_configure_serdes(struct tsec_private *priv)
 
 #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
+ */
+#define CRCPOLY_LE 0xedb88320
+static 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;
+}
+
 /* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
 
 /* Set the appropriate hash bit for the given addr */
diff --git a/include/net.h b/include/net.h
index 51c099dae2e5..359bfb5ef69f 100644
--- a/include/net.h
+++ b/include/net.h
@@ -289,7 +289,6 @@ 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
 
 
diff --git a/net/eth_legacy.c b/net/eth_legacy.c
index 2a9caa3509b0..d2e16b8fa3da 100644
--- a/net/eth_legacy.c
+++ b/net/eth_legacy.c
@@ -310,30 +310,6 @@ int eth_mcast_join(struct in_addr mcast_ip, int join)
 	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
 
 
-- 
2.19.2

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP
  2018-11-26  8:00 [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver Chris Packham
@ 2018-11-26  8:00 ` Chris Packham
  2018-11-26  8:15   ` Simon Goldschmidt
  2019-01-24 17:39   ` [U-Boot] " Joe Hershberger
  2018-11-26  8:12 ` [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver Simon Goldschmidt
  2019-01-24 17:38 ` [U-Boot] " Joe Hershberger
  2 siblings, 2 replies; 12+ messages in thread
From: Chris Packham @ 2018-11-26  8:00 UTC (permalink / raw)
  To: u-boot

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.

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver
  2018-11-26  8:00 [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver Chris Packham
  2018-11-26  8:00 ` [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP Chris Packham
@ 2018-11-26  8:12 ` Simon Goldschmidt
  2018-11-27  3:19   ` Chris Packham
  2019-01-24 17:38 ` [U-Boot] " Joe Hershberger
  2 siblings, 1 reply; 12+ messages in thread
From: Simon Goldschmidt @ 2018-11-26  8:12 UTC (permalink / raw)
  To: u-boot

On Mon, Nov 26, 2018 at 9:00 AM Chris Packham <judge.packham@gmail.com> wrote:
>
> ether_crc was added to the core net code in commit 53a5c424bf86
> ("multicast tftp: RFC2090") so that other drivers could use it. However
> the only current user of it is tsec.c so move it there.
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
>
>  drivers/net/tsec.c | 25 +++++++++++++++++++++++++
>  include/net.h      |  1 -
>  net/eth_legacy.c   | 24 ------------------------
>  3 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> index 03a46da2f8a1..9a4fab85e928 100644
> --- a/drivers/net/tsec.c
> +++ b/drivers/net/tsec.c
> @@ -80,6 +80,31 @@ static void tsec_configure_serdes(struct tsec_private *priv)
>
>  #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
> + */
> +#define CRCPOLY_LE 0xedb88320
> +static u32 ether_crc(size_t len, unsigned char const *p)

I haven't checked, but can't we use lib/crc32.c for this? The
polynomial is the same...

> +{
> +       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);

Does lib/bitrev.c do this job?

Regards,
Simon

> +       return crc;
> +}
> +
>  /* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
>
>  /* Set the appropriate hash bit for the given addr */
> diff --git a/include/net.h b/include/net.h
> index 51c099dae2e5..359bfb5ef69f 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -289,7 +289,6 @@ 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
>
>
> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> index 2a9caa3509b0..d2e16b8fa3da 100644
> --- a/net/eth_legacy.c
> +++ b/net/eth_legacy.c
> @@ -310,30 +310,6 @@ int eth_mcast_join(struct in_addr mcast_ip, int join)
>         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
>
>
> --
> 2.19.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP
  2018-11-26  8:00 ` [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP Chris Packham
@ 2018-11-26  8:15   ` Simon Goldschmidt
  2018-11-27  3:15     ` Chris Packham
  2019-01-24 17:39   ` [U-Boot] " Joe Hershberger
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Goldschmidt @ 2018-11-26  8:15 UTC (permalink / raw)
  To: u-boot

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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP
  2018-11-26  8:15   ` Simon Goldschmidt
@ 2018-11-27  3:15     ` Chris Packham
  2018-11-27  5:59       ` Simon Goldschmidt
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Packham @ 2018-11-27  3:15 UTC (permalink / raw)
  To: u-boot

On Mon, Nov 26, 2018 at 9:15 PM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> 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.

I actually thought your latest series [1] won't conflict because you
dropped your version of this. I think your changes should take
precedence as it is fixing a real issue as opposed to my removal of
dead code. If anything I should rebase on top of your series.

[1] - http://patchwork.ozlabs.org/patch/1002685/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver
  2018-11-26  8:12 ` [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver Simon Goldschmidt
@ 2018-11-27  3:19   ` Chris Packham
  2018-11-27  6:09     ` Simon Goldschmidt
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Packham @ 2018-11-27  3:19 UTC (permalink / raw)
  To: u-boot

On Mon, Nov 26, 2018 at 9:12 PM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On Mon, Nov 26, 2018 at 9:00 AM Chris Packham <judge.packham@gmail.com> wrote:
> >
> > ether_crc was added to the core net code in commit 53a5c424bf86
> > ("multicast tftp: RFC2090") so that other drivers could use it. However
> > the only current user of it is tsec.c so move it there.
> >
> > Signed-off-by: Chris Packham <judge.packham@gmail.com>
> > ---
> >
> >  drivers/net/tsec.c | 25 +++++++++++++++++++++++++
> >  include/net.h      |  1 -
> >  net/eth_legacy.c   | 24 ------------------------
> >  3 files changed, 25 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> > index 03a46da2f8a1..9a4fab85e928 100644
> > --- a/drivers/net/tsec.c
> > +++ b/drivers/net/tsec.c
> > @@ -80,6 +80,31 @@ static void tsec_configure_serdes(struct tsec_private *priv)
> >
> >  #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
> > + */
> > +#define CRCPOLY_LE 0xedb88320
> > +static u32 ether_crc(size_t len, unsigned char const *p)
>
> I haven't checked, but can't we use lib/crc32.c for this? The
> polynomial is the same...
>

Yes more than likely.  I erred on the side of not changing more than
absolutely necessary. However given the fact that this code isn't
currently run on any platform maybe that's too cautious.

> > +{
> > +       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);
>
> Does lib/bitrev.c do this job?
>
> Regards,
> Simon
>
> > +       return crc;
> > +}
> > +
> >  /* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
> >
> >  /* Set the appropriate hash bit for the given addr */
> > diff --git a/include/net.h b/include/net.h
> > index 51c099dae2e5..359bfb5ef69f 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -289,7 +289,6 @@ 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
> >
> >
> > diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> > index 2a9caa3509b0..d2e16b8fa3da 100644
> > --- a/net/eth_legacy.c
> > +++ b/net/eth_legacy.c
> > @@ -310,30 +310,6 @@ int eth_mcast_join(struct in_addr mcast_ip, int join)
> >         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
> >
> >
> > --
> > 2.19.2
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP
  2018-11-27  3:15     ` Chris Packham
@ 2018-11-27  5:59       ` Simon Goldschmidt
  2019-01-22 23:42         ` Joe Hershberger
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Goldschmidt @ 2018-11-27  5:59 UTC (permalink / raw)
  To: u-boot

On Tue, Nov 27, 2018 at 4:15 AM Chris Packham <judge.packham@gmail.com> wrote:
>
> On Mon, Nov 26, 2018 at 9:15 PM Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > 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.
>
> I actually thought your latest series [1] won't conflict because you
> dropped your version of this. I think your changes should take
> precedence as it is fixing a real issue as opposed to my removal of
> dead code. If anything I should rebase on top of your series.

Yes I dropped the code you remove here, but still your patch does not
apply for me. I guess this is because our changes are around the same
lines. So one of us needs to rebase after applying the other.

And since my patch fixes the buffer underrun issue for MCAST_TFTP,
too, you're right that it should probably be pushed first.

Thanks,
Simon

>
> [1] - http://patchwork.ozlabs.org/patch/1002685/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver
  2018-11-27  3:19   ` Chris Packham
@ 2018-11-27  6:09     ` Simon Goldschmidt
  2019-01-22 23:41       ` Joe Hershberger
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Goldschmidt @ 2018-11-27  6:09 UTC (permalink / raw)
  To: u-boot

On Tue, Nov 27, 2018 at 4:19 AM Chris Packham <judge.packham@gmail.com> wrote:
>
> On Mon, Nov 26, 2018 at 9:12 PM Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > On Mon, Nov 26, 2018 at 9:00 AM Chris Packham <judge.packham@gmail.com> wrote:
> > >
> > > ether_crc was added to the core net code in commit 53a5c424bf86
> > > ("multicast tftp: RFC2090") so that other drivers could use it. However
> > > the only current user of it is tsec.c so move it there.
> > >
> > > Signed-off-by: Chris Packham <judge.packham@gmail.com>
> > > ---
> > >
> > >  drivers/net/tsec.c | 25 +++++++++++++++++++++++++
> > >  include/net.h      |  1 -
> > >  net/eth_legacy.c   | 24 ------------------------
> > >  3 files changed, 25 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> > > index 03a46da2f8a1..9a4fab85e928 100644
> > > --- a/drivers/net/tsec.c
> > > +++ b/drivers/net/tsec.c
> > > @@ -80,6 +80,31 @@ static void tsec_configure_serdes(struct tsec_private *priv)
> > >
> > >  #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
> > > + */
> > > +#define CRCPOLY_LE 0xedb88320
> > > +static u32 ether_crc(size_t len, unsigned char const *p)
> >
> > I haven't checked, but can't we use lib/crc32.c for this? The
> > polynomial is the same...
> >
>
> Yes more than likely.  I erred on the side of not changing more than
> absolutely necessary. However given the fact that this code isn't
> currently run on any platform maybe that's too cautious.

A quick test shows me that a call to 'ether_crc(len, data)' can be
replaced by 'bitrev32(crc32_no_comp(~0, data, len))'.
Of course the code size increases as those files have tables (crc
table: 1K, bitrev table: 256 bytes).

Simon

>
> > > +{
> > > +       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);
> >
> > Does lib/bitrev.c do this job?
> >
> > Regards,
> > Simon
> >
> > > +       return crc;
> > > +}
> > > +
> > >  /* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
> > >
> > >  /* Set the appropriate hash bit for the given addr */
> > > diff --git a/include/net.h b/include/net.h
> > > index 51c099dae2e5..359bfb5ef69f 100644
> > > --- a/include/net.h
> > > +++ b/include/net.h
> > > @@ -289,7 +289,6 @@ 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
> > >
> > >
> > > diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> > > index 2a9caa3509b0..d2e16b8fa3da 100644
> > > --- a/net/eth_legacy.c
> > > +++ b/net/eth_legacy.c
> > > @@ -310,30 +310,6 @@ int eth_mcast_join(struct in_addr mcast_ip, int join)
> > >         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
> > >
> > >
> > > --
> > > 2.19.2
> > >
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver
  2018-11-27  6:09     ` Simon Goldschmidt
@ 2019-01-22 23:41       ` Joe Hershberger
  0 siblings, 0 replies; 12+ messages in thread
From: Joe Hershberger @ 2019-01-22 23:41 UTC (permalink / raw)
  To: u-boot

On Tue, Nov 27, 2018 at 12:10 AM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On Tue, Nov 27, 2018 at 4:19 AM Chris Packham <judge.packham@gmail.com> wrote:
> >
> > On Mon, Nov 26, 2018 at 9:12 PM Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >
> > > On Mon, Nov 26, 2018 at 9:00 AM Chris Packham <judge.packham@gmail.com> wrote:
> > > >
> > > > ether_crc was added to the core net code in commit 53a5c424bf86
> > > > ("multicast tftp: RFC2090") so that other drivers could use it. However
> > > > the only current user of it is tsec.c so move it there.
> > > >
> > > > Signed-off-by: Chris Packham <judge.packham@gmail.com>
> > > > ---
> > > >
> > > >  drivers/net/tsec.c | 25 +++++++++++++++++++++++++
> > > >  include/net.h      |  1 -
> > > >  net/eth_legacy.c   | 24 ------------------------
> > > >  3 files changed, 25 insertions(+), 25 deletions(-)
> > > >
> > > > diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> > > > index 03a46da2f8a1..9a4fab85e928 100644
> > > > --- a/drivers/net/tsec.c
> > > > +++ b/drivers/net/tsec.c
> > > > @@ -80,6 +80,31 @@ static void tsec_configure_serdes(struct tsec_private *priv)
> > > >
> > > >  #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
> > > > + */
> > > > +#define CRCPOLY_LE 0xedb88320
> > > > +static u32 ether_crc(size_t len, unsigned char const *p)
> > >
> > > I haven't checked, but can't we use lib/crc32.c for this? The
> > > polynomial is the same...
> > >
> >
> > Yes more than likely.  I erred on the side of not changing more than
> > absolutely necessary. However given the fact that this code isn't
> > currently run on any platform maybe that's too cautious.
>
> A quick test shows me that a call to 'ether_crc(len, data)' can be
> replaced by 'bitrev32(crc32_no_comp(~0, data, len))'.
> Of course the code size increases as those files have tables (crc
> table: 1K, bitrev table: 256 bytes).

Not sure if tsec devices are ever super memory constrained, but I
could go either way.

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

But if someone wants to test it on tsec boards and send a
consolidation script. I'm good with that too.

Thanks,
-Joe

> Simon
>
> >
> > > > +{
> > > > +       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);
> > >
> > > Does lib/bitrev.c do this job?
> > >
> > > Regards,
> > > Simon
> > >
> > > > +       return crc;
> > > > +}
> > > > +
> > > >  /* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
> > > >
> > > >  /* Set the appropriate hash bit for the given addr */
> > > > diff --git a/include/net.h b/include/net.h
> > > > index 51c099dae2e5..359bfb5ef69f 100644
> > > > --- a/include/net.h
> > > > +++ b/include/net.h
> > > > @@ -289,7 +289,6 @@ 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
> > > >
> > > >
> > > > diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> > > > index 2a9caa3509b0..d2e16b8fa3da 100644
> > > > --- a/net/eth_legacy.c
> > > > +++ b/net/eth_legacy.c
> > > > @@ -310,30 +310,6 @@ int eth_mcast_join(struct in_addr mcast_ip, int join)
> > > >         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
> > > >
> > > >
> > > > --
> > > > 2.19.2
> > > >
> > > > _______________________________________________
> > > > U-Boot mailing list
> > > > U-Boot at lists.denx.de
> > > > https://lists.denx.de/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP
  2018-11-27  5:59       ` Simon Goldschmidt
@ 2019-01-22 23:42         ` Joe Hershberger
  0 siblings, 0 replies; 12+ messages in thread
From: Joe Hershberger @ 2019-01-22 23:42 UTC (permalink / raw)
  To: u-boot

On Mon, Nov 26, 2018 at 11:59 PM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On Tue, Nov 27, 2018 at 4:15 AM Chris Packham <judge.packham@gmail.com> wrote:
> >
> > On Mon, Nov 26, 2018 at 9:15 PM Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >
> > > 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.
> >
> > I actually thought your latest series [1] won't conflict because you
> > dropped your version of this. I think your changes should take
> > precedence as it is fixing a real issue as opposed to my removal of
> > dead code. If anything I should rebase on top of your series.
>
> Yes I dropped the code you remove here, but still your patch does not
> apply for me. I guess this is because our changes are around the same
> lines. So one of us needs to rebase after applying the other.
>
> And since my patch fixes the buffer underrun issue for MCAST_TFTP,
> too, you're right that it should probably be pushed first.

It was fairly trivial to resolve the conflict, so taking as is.

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

Thanks,
-Joe

> Thanks,
> Simon
>
> >
> > [1] - http://patchwork.ozlabs.org/patch/1002685/
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] net: move ether_crc to tsec driver
  2018-11-26  8:00 [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver Chris Packham
  2018-11-26  8:00 ` [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP Chris Packham
  2018-11-26  8:12 ` [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver Simon Goldschmidt
@ 2019-01-24 17:38 ` Joe Hershberger
  2 siblings, 0 replies; 12+ messages in thread
From: Joe Hershberger @ 2019-01-24 17:38 UTC (permalink / raw)
  To: u-boot

Hi Chris,

https://patchwork.ozlabs.org/patch/1003048/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] net: remove CONFIG_MCAST_TFTP
  2018-11-26  8:00 ` [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP Chris Packham
  2018-11-26  8:15   ` Simon Goldschmidt
@ 2019-01-24 17:39   ` Joe Hershberger
  1 sibling, 0 replies; 12+ messages in thread
From: Joe Hershberger @ 2019-01-24 17:39 UTC (permalink / raw)
  To: u-boot

Hi Chris,

https://patchwork.ozlabs.org/patch/1003050/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-01-24 17:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-26  8:00 [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver Chris Packham
2018-11-26  8:00 ` [U-Boot] [PATCH 2/2] net: remove CONFIG_MCAST_TFTP Chris Packham
2018-11-26  8:15   ` Simon Goldschmidt
2018-11-27  3:15     ` Chris Packham
2018-11-27  5:59       ` Simon Goldschmidt
2019-01-22 23:42         ` Joe Hershberger
2019-01-24 17:39   ` [U-Boot] " Joe Hershberger
2018-11-26  8:12 ` [U-Boot] [PATCH 1/2] net: move ether_crc to tsec driver Simon Goldschmidt
2018-11-27  3:19   ` Chris Packham
2018-11-27  6:09     ` Simon Goldschmidt
2019-01-22 23:41       ` Joe Hershberger
2019-01-24 17:38 ` [U-Boot] " Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.