Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 16 Nov 2018 18:32:49 +0000 (UTC)
From:      "Jonathan T. Looney" <jtl@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r340483 - head/sys/netinet
Message-ID:  <201811161832.wAGIWn6K060281@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jtl
Date: Fri Nov 16 18:32:48 2018
New Revision: 340483
URL: https://svnweb.freebsd.org/changeset/base/340483

Log:
  Add some additional length checks to the IPv4 fragmentation code.
  
  Specifically, block 0-length fragments, even when the MF bit is clear.
  Also, ensure that every fragment with the MF bit clear ends at the same
  offset and that no subsequently-received fragments exceed that offset.
  
  Reviewed by:	glebius, markj
  MFC after:	3 days
  Sponsored by:	Netflix
  Differential Revision:	https://reviews.freebsd.org/D17922

Modified:
  head/sys/netinet/ip_reass.c
  head/sys/netinet/ip_var.h

Modified: head/sys/netinet/ip_reass.c
==============================================================================
--- head/sys/netinet/ip_reass.c	Fri Nov 16 17:07:54 2018	(r340482)
+++ head/sys/netinet/ip_reass.c	Fri Nov 16 18:32:48 2018	(r340483)
@@ -211,19 +211,21 @@ ip_reass(struct mbuf *m)
 	 * convert offset of this to bytes.
 	 */
 	ip->ip_len = htons(ntohs(ip->ip_len) - hlen);
-	if (ip->ip_off & htons(IP_MF)) {
-		/*
-		 * Make sure that fragments have a data length
-		 * that's a non-zero multiple of 8 bytes.
-		 */
-		if (ip->ip_len == htons(0) || (ntohs(ip->ip_len) & 0x7) != 0) {
-			IPSTAT_INC(ips_toosmall); /* XXX */
-			IPSTAT_INC(ips_fragdropped);
-			m_freem(m);
-			return (NULL);
-		}
+	/*
+	 * Make sure that fragments have a data length
+	 * that's a non-zero multiple of 8 bytes, unless
+	 * this is the last fragment.
+	 */
+	if (ip->ip_len == htons(0) ||
+	    ((ip->ip_off & htons(IP_MF)) && (ntohs(ip->ip_len) & 0x7) != 0)) {
+		IPSTAT_INC(ips_toosmall); /* XXX */
+		IPSTAT_INC(ips_fragdropped);
+		m_freem(m);
+		return (NULL);
+	}
+	if (ip->ip_off & htons(IP_MF))
 		m->m_flags |= M_IP_FRAG;
-	} else
+	else
 		m->m_flags &= ~M_IP_FRAG;
 	ip->ip_off = htons(ntohs(ip->ip_off) << 3);
 
@@ -301,9 +303,28 @@ ip_reass(struct mbuf *m)
 		fp->ipq_src = ip->ip_src;
 		fp->ipq_dst = ip->ip_dst;
 		fp->ipq_frags = m;
+		if (m->m_flags & M_IP_FRAG)
+			fp->ipq_maxoff = -1;
+		else
+			fp->ipq_maxoff = ntohs(ip->ip_off) + ntohs(ip->ip_len);
 		m->m_nextpkt = NULL;
 		goto done;
 	} else {
+		/*
+		 * If we already saw the last fragment, make sure
+		 * this fragment's offset looks sane. Otherwise, if
+		 * this is the last fragment, record its endpoint.
+		 */
+		if (fp->ipq_maxoff > 0) {
+			i = ntohs(ip->ip_off) + ntohs(ip->ip_len);
+			if (((m->m_flags & M_IP_FRAG) && i >= fp->ipq_maxoff) ||
+			    ((m->m_flags & M_IP_FRAG) == 0 &&
+			    i != fp->ipq_maxoff)) {
+				fp = NULL;
+				goto dropfrag;
+			}
+		} else if ((m->m_flags & M_IP_FRAG) == 0)
+			fp->ipq_maxoff = ntohs(ip->ip_off) + ntohs(ip->ip_len);
 		fp->ipq_nfrags++;
 		atomic_add_int(&nfrags, 1);
 #ifdef MAC

Modified: head/sys/netinet/ip_var.h
==============================================================================
--- head/sys/netinet/ip_var.h	Fri Nov 16 17:07:54 2018	(r340482)
+++ head/sys/netinet/ip_var.h	Fri Nov 16 18:32:48 2018	(r340483)
@@ -61,6 +61,7 @@ struct ipq {
 	u_char	ipq_ttl;		/* time for reass q to live */
 	u_char	ipq_p;			/* protocol of this fragment */
 	u_short	ipq_id;			/* sequence id for reassembly */
+	int	ipq_maxoff;		/* total length of packet */
 	struct mbuf *ipq_frags;		/* to ip headers of fragments */
 	struct	in_addr ipq_src,ipq_dst;
 	u_char	ipq_nfrags;		/* # frags in this packet */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201811161832.wAGIWn6K060281>