kern/112471: [patch] sys/netinet/udp_usrreq.c modifies received UDP checksum

[Matthew Luckie]

>Number:         112471
>Category:       kern
>Synopsis:       [patch] sys/netinet/udp_usrreq.c modifies received UDP checksum
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sun May 06 23:10:00 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     Matthew Luckie
>Release:        FreeBSD 6.2-RC2 i386
>Organization:
>Environment:
System: FreeBSD sorcerer.cs.waikato.ac.nz 6.2-RC2 FreeBSD 6.2-RC2 #0: Sun Dec 24 23:42:30 UTC 2006 root at dessler.cse.buffalo.edu:/usr/obj/usr/src/sys/SMP i386


	
>Description:

At the moment, freebsd checks a UDP checksum in place, overwriting
whatever is there.  This has a side effect of the ICMP code sending
back the first eight bytes of the UDP payload with 2 bytes different
to what that system sent.

NetBSD and OpenBSD are more careful with the UDP packet in case it is
needed for ICMP.  See, for example, revision 1.46 of NetBSD
udp_usrreq.c

http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet/udp_usrreq.c

Here's an example of the problem.

listening on lo0, link-type NULL (BSD loopback), capture size 1500 bytes
13:41:24.382239 IP localhost.40858 > localhost.33435: UDP, length 12
        0x0000:  0200 0000 4500 0028 9f9b 0000 0111 1c28
        0x0010:  7f00 0001 7f00 0001 9f9a 829b 0014 df8d
        0x0020:  0000 0000 0000 0000 0000 0000
13:41:24.382250 IP localhost > localhost: ICMP localhost udp port 33435 unreachable, length 36
        0x0000:  0200 0000 4500 0038 00cc 0000 4001 7bf7
        0x0010:  7f00 0001 7f00 0001 0303 dab2 0000 0000
        0x0020:  4500 0028 9f9b 0000 0111 1c28 7f00 0001
        0x0030:  7f00 0001 9f9a 829b 0014 0000

With the patch below, the checksum is not checked in place -- i.e.

13:54:47.371646 IP localhost.33826 > localhost.33435: UDP, length 12
        0x0000:  0200 0000 4500 0028 8423 0000 0111 37a0
        0x0010:  7f00 0001 7f00 0001 8422 829b 0014 fb05
        0x0020:  0000 0000 0000 0000 0000 0000
13:54:47.371658 IP localhost > localhost: ICMP localhost udp port 33435 unreachable, length 36
        0x0000:  0200 0000 4500 0038 001b 0000 4001 7ca8
        0x0010:  7f00 0001 7f00 0001 0303 fb24 0000 0000
        0x0020:  4500 0028 8423 0000 0111 37a0 7f00 0001
        0x0030:  7f00 0001 8422 829b 0014 fb05

Patch is against -current, but applies to FreeBSD 6.2 as well.

>How-To-Repeat:
	tcpdump -i lo0 -s 1500 -xx
	traceroute -q 1 -n 127.0.0.1
>Fix:

	

--- udp_usrreq.c.patch begins here ---
--- udp_usrreq.c.orig	Thu May  3 12:24:55 2007
+++ udp_usrreq.c	Thu May  3 12:26:47 2007
@@ -248,23 +248,24 @@
 	 * Checksum extended UDP header and data.
 	 */
 	if (uh->uh_sum) {
+		u_short uh_sum;
 		if (m->m_pkthdr.csum_flags & CSUM_DATA_VALID) {
 			if (m->m_pkthdr.csum_flags & CSUM_PSEUDO_HDR)
-				uh->uh_sum = m->m_pkthdr.csum_data;
+				uh_sum = m->m_pkthdr.csum_data;
 			else
-				uh->uh_sum = in_pseudo(ip->ip_src.s_addr,
+				uh_sum = in_pseudo(ip->ip_src.s_addr,
 				    ip->ip_dst.s_addr, htonl((u_short)len +
 				    m->m_pkthdr.csum_data + IPPROTO_UDP));
-			uh->uh_sum ^= 0xffff;
+			uh_sum ^= 0xffff;
 		} else {
 			char b[9];
 			bcopy(((struct ipovly *)ip)->ih_x1, b, 9);
 			bzero(((struct ipovly *)ip)->ih_x1, 9);
 			((struct ipovly *)ip)->ih_len = uh->uh_ulen;
-			uh->uh_sum = in_cksum(m, len + sizeof (struct ip));
+			uh_sum = in_cksum(m, len + sizeof (struct ip));
 			bcopy(b, ((struct ipovly *)ip)->ih_x1, 9);
 		}
-		if (uh->uh_sum) {
+		if (uh_sum) {
 			udpstat.udps_badsum++;
 			m_freem(m);
 			return;
--- udp_usrreq.c.patch ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted: