Bug 175852 - [amd64] [patch] in_cksum_hdr() behaves differently on amd64 vs i386 with unaligned data
Summary: [amd64] [patch] in_cksum_hdr() behaves differently on amd64 vs i386 with unal...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-05 01:20 UTC by fodillemlinkarim
Modified: 2017-12-31 22:29 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description fodillemlinkarim 2013-02-05 01:20:00 UTC
TLDR: If the IP header is not aligned on an even address then the amd64 version of in_cksum_hdr() will not work while the i386 version of it will.

I came across this problem while working on custom software using the FreeBSD bridge. Our code sometimes decapsulate packets with the resulting header starting on an odd address and we need to send it through the FreeBSD bridge where we hit in_cksum_hdr() in bridge_pfil(). While this always worked on i386 we started seeing 'reversed' checksums on amd64:

21:47:29.178620 IP (tos 0x0, ttl 63, id 3819, offset 0, flags [none], proto ICMP (1), length 84)
    192.168.76.100 > 192.168.73.200: ICMP echo request, id 44019, seq 2, length 64
21:47:29.179972 IP (tos 0x0, ttl 62, id 1701, offset 0, flags [none], proto ICMP (1), length 84, bad cksum 875e (->5e87)!)
    192.168.73.200 > 192.168.76.100: ICMP echo reply, id 44019, seq 2, length 64

Please note the reversed checksum on the ICMP reply (as if someone had called htons on ip_sum ...). Needless to say this caused a lot of head scratching over here.

Now it looks like the i386 version of in_cksum_hdr() is totally different then the amd64 one.

The FreeBSD source uses in_cksum_hdr() in many other places then if_bridge.c and while the i386 version of it is capable of dealing with unaligned addresses the amd64 one is not (we haven't checked other architectures). My question(s) to the list is what is the proper way to fix this? Should we replace all occurrence of in_cksum_hdr() with in_cksum()? Should we write another inline assembly of the in_cksum_hdr function for 64bit? Should in_cksum_hdr() in amd64 changed to deal with misaligned addresses? Other solutions?

Fix: 

u_int in_cksum_hdr(const struct ip *ip)
 {
-    u_int64_t sum = in_cksumdata(ip, sizeof(struct ip));
-    union q_util q_util;
-    union l_util l_util;
-    REDUCE16;
-    return (~sum & 0xffff);
+       u_int64_t sum;
+       union q_util q_util;
+       union l_util l_util;
+       if ((uintptr_t)ip & 1)
+               sum = in_cksumdata(ip, sizeof(struct ip)) << 8;
+       else
+               sum = in_cksumdata(ip, sizeof(struct ip));
+       REDUCE16;
+       return (~sum & 0xffff);
 }--jhumMFM3X8qMzWD52y6liFihUEjZTM6UALmmtL4lpKgttCFF
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

diff --git a/freebsd/sys/amd64/amd64/in_cksum.c b/freebsd/sys/amd64/amd64/in_cksum.c
index ae02e91..71749e1 100644
--- a/freebsd/sys/amd64/amd64/in_cksum.c
+++ b/freebsd/sys/amd64/amd64/in_cksum.c
@@ -233,9 +233,13 @@ skip_start:
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2013-02-05 13:17:43 UTC
Responsible Changed
From-To: freebsd-amd64->freebsd-net

Even though this is an amd64-specific patch, I'm going to try to assign 
it to the networking mailing list since it affects the networking code.
Comment 2 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:00:56 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped