mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: barebox@lists.infradead.org
Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Subject: [PATCH 02/10] net: ip: don't blindly trust driver supplied frame size
Date: Thu,  4 Apr 2024 20:39:53 +0200	[thread overview]
Message-ID: <20240404184001.1532897-3-a.fatoum@pengutronix.de> (raw)
In-Reply-To: <20240404184001.1532897-1-a.fatoum@pengutronix.de>

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>
---
 net/net.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/net/net.c b/net/net.c
index 2625fb8604fb..56a599d0bc61 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)
-- 
2.39.2




  parent reply	other threads:[~2024-04-04 18:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-04 18:39 [PATCH 00/10] net: fix problems handling trailing bytes Ahmad Fatoum
2024-04-04 18:39 ` [PATCH 01/10] net: free packets with net_free_packet Ahmad Fatoum
2024-04-04 18:39 ` Ahmad Fatoum [this message]
2024-04-04 18:39 ` [PATCH 03/10] net: icmp: don't blindly trust driver supplied frame size Ahmad Fatoum
2024-04-04 18:39 ` [PATCH 04/10] net: icmp: properly set IP TTL and fragement fields Ahmad Fatoum
2024-04-04 18:39 ` [PATCH 05/10] net: icmp: don't overrun buffer on send Ahmad Fatoum
2024-04-04 18:39 ` [PATCH 06/10] net: cpsw: report correct frame size to network stack Ahmad Fatoum
2024-04-04 18:39 ` [PATCH 07/10] net: usb: smsc95xx: don't opencode get/put_aligned_le32 Ahmad Fatoum
2024-04-04 18:39 ` [PATCH 08/10] net: usb: smsc95xx: don't blindly trust hardware size Ahmad Fatoum
2024-04-04 18:40 ` [PATCH 09/10] net: usb: smsc95xx: fix handling of multiple packets per urb Ahmad Fatoum
2024-04-05  7:18   ` [PATCH] fixup! " Ahmad Fatoum
2024-04-04 18:40 ` [PATCH 10/10] net: usb: smsc95xx: disable HW checksumming in driver Ahmad Fatoum
2024-04-04 19:49 ` [PATCH 00/10] net: fix problems handling trailing bytes Ahmad Fatoum
2024-04-05  9:57 ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240404184001.1532897-3-a.fatoum@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox