Re: kern/49427: netinet/in4_cksum.c message flood

[Robert Elz <kre%munnari.OZ.AU@localhost>]

The following reply was made to PR kern/49427; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: christos%netbsd.org@localhost
Subject: Re: kern/49427: netinet/in4_cksum.c message flood
Date: Sun, 30 Nov 2014 23:32:01 +0700

 Maybe I'm missing something, but I cannot see how the code in question
 can possibly work ...
 
 looutput() does ...
 
                 if (csum_flags != 0 && IN_LOOPBACK_NEED_CHECKSUM(csum_flags)) {
                         ip_undefer_csum(m, 0, csum_flags);
                 }
 
 ip_undefer_csum(m, hdrlen, csum_flags) does ...
 
         if (csum_flags & M_CSUM_IPv4) {
                 csum = in4_cksum(m, 0, hdrlen, iphdrlen);
 
 Note, hdrlen passed down from looutput() is 0.
 
 in4_cksum(m, nxt, off, len) does ...
 
 Note that nxt & off are both 0 (nxt the const 0, and off because hdrlen == 0)
 
         if (__predict_false(off < sizeof(struct ip)))
                 PANIC("%s: offset %d too short for IP header %zu", __func__,
                     off, sizeof(struct ip));
 
 (where the PANIC() is just printf() & return if !DIAGNOSTIC).
 
 Since off is 0 (was hdrlen in ip_undefer_csum()), off < sizeof(almost anything)
 and the PANIC() is guaranteed.
 
 Turning off net.inet.ip.do_loopback_cksum=1 (making it be 0), so that
 IN_LOOPBACK_NEED_CHECKSUM() becomes false would avoid the problem, but
 someone who understands what is supposed to be happening here needs to
 look at this code carefully.
 
 To me it looks as if in4_cksum() cannot really be used to calculate IP
 header checksums - it always wants to include a pseudo-header checksum,
 suitable for UDP & TCP (and ICMPv6) but not for IP itself.
 
 That is, unless the
 
         if (nxt == 0) 
                 return cpu_in_cksum(m, len, off, 0);
 
 case is supposed to handle that, in which case, perhaps the problem is
 just that the validation tests immediately above shouldn't be done in
 this case.   I notice that switching the order of those tests is the
 most recent change to in4_cksum() which could explain why this being
 newly seen in NetBSD 7 (though it is about 18 months old - was in the
 6.99.x series for a long time - I guess almost no-one bothers turning
 in loopback checksum calculations).
 
 kre