panic: TCP header not in one mbuf

[Matthew Dillon]

:Hello.
:Encountered this panic while downloading NetBSD source tree with CVSup.
:The kernel was compiled at 12th of July, but as far as I looked at commits@
:list, there's not a relavant fix yet. FWIW, the panicked DragonFly
:machine is behind a FreeBSD-CURRENT ipnat box, and I'm using mssclamp
:option in the ipnat rule file to workaround MTU problem. I'm going to
:do some more tests to see if it's reproducible.

   Try this patch.  It's another check ordering issue in ip_demux.c.
   Fragmented packets are skipping the check.  The TCP and UDP header
   length must be checked within the first fragment and, in fact, it
   is also a very good idea to disallow tcp/ip and udp/ip headers that 
   cross fragment boundaries (there are a ton of attacks that use that
   trick to get through various firewalls).

					-Matt
					Matthew Dillon 
					<dillon at xxxxxxxxxxxxx>

Index: ip_demux.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_demux.c,v
retrieving revision 1.24
diff -u -r1.24 ip_demux.c
--- ip_demux.c	8 Jul 2004 22:43:01 -0000	1.24
+++ ip_demux.c	17 Jul 2004 18:11:44 -0000
@@ -112,24 +112,73 @@
 	lwkt_port_t port;
 	int cpu;
 
+	/*
+	 * The packet must be at least the size of an IP header
+	 */
 	if (m->m_pkthdr.len < sizeof(struct ip)) {
 		ipstat.ips_tooshort++;
 		return (NULL);
 	}
 
+	/*
+	 * The first mbuf must entirely contain the IP header
+	 */
 	if (m->m_len < sizeof(struct ip) &&
 	    (m = m_pullup(m, sizeof(struct ip))) == NULL) {
 		ipstat.ips_toosmall++;
 		return (NULL);
 	}
-
 	ip = mtod(m, struct ip *);
 
+	/*
+	 * Extract the actual IP header length and do a bounds check.  The
+	 * first mbuf must entirely contain the extended IP header.
+	 */
 	iphlen = ip->ip_hl << 2;
 	if (iphlen < sizeof(struct ip)) {	/* minimum header length */
 		ipstat.ips_badhlen++;
 		return (NULL);
 	}
+	if (m->m_len < iphlen) {
+		m = m_pullup(m, iphlen);
+		if (m == NULL) {
+			ipstat.ips_badhlen++;
+			return (NULL);
+		}
+		ip = mtod(m, struct ip *);
+	}
+
+	/*
+	 * The TCP/IP or UDP/IP header must be entirely contained within
+	 * the first fragment of a packet.  Packet filters will break if they
+	 * aren't.
+	 */
+	if ((ntohs(ip->ip_off) & IP_OFFMASK) == 0) {
+		switch (ip->ip_p) {
+		case IPPROTO_TCP:
+			if (m->m_len < iphlen + sizeof(struct tcphdr)) {
+				m = m_pullup(m, iphlen + sizeof(struct tcphdr));
+				if (m == NULL) {
+					tcpstat.tcps_rcvshort++;
+					return (NULL);
+				}
+				ip = mtod(m, struct ip *);
+			}
+			break;
+		case IPPROTO_UDP:
+			if (m->m_len < iphlen + sizeof(struct udphdr)) {
+				m = m_pullup(m, iphlen + sizeof(struct udphdr));
+				if (m == NULL) {
+					udpstat.udps_hdrops++;
+					return (NULL);
+				}
+				ip = mtod(m, struct ip *);
+			}
+			break;
+		default:
+			break;
+		}
+	}
 
 	/*
 	 * XXX generic packet handling defrag on CPU 0 for now.
@@ -139,11 +188,6 @@
 
 	switch (ip->ip_p) {
 	case IPPROTO_TCP:
-		if (m->m_len < iphlen + sizeof(struct tcphdr) &&
-		    (m = m_pullup(m, iphlen + sizeof(struct tcphdr))) == NULL) {
-			tcpstat.tcps_rcvshort++;
-			return (NULL);
-		}
 		th = (struct tcphdr *)((caddr_t)ip + iphlen);
 		thoff = th->th_off << 2;
 		if (thoff < sizeof(struct tcphdr) ||
@@ -166,14 +210,6 @@
 		port = &tcp_thread[cpu].td_msgport;
 		break;
 	case IPPROTO_UDP:
-		if (m->m_len < iphlen + sizeof(struct udphdr)) {
-			m = m_pullup(m, iphlen + sizeof(struct udphdr));
-			if (m == NULL) {
-				udpstat.udps_hdrops++;
-				return (NULL);
-			}
-			ip = mtod(m, struct ip *);
-		}
 		uh = (struct udphdr *)((caddr_t)ip + iphlen);
 
 		if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) ||
@@ -186,10 +222,6 @@
 		port = &udp_thread[cpu].td_msgport;
 		break;
 	default:
-		if (m->m_len < iphlen && (m = m_pullup(m, iphlen)) == NULL) {
-			ipstat.ips_badhlen++;
-			return (NULL);
-		}
 		port = &netisr_cpu[0].td_msgport;
 		break;
 	}