net: ip: don't blindly trust driver supplied frame size

We currently assume that the size reported by the network driver
reflects the number of bytes actually transmitted over the wire, but
this is apparently not always the case, at least for the barebox cpsw
and smc95xx drivers. These don't handle hardware checksum offloading
correctly and thus pass extraneous checksum bytes inserted by the
hardware to the network stack as if these were part of the transmitted
frame.

These drivers will be fixed in follow-up commits, but on the off-chance
more drivers are affected, let's use the size reported in the IP header
when doing IP packet processing.

As the old size is needed to dump the packet in net_bad_packet(), this
is moved inside the new function.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Link: https://lore.barebox.org/20240404184001.1532897-3-a.fatoum@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

1 files changed, 40 insertions, 3 deletions

diff --git a/net/net.c b/net/net.c
index 2625fb8604..56a599d0bc 100644
--- a/net/net.c
+++ b/net/net.c
@@ -645,6 +645,43 @@ static int net_handle_udp(unsigned char *pkt, int len)
 	return -EINVAL;
 }
 
+static struct iphdr *ip_verify_size(unsigned char *pkt, int *total_len_nic)
+{
+	struct iphdr *ip = (struct iphdr *)(pkt + ETHER_HDR_SIZE);
+	int total_len_pkt;
+
+	total_len_pkt = ntohs(ip->tot_len) + ETHER_HDR_SIZE;
+
+	/* Only trust the packet's size if it's within bounds */
+	if (*total_len_nic < sizeof(struct ethernet) + sizeof(struct iphdr) ||
+	    *total_len_nic < total_len_pkt) {
+		net_bad_packet(pkt, *total_len_nic);
+		return NULL;
+	}
+
+#ifdef DEBUG
+	/* Hitting this warning means we have trailing bytes after the IP
+	 * payload that are not needed for padding.
+	 *
+	 * This may be an indication that the NIC driver is doing funny
+	 * offloading stuff it shouldn't, but can also mean that some sender
+	 * in the network likes to waste bit time for nought.
+	 *
+	 * We can't differentiate between the two, so we just print the
+	 * warning when DEBUG is defined, so developers may investigate the
+	 * reason without annoying users about something that might not even
+	 * be barebox's fault.
+	 */
+	if (WARN_ON_ONCE(*total_len_nic > total_len_pkt &&
+			 *total_len_nic > 64)) {
+		net_bad_packet(pkt, *total_len_nic);
+	}
+#endif
+
+	*total_len_nic = total_len_pkt;
+	return ip;
+}
+
 static int ping_reply(struct eth_device *edev, unsigned char *pkt, int len)
 {
 	struct ethernet *et = (struct ethernet *)pkt;
@@ -709,10 +746,10 @@ static int net_handle_ip(struct eth_device *edev, unsigned char *pkt, int len)
 
 	pr_debug("%s\n", __func__);
 
-	if (len < sizeof(struct ethernet) + sizeof(struct iphdr) ||
-		len < ETHER_HDR_SIZE + ntohs(ip->tot_len)) {
+	ip = ip_verify_size(pkt, &len);
+	if (!ip) {
 		pr_debug("%s: bad len\n", __func__);
-		goto bad;
+		return 0;
 	}
 
 	if ((ip->hl_v & 0xf0) != 0x40)