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: "Björn Lässig" <bla@pengutronix.de>,
	"Ahmad Fatoum" <a.fatoum@pengutronix.de>
Subject: [PATCH 03/10] net: icmp: don't blindly trust driver supplied frame size
Date: Thu,  4 Apr 2024 20:39:54 +0200	[thread overview]
Message-ID: <20240404184001.1532897-4-a.fatoum@pengutronix.de> (raw)
In-Reply-To: <20240404184001.1532897-1-a.fatoum@pengutronix.de>

The barebox network stack calls ping_reply() to handle ICMP echo
requests. The function copies destination addresses to source addresses,
fixes up some fields, recalculates checksums and then sends back the
ICMP echo response.

There are two checksums involved: The IP checksum over the IP header and
the ICMP header spanning the ICMP payload, which extends to the end of
packet.

The number of bytes that the ICMP checksum should be computed for was
being calculated as total_size_reported_by_nic - iphdr_size - ethhdr_size.

This is wrong as it assumes that the IP payload extends until the end of
packet. There are multiple reasons this may not be the case:

  - The packet is smaller than the minimum and is thus padded.
  - The sender adds trailing bytes after the IP payload for no reason.
  - The network driver reports a wrong size, because it doesn't
    correctly handle extra bytes added by hardware checksum offloading.

The last one affects the barebox cpsw and smsc95xx drivers, which will
be fixed up in follow-up commits.

The proper solution is to use the same length for the payload and
for its checksum. Do this by using the new ip_verify_size(), which will
take care to fix up the size discrepancy.

This issue was found by trying to ping a Raspberry Pi 3b running barebox
sitting behind a router employing conntrack, which apparently discarded
the ping responses due to the wrong ICMP checksum. These issues did not
occur without such a router in-beween, because the ping command itself
doesn't bother to verify the checksum.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Cc: Jan Lübbe <jlu@pengutronix.de>
Cc: Björn Lässig <bla@pengutronix.de>
---
 net/net.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/net.c b/net/net.c
index 56a599d0bc61..6745085635dc 100644
--- a/net/net.c
+++ b/net/net.c
@@ -686,7 +686,7 @@ static int ping_reply(struct eth_device *edev, unsigned char *pkt, int len)
 {
 	struct ethernet *et = (struct ethernet *)pkt;
 	struct icmphdr *icmp;
-	struct iphdr *ip = (struct iphdr *)(pkt + ETHER_HDR_SIZE);
+	struct iphdr *ip;
 	unsigned char *packet;
 	int ret;
 
@@ -696,6 +696,10 @@ static int ping_reply(struct eth_device *edev, unsigned char *pkt, int len)
 
 	icmp = net_eth_to_icmphdr(pkt);
 
+	ip = ip_verify_size(pkt, &len);
+	if (!ip)
+		return -EILSEQ;
+
 	icmp->type = ICMP_ECHO_REPLY;
 	icmp->checksum = 0;
 	icmp->checksum = ~net_checksum((unsigned char *)icmp,
-- 
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 ` [PATCH 02/10] net: ip: don't blindly trust driver supplied frame size Ahmad Fatoum
2024-04-04 18:39 ` Ahmad Fatoum [this message]
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-4-a.fatoum@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=bla@pengutronix.de \
    /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