From: bluhm <bluhm@openbsd.org>
Date: Thu, 28 Jul 2022 22:05:39 +0000 (+0000)
Subject: Checking the fragment flags of an incoming IP packet does not need
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=0541ad29664165bece2a7957d02a188f9bafb73b;p=openbsd

Checking the fragment flags of an incoming IP packet does not need
the mutex for the fragment list.  Move this code before the critical
section.  Use ISSET() to make clear which flags are checked.
OK mvs@
---

diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index 8ec2807459d..91765d2bab3 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_input.c,v 1.374 2022/07/25 23:19:34 bluhm Exp $	*/
+/*	$OpenBSD: ip_input.c,v 1.375 2022/07/28 22:05:39 bluhm Exp $	*/
 /*	$NetBSD: ip_input.c,v 1.30 1996/03/16 23:53:58 christos Exp $	*/
 
 /*
@@ -583,13 +583,13 @@ ip_fragcheck(struct mbuf **mp, int *offp)
 	hlen = ip->ip_hl << 2;
 
 	/*
-	 * If offset or IP_MF are set, must reassemble.
+	 * If offset or more fragments are set, must reassemble.
 	 * Otherwise, nothing need be done.
 	 * (We could look in the reassembly queue to see
 	 * if the packet was previously fragmented,
 	 * but it's not worth the time; just let them time out.)
 	 */
-	if (ip->ip_off &~ htons(IP_DF | IP_RF)) {
+	if (ISSET(ip->ip_off, htons(IP_OFFMASK | IP_MF))) {
 		if ((*mp)->m_flags & M_EXT) {		/* XXX */
 			if ((*mp = m_pullup(*mp, hlen)) == NULL) {
 				ipstat_inc(ips_toosmall);
@@ -598,27 +598,13 @@ ip_fragcheck(struct mbuf **mp, int *offp)
 			ip = mtod(*mp, struct ip *);
 		}
 
-		mtx_enter(&ipq_mutex);
-
-		/*
-		 * Look for queue of fragments
-		 * of this datagram.
-		 */
-		LIST_FOREACH(fp, &ipq, ipq_q) {
-			if (ip->ip_id == fp->ipq_id &&
-			    ip->ip_src.s_addr == fp->ipq_src.s_addr &&
-			    ip->ip_dst.s_addr == fp->ipq_dst.s_addr &&
-			    ip->ip_p == fp->ipq_p)
-				break;
-		}
-
 		/*
 		 * Adjust ip_len to not reflect header,
 		 * set ipqe_mff if more fragments are expected,
 		 * convert offset of this to bytes.
 		 */
 		ip->ip_len = htons(ntohs(ip->ip_len) - hlen);
-		mff = (ip->ip_off & htons(IP_MF)) != 0;
+		mff = ISSET(ip->ip_off, htons(IP_MF));
 		if (mff) {
 			/*
 			 * Make sure that fragments have a data length
@@ -627,11 +613,26 @@ ip_fragcheck(struct mbuf **mp, int *offp)
 			if (ntohs(ip->ip_len) == 0 ||
 			    (ntohs(ip->ip_len) & 0x7) != 0) {
 				ipstat_inc(ips_badfrags);
-				goto bad;
+				m_freemp(mp);
+				return IPPROTO_DONE;
 			}
 		}
 		ip->ip_off = htons(ntohs(ip->ip_off) << 3);
 
+		mtx_enter(&ipq_mutex);
+
+		/*
+		 * Look for queue of fragments
+		 * of this datagram.
+		 */
+		LIST_FOREACH(fp, &ipq, ipq_q) {
+			if (ip->ip_id == fp->ipq_id &&
+			    ip->ip_src.s_addr == fp->ipq_src.s_addr &&
+			    ip->ip_dst.s_addr == fp->ipq_dst.s_addr &&
+			    ip->ip_p == fp->ipq_p)
+				break;
+		}
+
 		/*
 		 * If datagram marked as having more fragments
 		 * or if this is not the first fragment,