Date: Thu, 2 Oct 2014 09:32:34 +0200 From: Michael Tuexen <Michael.Tuexen@lurchi.franken.de> To: Bryan Venteicher <bryanv@daemoninthecloset.org> Cc: FreeBSD Net <freebsd-net@freebsd.org> Subject: Re: UDP/IPv6 handling Message-ID: <6AF1921D-BAFB-4969-80EF-C1CE37446D65@lurchi.franken.de> In-Reply-To: <CAMo0n6Q56yvHYp8XUG499gkkxL0=QRdTVDvph9jA=kNL4%2BS-1A@mail.gmail.com> References: <B30E0A41-51B0-442C-9476-0D9E99C0D37C@lurchi.franken.de> <CAMo0n6Q56yvHYp8XUG499gkkxL0=QRdTVDvph9jA=kNL4%2BS-1A@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 02 Oct 2014, at 05:51, Bryan Venteicher =
<bryanv@daemoninthecloset.org> wrote:
>=20
>=20
> On Wed, Oct 1, 2014 at 11:58 AM, Michael Tuexen =
<Michael.Tuexen@lurchi.franken.de> wrote:
> Dear all,
>=20
> in udp6_input() we have the following code:
>=20
> if (nxt =3D=3D IPPROTO_UDP && plen !=3D ulen) {
> UDPSTAT_INC(udps_badlen);
> goto badunlocked;
> }
> /*
> * Checksum extended UDP header and data.
> */
> if (uh->uh_sum =3D=3D 0) {
> if (ulen > plen || ulen < sizeof(struct udphdr)) {
> UDPSTAT_INC(udps_nosum);
> goto badunlocked;
> }
> }
>=20
> I'm trying to understand the UDP code path...
>=20
>=20
> =E2=80=8BI too was recently confused by this code. =E2=80=8BI pointed =
out one issue to kevlo@ recently, but it still kind of seemed like the =
UDP-Lite was mismerged to IPv6.
I have a patch (to be committed soon which fixes UDPLite/IPv6).
>=20
> So (ulen > plen) can't be true. I'm wondering why do we only check the =
ulen is not too
> short only in the case when the UDP checksum is zero. A zero checksum =
should also never happen.
Yepp.
>=20
>=20
> =E2=80=8BI hope to have a patch for =E2=80=8BRFC6935 [1] soon so a =
zero checksum may be allowed if the inp/udpcb is configured for it.
Great. However, we need to check that ulen is at least sizeof(struct =
udphdr) in any case.
>=20
>=20
> I think we should check for ulen < sizeof(struct udphdr) in any case.
>=20
>=20
> =E2=80=8BI think previously, the checks in ip6_input(), =
IP6_EXTHDR_CHECK(), and plen =3D=3D ulen made this unnecessary. I think =
we'd want to do it for UDP-Lite if ulen was not initially zero.
But IP6_EXTHDR_CHECK doesn't check any fields in the packet. So it can =
happen that plen =3D=3D ulen and ulen < sizeof(struct udphdr)...
Best regards
Michael
> =E2=80=8B[1] - http://tools.ietf.org/html/rfc6935=E2=80=8B
> =20
> Opinions?
>=20
> Best regards
> Michael
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
>=20
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6AF1921D-BAFB-4969-80EF-C1CE37446D65>