On Tue, Mar 1, 2016 at 12:09 PM, David Miller <da...@davemloft.net> wrote: > From: Alexander Duyck <adu...@mirantis.com> > Date: Thu, 25 Feb 2016 19:10:59 -0800 > >> This change makes it so that we only use a 16 bit length field instead of a >> 32 bit length field when computing a UDP checksum for IPv6. >> >> This fixes an issue found with UDP tunnels over IPv6 where the total size >> exceeded 65536 for a frame that was to be segmented. As a result the >> checksum being computed didn't match the frame data so we ended up being >> off by 1 for the final checksum value since we didn't cancel out the upper >> 16 bits of the length. >> >> The reasoning behind this is that RFC2460 states that for protocols such as >> UDP that carry their own length field we should use that when computing the >> checksum for the pseudo-header. As such we should be using a 16 bit value, >> not a 32 bit as is currently occurring when computing the UDP checksum for >> IPv6. >> >> Signed-off-by: Alexander Duyck <adu...@mirantis.com> > > What a can of worms. :-/ > > Reading RFC2460 over a few times, indeed using the truncated 16-bit length > is the thing to do for the pseudo-header checksum. > > We have this mistake in a few places, for example ip6_compute_pseudo() > unconditionally uses skb->len, yet is used by UDP on receive. > > Can you do a little audit and fix as many of these cases as you can find > and wrap them all into this patch? > > Thanks!
So I have been thinking about it and limiting the length to 16 bits might restrict us in terms of ipv6 jumbograms (RFC 2675), though it doesn't appear we support them but if we ever did then we might be painted into a bit of a corner. At the same time it isn't as if the frames are exactly valid anyway since we have exceeded the 64K limit without really taking any approach to resolve it as defined in RFC 2675. In addition we have the same issue for IPv4 which isn't even supposed to support frames larger than 64K. I was wondering what your thoughts would be about widening the size of the length field that we pass into csum_tcpudp_magic from a 16 bit to a 24 or 32 bit value? The general idea would be to shift tunnels over to uniformly using skb->len instead of a mix of 16 bit or 32 bit lengths. My thought is it might add a bit of security since it would invalidate the outer header checksum for the case where length has exceeded 65535 resulting in uh->len field being invalid anyway. - Alex