Remove kernel lock from ifa_ifwithaddr() and ifa_ifwithdstaddr().

Netlock protects `if_list', `ifa_list' and returned `ifa' dereference,
so put netlock assertion within.

Please note, rtable_setsource() doesn't destroy data pointed by
`ar_source'. This is the `ifa_addr' data belongs to `ifa' and exclusive
netlock is required to destroy it. So the kernel lock is not required
within rt_setsource(). Take netlock by rt_setsource() caller to make
`ifa' dereference safe.

Suggestions and ok by bluhm@

diff --git a/sys/dev/usb/if_umb.c b/sys/dev/usb/if_umb.c
index ca89f55..77dd539 100644 (file)
--- a/sys/dev/usb/if_umb.c
+++ b/sys/dev/usb/if_umb.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_umb.c,v 1.50 2023/03/31 23:53:49 dlg Exp $ */
+/*     $OpenBSD: if_umb.c,v 1.51 2023/04/18 22:01:23 mvs Exp $ */
 
 /*
  * Copyright (c) 2016 genua mbH
@@ -1829,6 +1829,7 @@ umb_add_inet_config(struct umb_softc *sc, struct in_addr ip, u_int prefixlen,
        default_sin.sin_len = sizeof (default_sin);
 
        memset(&info, 0, sizeof(info));
+       NET_LOCK();
        info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
        info.rti_ifa = ifa_ifwithaddr(sintosa(&ifra.ifra_addr),
            ifp->if_rdomain);
@@ -1836,7 +1837,6 @@ umb_add_inet_config(struct umb_softc *sc, struct in_addr ip, u_int prefixlen,
        info.rti_info[RTAX_NETMASK] = sintosa(&default_sin);
        info.rti_info[RTAX_GATEWAY] = sintosa(&ifra.ifra_dstaddr);
 
-       NET_LOCK();
        rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
        NET_UNLOCK();
        if (rv) {
@@ -1910,6 +1910,7 @@ umb_add_inet6_config(struct umb_softc *sc, struct in6_addr *ip, u_int prefixlen,
        default_sin6.sin6_len = sizeof (default_sin6);
 
        memset(&info, 0, sizeof(info));
+       NET_LOCK();
        info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
        info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
            ifp->if_rdomain);
@@ -1917,7 +1918,6 @@ umb_add_inet6_config(struct umb_softc *sc, struct in6_addr *ip, u_int prefixlen,
        info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
        info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
 
-       NET_LOCK();
        rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
        NET_UNLOCK();
        if (rv) {

diff --git a/sys/net/if.c b/sys/net/if.c
index 1cd91f8..e25fe1c 100644 (file)
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if.c,v 1.689 2023/04/18 22:00:19 mvs Exp $    */
+/*     $OpenBSD: if.c,v 1.690 2023/04/18 22:01:24 mvs Exp $    */
 /*     $NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $  */
 
 /*
@@ -1413,8 +1413,9 @@ ifa_ifwithaddr(struct sockaddr *addr, u_int rtableid)
        struct ifaddr *ifa;
        u_int rdomain;
 
+       NET_ASSERT_LOCKED();
+
        rdomain = rtable_l2(rtableid);
-       KERNEL_LOCK();
        TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
                if (ifp->if_rdomain != rdomain)
                        continue;
@@ -1424,12 +1425,10 @@ ifa_ifwithaddr(struct sockaddr *addr, u_int rtableid)
                                continue;
 
                        if (equal(addr, ifa->ifa_addr)) {
-                               KERNEL_UNLOCK();
                                return (ifa);
                        }
                }
        }
-       KERNEL_UNLOCK();
        return (NULL);
 }
 
@@ -1442,8 +1441,9 @@ ifa_ifwithdstaddr(struct sockaddr *addr, u_int rdomain)
        struct ifnet *ifp;
        struct ifaddr *ifa;
 
+       NET_ASSERT_LOCKED();
+
        rdomain = rtable_l2(rdomain);
-       KERNEL_LOCK();
        TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
                if (ifp->if_rdomain != rdomain)
                        continue;
@@ -1453,13 +1453,11 @@ ifa_ifwithdstaddr(struct sockaddr *addr, u_int rdomain)
                                    addr->sa_family || ifa->ifa_dstaddr == NULL)
                                        continue;
                                if (equal(addr, ifa->ifa_dstaddr)) {
-                                       KERNEL_UNLOCK();
                                        return (ifa);
                                }
                        }
                }
        }
-       KERNEL_UNLOCK();
        return (NULL);
 }
 
@@ -3171,12 +3169,14 @@ ifsettso(struct ifnet *ifp, int on)
 void
 ifa_add(struct ifnet *ifp, struct ifaddr *ifa)
 {
+       NET_ASSERT_LOCKED_EXCLUSIVE();
        TAILQ_INSERT_TAIL(&ifp->if_addrlist, ifa, ifa_list);
 }
 
 void
 ifa_del(struct ifnet *ifp, struct ifaddr *ifa)
 {
+       NET_ASSERT_LOCKED_EXCLUSIVE();
        TAILQ_REMOVE(&ifp->if_addrlist, ifa, ifa_list);
 }
 

diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 0eeebdc..853dbcc 100644 (file)
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_var.h,v 1.124 2023/04/18 22:00:19 mvs Exp $        */
+/*     $OpenBSD: if_var.h,v 1.125 2023/04/18 22:01:24 mvs Exp $        */
 /*     $NetBSD: if.h,v 1.23 1996/05/07 02:40:27 thorpej Exp $  */
 
 /*
@@ -240,7 +240,8 @@ struct ifaddr {
 #define        ifa_broadaddr   ifa_dstaddr     /* broadcast address interface */
        struct  sockaddr *ifa_netmask;  /* used to determine subnet */
        struct  ifnet *ifa_ifp;         /* back-pointer to interface */
-       TAILQ_ENTRY(ifaddr) ifa_list;   /* list of addresses for interface */
+       TAILQ_ENTRY(ifaddr) ifa_list;   /* [N] list of addresses for
+                                           interface */
        u_int   ifa_flags;              /* interface flags, see below */
        struct  refcnt ifa_refcnt;      /* number of `rt_ifa` references */
        int     ifa_metric;             /* cost of going out this interface */

diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c
index b6d3c06..d3cc836 100644 (file)
--- a/sys/net/rtsock.c
+++ b/sys/net/rtsock.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: rtsock.c,v 1.362 2023/04/18 09:56:54 mvs Exp $        */
+/*     $OpenBSD: rtsock.c,v 1.363 2023/04/18 22:01:24 mvs Exp $        */
 /*     $NetBSD: rtsock.c,v 1.18 1996/03/29 00:32:10 cgd Exp $  */
 
 /*
@@ -853,8 +853,10 @@ route_output(struct mbuf *m, struct socket *so)
                        error = EINVAL;
                        goto fail;
                }
-               if ((error =
-                   rt_setsource(tableid, info.rti_info[RTAX_IFA])) != 0)
+               NET_LOCK();
+               error = rt_setsource(tableid, info.rti_info[RTAX_IFA]);
+               NET_UNLOCK();
+               if (error)
                        goto fail;
        } else {
                error = rtm_output(rtm, &rt, &info, prio, tableid);
@@ -2350,7 +2352,6 @@ int
 rt_setsource(unsigned int rtableid, struct sockaddr *src)
 {
        struct ifaddr   *ifa;
-       int             error;
        /*
         * If source address is 0.0.0.0 or ::
         * use automatic source selection
@@ -2374,20 +2375,14 @@ rt_setsource(unsigned int rtableid, struct sockaddr *src)
                return (EAFNOSUPPORT);
        }
 
-       KERNEL_LOCK();
        /*
         * Check if source address is assigned to an interface in the
         * same rdomain
         */
-       if ((ifa = ifa_ifwithaddr(src, rtableid)) == NULL) {
-               KERNEL_UNLOCK();
+       if ((ifa = ifa_ifwithaddr(src, rtableid)) == NULL)
                return (EINVAL);
-       }
-
-       error = rtable_setsource(rtableid, src->sa_family, ifa->ifa_addr);
-       KERNEL_UNLOCK();
 
-       return (error);
+       return rtable_setsource(rtableid, src->sa_family, ifa->ifa_addr);
 }
 
 /*