Page MenuHomeFreeBSD

netinet: fix checksum calculation bug
AcceptedPublic

Authored by timo.voelker_fh-muenster.de on Wed, Dec 17, 3:32 PM.

Details

Summary

D49677 introduced the new function in_delayed_cksum_o in ip_output.c that computes the checksum even if the mbuf does not start with the IP header by using the parameter offset as the offset to the IP header.

If the offset is larger than 0, this function computes a wrong checksum for a TCP packet. For a TCP packet, it calls in_cksum_skip with the IP total length field value as second parameter, where in_cksum_skip expects the length of the mbuf.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/netinet/ip_output.c
1069

Would it be better to use

	 cklen = ntohs(ip->ip_len) - (ip->ip_hl << 2);

That would keep the semantic of cklen. Then you could use

	csum = in_cksum_skip(m, cklen + offset, offset);

below.
You could then simplify the code even further to use this statement always. Only the mapping of 0 to 0xffff would be UDP specific.

This revision is now accepted and ready to land.Wed, Dec 17, 5:39 PM
sys/netinet/ip_output.c
1069

Using

cklen = ntohs(ip->ip_len) - (ip->ip_hl << 2);

for UDP indeed simplifies the code but changes the behavior if the value in the UDP length field is not equal to the value in the IP total length field - the IP header length. This might be the case with UDP options (RFC9868). Would that be an issue?

sys/netinet/ip_output.c
1069

I don't know what else needs changes if you implement UDP options. We can at least argue that this change should fix the function, not simplify it by changing its behavior.

I still would suggest to change cklen to assign to it the number of bytes the checksum will be computed over.