From: bluhm <bluhm@openbsd.org>
Date: Mon, 27 Jun 2022 20:47:10 +0000 (+0000)
Subject: Push the kernel lock down into arpresolve().  We still need it to
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=32b4d8f21105c3409933ef802d7e28b0c0ca8f4c;p=openbsd

Push the kernel lock down into arpresolve().  We still need it to
prevent concurrent access to rt_llinfo from rtrequest_delete().
But the common case, when the MAC address is already known, works
without lock.
tested by Hrvoje Popovski; OK mvs@
---

diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c
index d49696cdda2..e3609289fcc 100644
--- a/sys/net/if_ethersubr.c
+++ b/sys/net/if_ethersubr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_ethersubr.c,v 1.281 2022/06/26 21:19:53 mvs Exp $	*/
+/*	$OpenBSD: if_ethersubr.c,v 1.282 2022/06/27 20:47:10 bluhm Exp $	*/
 /*	$NetBSD: if_ethersubr.c,v 1.19 1996/05/07 02:40:30 thorpej Exp $	*/
 
 /*
@@ -221,10 +221,7 @@ ether_resolve(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
 
 	switch (af) {
 	case AF_INET:
-		KERNEL_LOCK();
-		/* XXXSMP there is a MP race in arpresolve() */
 		error = arpresolve(ifp, rt, m, dst, eh->ether_dhost);
-		KERNEL_UNLOCK();
 		if (error)
 			return (error);
 		eh->ether_type = htons(ETHERTYPE_IP);
@@ -285,10 +282,7 @@ ether_resolve(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
 			break;
 #endif
 		case AF_INET:
-			KERNEL_LOCK();
-			/* XXXSMP there is a MP race in arpresolve() */
 			error = arpresolve(ifp, rt, m, dst, eh->ether_dhost);
-			KERNEL_UNLOCK();
 			if (error)
 				return (error);
 			break;
diff --git a/sys/netinet/if_ether.c b/sys/netinet/if_ether.c
index d72190d43a8..0f193904274 100644
--- a/sys/netinet/if_ether.c
+++ b/sys/netinet/if_ether.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_ether.c,v 1.249 2022/06/27 12:47:07 bluhm Exp $	*/
+/*	$OpenBSD: if_ether.c,v 1.250 2022/06/27 20:47:10 bluhm Exp $	*/
 /*	$NetBSD: if_ether.c,v 1.31 1996/05/11 12:59:58 mycroft Exp $	*/
 
 /*
@@ -352,8 +352,7 @@ arpresolve(struct ifnet *ifp, struct rtentry *rt0, struct mbuf *m,
 		log(LOG_DEBUG, "%s: %s: route contains no arp information\n",
 		    __func__, inet_ntop(AF_INET, &satosin(rt_key(rt))->sin_addr,
 		    addr, sizeof(addr)));
-		m_freem(m);
-		return (EINVAL);
+		goto bad;
 	}
 
 	sdl = satosdl(rt->rt_gateway);
@@ -391,6 +390,21 @@ arpresolve(struct ifnet *ifp, struct rtentry *rt0, struct mbuf *m,
 	if (ifp->if_flags & (IFF_NOARP|IFF_STATICARP))
 		goto bad;
 
+	KERNEL_LOCK();
+	/*
+	 * Re-check since we grab the kernel lock after the first check.
+	 * rtrequest_delete() can be called with shared netlock.  From
+	 * there arp_rtrequest() is reached which touches RTF_LLINFO
+	 * and rt_llinfo.  As this is called with kernel lock we grab the
+	 * kernel lock here and are safe.  XXXSMP
+	 */
+	if (!ISSET(rt->rt_flags, RTF_LLINFO)) {
+		KERNEL_UNLOCK();
+		goto bad;
+	}
+	la = (struct llinfo_arp *)rt->rt_llinfo;
+	KASSERT(la != NULL);
+
 	/*
 	 * There is an arptab entry, but no ethernet address
 	 * response yet. Insert mbuf in hold queue if below limit
@@ -435,6 +449,7 @@ arpresolve(struct ifnet *ifp, struct rtentry *rt0, struct mbuf *m,
 		}
 	}
 
+	KERNEL_UNLOCK();
 	return (EAGAIN);
 
 bad: