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@xxxxxxxxxxxxxx>
---
Cc: Jan Lübbe <jlu@xxxxxxxxxxxxxx>
Cc: Björn Lässig <bla@xxxxxxxxxxxxxx>
---
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
