Fix potential crash calling TCP delayed checksum function when ng_nat is attached to the layer 2

[sobomax]

Summary

Fix potential crash in the ng_nat module when attaching directly to the layer 2 (ethernet) while calculating TCP checksum.

The issue is due to in_delayed_cksum() expecting to access IP header at the offset 0 from the mbuf start, while if we are attached to the L2 directly, the IP header at going to be at the certain offset. As a result, in_delayed_cksum() will read some bogus value and try to use it as ip_len, obviously screwing up the calculation but also potentially causing assertion panic trying to read past allocated mbuf boundary.

Provide a specialized version of the in_delayed_cksum() kernel API to DTRT in such case.

Test Plan

We have this patch deployed on a fleet of systems for about 2 months and have not seen any of those crashes since.

[sobomax]

Fix mismerge.

[sobomax]

Fix error message.

[sobomax]

printf() -> log(). Fix type mismatch.

[tuexen]

If I understand the patch correctly, it contains three components:

• the fix by replacing in_delayed_cksum(m) with in_delayed_cksum_o(m, ipofs) in ng_nat.c.
• adding protection code which logs a warning.
• refactoring the code to remove indentation by using goto send.

It might be useful to separate these three when committing the code.

[markj]

sys/netgraph/ng_nat.c

822

Since ip_len isn't used elsewhere, just check m->m_pkthdr.len < ipofs + ntohs(ip->ip_len)?

857–859

When does libalias change the packet length? I see that ipfw updates the mbuf length as well, but there's no explanation for why.

859

882

Variables should be declared at the beginning of a scope.

sys/netinet/ip_output.c

1053–1054

We should add an assertion here KASSERT(iph_offset + sizeof(*ip) <= m->m_len, ("%s: truncated mbuf %p", __func__, m));