Subject: m_pullup for arp/rarp input
To: None <tech-net@netbsd.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-net
Date: 06/29/2002 21:07:58
----Next_Part(Sat_Jun_29_21:07:58_2002_616)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

hi.

is there any reason not to try m_pullup for
arp/rarp input packets?
is attached diff correct?
- try m_pullup for arp/rarp input
- move ieee1394 check from in_revarpinput to revarpinput
  since it isn't inet-specific and should be done before
  packet size check.

---
YAMAMOTO Takashi<yamt@mwd.biglobe.ne.jp>

----Next_Part(Sat_Jun_29_21:07:58_2002_616)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="arp.diff"

Index: if_arp.c
===================================================================
RCS file: /cvs/cvsroot/syssrc/sys/netinet/if_arp.c,v
retrieving revision 1.89
diff -u -p -r1.89 if_arp.c
--- if_arp.c	2002/06/25 04:16:31	1.89
+++ if_arp.c	2002/06/29 11:55:26
@@ -772,7 +772,8 @@ arpintr()
 		/*
 		 * First, make sure we have at least struct arphdr.
 		 */
-		if (m->m_len < sizeof(struct arphdr) ||
+		if ((m->m_len < sizeof(struct arphdr) &&
+		    (m = m_pullup(m, sizeof(struct arphdr))) == NULL) ||
 		    (ar = mtod(m, struct arphdr *)) == NULL)
 			goto badlen;
 
@@ -787,21 +788,27 @@ arpintr()
 			break;
 		}
 
-		if (/* XXX ntohs(ar->ar_hrd) == ARPHRD_ETHER && */
-		    m->m_len >= arplen)
-			switch (ntohs(ar->ar_pro)) {
-			case ETHERTYPE_IP:
-			case ETHERTYPE_IPTRAILERS:
-				in_arpinput(m);
-				continue;
-			default:
-				arpstat.as_rcvbadproto++;
-			}
-		else {
-badlen:
-			arpstat.as_rcvbadlen++;
+		/* XXX check ar->ar_hrd ? */
+
+		if (m->m_len < arplen) {
+			if ((m = m_pullup(m, arplen)) == NULL)
+				goto badlen;
+			ar = mtod(m, struct arphdr *);
+		}
+
+		switch (ntohs(ar->ar_pro)) {
+		case ETHERTYPE_IP:
+		case ETHERTYPE_IPTRAILERS:
+			in_arpinput(m);
+			continue;
+		default:
+			arpstat.as_rcvbadproto++;
 		}
 		m_freem(m);
+		continue;
+badlen:
+		arpstat.as_rcvbadlen++;
+		m_freem(m);
 	}
 }
 
@@ -1215,16 +1222,30 @@ revarpinput(m)
 	struct mbuf *m;
 {
 	struct arphdr *ar;
+	int rarplen;
 
-	if (m->m_len < sizeof(struct arphdr))
+	switch (m->m_pkthdr.rcvif->if_type) {
+	case IFT_IEEE1394:
+		/* ARP without target hardware address is not supported */
 		goto out;
+	default:
+		break;
+	}
+
+	if (m->m_len < sizeof(struct arphdr) &&
+	    (m = m_pullup(m, sizeof(struct arphdr))) == NULL)
+		goto out;
 	ar = mtod(m, struct arphdr *);
 #if 0 /* XXX I don't think we need this... and it will prevent other LL */
 	if (ntohs(ar->ar_hrd) != ARPHRD_ETHER)
 		goto out;
 #endif
-	if (m->m_len < sizeof(struct arphdr) + 2 * (ar->ar_hln + ar->ar_pln))
-		goto out;
+	rarplen = sizeof(struct arphdr) + 2 * (ar->ar_hln + ar->ar_pln);
+	if (m->m_len < rarplen) {
+		if ((m = m_pullup(m, rarplen)) == NULL)
+			goto out;
+		ar = mtod(m, struct arphdr *);
+	}
 	switch (ntohs(ar->ar_pro)) {
 	case ETHERTYPE_IP:
 	case ETHERTYPE_IPTRAILERS:
@@ -1259,14 +1280,6 @@ in_revarpinput(m)
 
 	ah = mtod(m, struct arphdr *);
 	op = ntohs(ah->ar_op);
-
-	switch (m->m_pkthdr.rcvif->if_type) {
-	case IFT_IEEE1394:
-		/* ARP without target hardware address is not supported */
-		goto out;
-	default:
-		break;
-	}
 
 	switch (op) {
 	case ARPOP_REQUEST:

----Next_Part(Sat_Jun_29_21:07:58_2002_616)----