Push the kernel lock down into arpresolve(). We still need it to
authorbluhm <bluhm@openbsd.org>
Mon, 27 Jun 2022 20:47:10 +0000 (20:47 +0000)
committerbluhm <bluhm@openbsd.org>
Mon, 27 Jun 2022 20:47:10 +0000 (20:47 +0000)
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@

sys/net/if_ethersubr.c
sys/netinet/if_ether.c

index d49696c..e360928 100644 (file)
@@ -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;
index d72190d..0f19390 100644 (file)
@@ -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: