From b01ae44a0a8c0489aab069e00c511d15f619edb1 Mon Sep 17 00:00:00 2001
From: Matthew Dillon <dillon@dragonflybsd.org>
Date: Sun, 18 Jul 2004 16:26:43 +0000
Subject: [PATCH] Fix two serious bugs in the IP demux code.  First, if
 ip_mport() m_pullup()'s an mbuf, the new/modified mbuf is not returned to the
 caller and the caller may wind up using a stale/freed mbuf.  Second,
 ip_mport() was not consistently freeding mbufs which could lead to both a
 memory leak and a double free.

Reported-by: YONETANI Tomokazu <qhwt+dragonfly-bugs@les.ath.cx> (panic: TCP header not in one mbuf).
---
 sys/net/netisr.c       |  6 +--
 sys/net/netisr.h       |  6 +--
 sys/netinet/ip_demux.c | 94 +++++++++++++++++++++++++++++++-----------
 sys/netinet/ip_input.c | 18 ++++----
 sys/netinet/ip_var.h   |  4 +-
 5 files changed, 88 insertions(+), 40 deletions(-)

diff --git a/sys/net/netisr.c b/sys/net/netisr.c
index 87a054e597..1a8e675ef6 100644
--- a/sys/net/netisr.c
+++ b/sys/net/netisr.c
@@ -35,7 +35,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $DragonFly: src/sys/net/netisr.c,v 1.20 2004/07/16 05:48:08 dillon Exp $
+ * $DragonFly: src/sys/net/netisr.c,v 1.21 2004/07/18 16:26:41 dillon Exp $
  */
 
 /*
@@ -242,7 +242,7 @@ netisr_queue(int num, struct mbuf *m)
 	return (EIO);
     }
 
-    if (!(port = ni->ni_mport(m)))
+    if ((port = ni->ni_mport(&m)) == NULL)
 	return (EIO);
 
     /* use better message allocation system with limits later XXX JH */
@@ -281,7 +281,7 @@ netisr_unregister(int num)
  * Return message port for default handler thread on CPU 0.
  */
 lwkt_port_t
-cpu0_portfn(struct mbuf *m)
+cpu0_portfn(struct mbuf **mptr)
 {
     return (&netisr_cpu[0].td_msgport);
 }
diff --git a/sys/net/netisr.h b/sys/net/netisr.h
index c0b0aa02c0..996e10cd6b 100644
--- a/sys/net/netisr.h
+++ b/sys/net/netisr.h
@@ -82,7 +82,7 @@
  *
  *	@(#)netisr.h	8.1 (Berkeley) 6/10/93
  * $FreeBSD: src/sys/net/netisr.h,v 1.21.2.5 2002/02/09 23:02:39 luigi Exp $
- * $DragonFly: src/sys/net/netisr.h,v 1.19 2004/07/08 22:07:34 hsu Exp $
+ * $DragonFly: src/sys/net/netisr.h,v 1.20 2004/07/18 16:26:41 dillon Exp $
  */
 
 #ifndef _NET_NETISR_H_
@@ -199,7 +199,7 @@ int netmsg_pr_timeout(lwkt_msg_t);
 int netmsg_so_notify(lwkt_msg_t);
 int netmsg_so_notify_abort(lwkt_msg_t);
 
-typedef lwkt_port_t (*lwkt_portfn_t)(struct mbuf *);
+typedef lwkt_port_t (*lwkt_portfn_t)(struct mbuf **);
 
 struct netisr {
 	lwkt_port	ni_port;	/* must be first */
@@ -210,7 +210,7 @@ struct netisr {
 
 extern lwkt_port netisr_afree_rport;
 
-lwkt_port_t	cpu0_portfn(struct mbuf *m);
+lwkt_port_t	cpu0_portfn(struct mbuf **mptr);
 void		netisr_dispatch(int, struct mbuf *);
 int		netisr_queue(int, struct mbuf *);
 void		netisr_register(int, lwkt_portfn_t, netisr_fn_t);
diff --git a/sys/netinet/ip_demux.c b/sys/netinet/ip_demux.c
index 0d53dbd7ee..30160600d0 100644
--- a/sys/netinet/ip_demux.c
+++ b/sys/netinet/ip_demux.c
@@ -30,7 +30,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $DragonFly: src/sys/netinet/ip_demux.c,v 1.24 2004/07/08 22:43:01 hsu Exp $
+ * $DragonFly: src/sys/netinet/ip_demux.c,v 1.25 2004/07/18 16:26:43 dillon Exp $
  */
 
 /*
@@ -99,62 +99,122 @@ INP_MPORT_HASH(in_addr_t faddr, in_addr_t laddr,
 }
 
 /*
- * Map a packet to a protocol processing thread.
+ * Map a packet to a protocol processing thread and return the thread's port.
+ * If an error occurs, the passed mbuf will be freed, *mptr will be set
+ * to NULL, and NULL will be returned.  If no error occurs, the passed mbuf
+ * may be modified and a port pointer will be returned.
  */
 lwkt_port_t
-ip_mport(struct mbuf *m)
+ip_mport(struct mbuf **mptr)
 {
 	struct ip *ip;
 	int iphlen;
 	struct tcphdr *th;
 	struct udphdr *uh;
+	struct mbuf *m = *mptr;
 	int thoff;				/* TCP data offset */
 	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++;
+		m_freem(m);
+		*mptr = NULL;
 		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++;
+		*mptr = NULL;
 		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++;
+		m_freem(m);
 		return (NULL);
 	}
+	if (m->m_len < iphlen) {
+		m = m_pullup(m, iphlen);
+		if (m == NULL) {
+			ipstat.ips_badhlen++;
+			*mptr = NULL;
+			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++;
+					*mptr = NULL;
+					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++;
+					*mptr = NULL;
+					return (NULL);
+				}
+				ip = mtod(m, struct ip *);
+			}
+			break;
+		default:
+			break;
+		}
+	}
 
 	/*
 	 * XXX generic packet handling defrag on CPU 0 for now.
 	 */
-	if (ntohs(ip->ip_off) & (IP_MF | IP_OFFMASK))
+	if (ntohs(ip->ip_off) & (IP_MF | IP_OFFMASK)) {
+		*mptr = m;
 		return (&netisr_cpu[0].td_msgport);
+	}
 
 	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) ||
 		    thoff > ntohs(ip->ip_len)) {
 			tcpstat.tcps_rcvbadoff++;
+			m_freem(m);
+			*mptr = NULL;
 			return (NULL);
 		}
 		if (m->m_len < iphlen + thoff) {
 			m = m_pullup(m, iphlen + thoff);
 			if (m == NULL) {
 				tcpstat.tcps_rcvshort++;
+				*mptr = NULL;
 				return (NULL);
 			}
 			ip = mtod(m, struct ip *);
@@ -166,14 +226,6 @@ ip_mport(struct mbuf *m)
 		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,15 +238,11 @@ ip_mport(struct mbuf *m)
 		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;
 	}
 	KKASSERT(port->mp_putport != NULL);
-
+	*mptr = m;
 	return (port);
 }
 
diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index 077a25a946..53649d8020 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -82,7 +82,7 @@
  *
  *	@(#)ip_input.c	8.2 (Berkeley) 1/4/94
  * $FreeBSD: src/sys/netinet/ip_input.c,v 1.130.2.52 2003/03/07 07:01:28 silby Exp $
- * $DragonFly: src/sys/netinet/ip_input.c,v 1.33 2004/07/08 22:07:35 hsu Exp $
+ * $DragonFly: src/sys/netinet/ip_input.c,v 1.34 2004/07/18 16:26:43 dillon Exp $
  */
 
 #define	_IP_VHL
@@ -1096,7 +1096,6 @@ DPRINTF(("ip_input: no SP, packet discarded\n"));/*XXX*/
 		lwkt_initmsg(&msg->nm_lmsg, &netisr_afree_rport, 0,
 			lwkt_cmd_func(transport_processing_handler),
 			lwkt_cmd_op_none);
-		msg->nm_mbuf = m;
 		msg->nm_hlen = hlen;
 		msg->nm_hasnexthop = (args.next_hop != NULL);
 		if (msg->nm_hasnexthop)
@@ -1104,13 +1103,14 @@ DPRINTF(("ip_input: no SP, packet discarded\n"));/*XXX*/
 
 		ip->ip_off = htons(ip->ip_off);
 		ip->ip_len = htons(ip->ip_len);
-		port = ip_mport(m);
-		if (port == NULL)
-			goto bad;
-		ip->ip_len = ntohs(ip->ip_len);
-		ip->ip_off = ntohs(ip->ip_off);
-
-		lwkt_sendmsg(port, &msg->nm_lmsg);
+		port = ip_mport(&m);
+		if (port) {
+		    msg->nm_mbuf = m;
+		    ip = mtod(m, struct ip *);
+		    ip->ip_len = ntohs(ip->ip_len);
+		    ip->ip_off = ntohs(ip->ip_off);
+		    lwkt_sendmsg(port, &msg->nm_lmsg);
+		}
 	} else {
 		transport_processing_oncpu(m, hlen, ip, args.next_hop);
 	}
diff --git a/sys/netinet/ip_var.h b/sys/netinet/ip_var.h
index 2795f935f0..fe28223d8d 100644
--- a/sys/netinet/ip_var.h
+++ b/sys/netinet/ip_var.h
@@ -32,7 +32,7 @@
  *
  *	@(#)ip_var.h	8.2 (Berkeley) 1/9/95
  * $FreeBSD: src/sys/netinet/ip_var.h,v 1.50.2.13 2003/08/24 08:24:38 hsu Exp $
- * $DragonFly: src/sys/netinet/ip_var.h,v 1.10 2004/06/24 08:15:17 dillon Exp $
+ * $DragonFly: src/sys/netinet/ip_var.h,v 1.11 2004/07/18 16:26:43 dillon Exp $
  */
 
 #ifndef _NETINET_IP_VAR_H_
@@ -183,7 +183,7 @@ void	 ip_init(void);
 extern int	 (*ip_mforward)(struct ip *, struct ifnet *, struct mbuf *,
 			  struct ip_moptions *);
 struct lwkt_port *
-	 ip_mport(struct mbuf *);
+	 ip_mport(struct mbuf **);
 int	 ip_output(struct mbuf *,
 	    struct mbuf *, struct route *, int, struct ip_moptions *,
 	    struct inpcb *);
-- 
2.41.0