summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTheo Buehler <tb@cvs.openbsd.org>2018-04-30 19:07:45 +0000
committerTheo Buehler <tb@cvs.openbsd.org>2018-04-30 19:07:45 +0000
commitc5715eb7792149be764c7083fc46d6dc0081e12a (patch)
tree759d4a46814212692f13971980a70a12116b0923
parent896330ed72a6b6850a3911ff8f27c287e3eac9bd (diff)
Reduce the scope of the NET_LOCK() in in_control(). Two functions were
protected: mrt_ioctl() and in_ioctl(). The former has no other callers and only needs a read lock. The latter will need refactoring to reduce the lock's scope further. In a first step, establish a single exit point and protect most of the function body with the NET_LOCK() while removing the NET_LOCK() from a handful of callers. suggested by & ok mpi, ok visa
-rw-r--r--sys/dev/usb/if_umb.c6
-rw-r--r--sys/netinet/in.c82
-rw-r--r--sys/netinet/ip_mroute.c6
3 files changed, 56 insertions, 38 deletions
diff --git a/sys/dev/usb/if_umb.c b/sys/dev/usb/if_umb.c
index 36b15d3da6d..7fc04eb6113 100644
--- a/sys/dev/usb/if_umb.c
+++ b/sys/dev/usb/if_umb.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: if_umb.c,v 1.18 2018/02/19 08:59:52 mpi Exp $ */
+/* $OpenBSD: if_umb.c,v 1.19 2018/04/30 19:07:44 tb Exp $ */
/*
* Copyright (c) 2016 genua mbH
@@ -965,7 +965,6 @@ umb_state_task(void *arg)
*/
memset(sc->sc_info.ipv4dns, 0,
sizeof (sc->sc_info.ipv4dns));
- NET_LOCK();
if (in_ioctl(SIOCGIFADDR, (caddr_t)&ifr, ifp, 1) == 0 &&
satosin(&ifr.ifr_addr)->sin_addr.s_addr !=
INADDR_ANY) {
@@ -974,7 +973,6 @@ umb_state_task(void *arg)
sizeof (ifra.ifra_addr));
in_ioctl(SIOCDIFADDR, (caddr_t)&ifra, ifp, 1);
}
- NET_UNLOCK();
}
if_link_state_change(ifp);
}
@@ -1661,9 +1659,7 @@ umb_decode_ip_configuration(struct umb_softc *sc, void *data, int len)
sin->sin_len = sizeof (ifra.ifra_mask);
in_len2mask(&sin->sin_addr, ipv4elem.prefixlen);
- NET_LOCK();
rv = in_ioctl(SIOCAIFADDR, (caddr_t)&ifra, ifp, 1);
- NET_UNLOCK();
if (rv == 0) {
if (ifp->if_flags & IFF_DEBUG)
log(LOG_INFO, "%s: IPv4 addr %s, mask %s, "
diff --git a/sys/netinet/in.c b/sys/netinet/in.c
index eda6ac60b97..32d5f6777ad 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: in.c,v 1.149 2018/04/24 19:53:38 florian Exp $ */
+/* $OpenBSD: in.c,v 1.150 2018/04/30 19:07:44 tb Exp $ */
/* $NetBSD: in.c,v 1.26 1996/02/13 23:41:39 christos Exp $ */
/*
@@ -187,7 +187,6 @@ in_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp)
int privileged;
int error;
- NET_LOCK();
privileged = 0;
if ((so->so_state & SS_PRIV) != 0)
privileged++;
@@ -204,7 +203,6 @@ in_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp)
break;
}
- NET_UNLOCK();
return error;
}
@@ -216,13 +214,13 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
struct in_ifaddr *ia = NULL;
struct in_aliasreq *ifra = (struct in_aliasreq *)data;
struct sockaddr_in oldaddr;
- int error;
+ int error = 0;
int newifaddr;
if (ifp == NULL)
return (ENXIO);
- NET_ASSERT_LOCKED();
+ NET_LOCK();
TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
if (ifa->ifa_addr->sa_family == AF_INET) {
@@ -244,12 +242,16 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
}
ia = ifatoia(ifa);
}
- if (cmd == SIOCDIFADDR && ia == NULL)
- return (EADDRNOTAVAIL);
+ if (cmd == SIOCDIFADDR && ia == NULL) {
+ error = EADDRNOTAVAIL;
+ goto err;
+ }
/* FALLTHROUGH */
case SIOCSIFADDR:
- if (!privileged)
- return (EPERM);
+ if (!privileged) {
+ error = EPERM;
+ goto err;
+ }
if (ia == NULL) {
ia = malloc(sizeof *ia, M_IFADDR, M_WAITOK | M_ZERO);
@@ -273,8 +275,10 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
case SIOCSIFNETMASK:
case SIOCSIFDSTADDR:
case SIOCSIFBRDADDR:
- if (!privileged)
- return (EPERM);
+ if (!privileged) {
+ error = EPERM;
+ goto err;
+ }
/* FALLTHROUGH */
case SIOCGIFADDR:
@@ -291,8 +295,10 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
}
}
}
- if (ia == NULL)
- return (EADDRNOTAVAIL);
+ if (ia == NULL) {
+ error = EADDRNOTAVAIL;
+ goto err;
+ }
break;
}
switch (cmd) {
@@ -302,14 +308,18 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
break;
case SIOCGIFBRDADDR:
- if ((ifp->if_flags & IFF_BROADCAST) == 0)
- return (EINVAL);
+ if ((ifp->if_flags & IFF_BROADCAST) == 0) {
+ error = EINVAL;
+ break;
+ }
*satosin(&ifr->ifr_dstaddr) = ia->ia_broadaddr;
break;
case SIOCGIFDSTADDR:
- if ((ifp->if_flags & IFF_POINTOPOINT) == 0)
- return (EINVAL);
+ if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
+ error = EINVAL;
+ break;
+ }
*satosin(&ifr->ifr_dstaddr) = ia->ia_dstaddr;
break;
@@ -318,31 +328,36 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
break;
case SIOCSIFDSTADDR:
- if ((ifp->if_flags & IFF_POINTOPOINT) == 0)
- return (EINVAL);
+ if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
+ error = EINVAL;
+ break;
+ }
oldaddr = ia->ia_dstaddr;
ia->ia_dstaddr = *satosin(&ifr->ifr_dstaddr);
error = (*ifp->if_ioctl)(ifp, SIOCSIFDSTADDR, (caddr_t)ia);
if (error) {
ia->ia_dstaddr = oldaddr;
- return (error);
+ break;
}
in_scrubhost(ia, &oldaddr);
in_addhost(ia, &ia->ia_dstaddr);
break;
case SIOCSIFBRDADDR:
- if ((ifp->if_flags & IFF_BROADCAST) == 0)
- return (EINVAL);
+ if ((ifp->if_flags & IFF_BROADCAST) == 0) {
+ error = EINVAL;
+ break;
+ }
ifa_update_broadaddr(ifp, &ia->ia_ifa, &ifr->ifr_broadaddr);
break;
case SIOCSIFADDR:
in_ifscrub(ifp, ia);
error = in_ifinit(ifp, ia, satosin(&ifr->ifr_addr), newifaddr);
- if (!error)
- dohooks(ifp->if_addrhooks, 0);
- return (error);
+ if (error)
+ break;
+ dohooks(ifp->if_addrhooks, 0);
+ break;
case SIOCSIFNETMASK:
ia->ia_netmask = ia->ia_sockmask.sin_addr.s_addr =
@@ -352,8 +367,6 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
case SIOCAIFADDR: {
int needinit = 0;
- error = 0;
-
if (ia->ia_addr.sin_family == AF_INET) {
if (ifra->ifra_addr.sin_len == 0)
ifra->ifra_addr = ia->ia_addr;
@@ -384,9 +397,10 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
if (ifra->ifra_addr.sin_family == AF_INET && needinit) {
error = in_ifinit(ifp, ia, &ifra->ifra_addr, newifaddr);
}
- if (!error)
- dohooks(ifp->if_addrhooks, 0);
- return (error);
+ if (error)
+ break;
+ dohooks(ifp->if_addrhooks, 0);
+ break;
}
case SIOCDIFADDR:
/*
@@ -400,9 +414,13 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
break;
default:
- return (EOPNOTSUPP);
+ error = EOPNOTSUPP;
+ break;
}
- return (0);
+
+err:
+ NET_UNLOCK();
+ return (error);
}
/*
* Delete any existing route for an interface.
diff --git a/sys/netinet/ip_mroute.c b/sys/netinet/ip_mroute.c
index 1fe541524df..78ac4c52bc2 100644
--- a/sys/netinet/ip_mroute.c
+++ b/sys/netinet/ip_mroute.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ip_mroute.c,v 1.121 2017/09/01 15:05:31 mpi Exp $ */
+/* $OpenBSD: ip_mroute.c,v 1.122 2018/04/30 19:07:44 tb Exp $ */
/* $NetBSD: ip_mroute.c,v 1.85 2004/04/26 01:31:57 matt Exp $ */
/*
@@ -264,12 +264,16 @@ mrt_ioctl(struct socket *so, u_long cmd, caddr_t data)
else
switch (cmd) {
case SIOCGETVIFCNT:
+ NET_RLOCK();
error = get_vif_cnt(inp->inp_rtableid,
(struct sioc_vif_req *)data);
+ NET_RUNLOCK();
break;
case SIOCGETSGCNT:
+ NET_RLOCK();
error = get_sg_cnt(inp->inp_rtableid,
(struct sioc_sg_req *)data);
+ NET_RUNLOCK();
break;
default:
error = ENOTTY;