With mpi's encouragement and guidance, here's a diff that reduces the
scope of the NET_LOCK() a bit.
in_control() is the only caller of mrt_ioctl() and the latter is a
simple function only requiring a read lock.
There are only a handful callers of in_ioctl(). The two switches create
relatively tangled codepaths, so this will need some refactoring before
we can push the lock further down and split the read and write cases.
For now, establish a single exit point, grab the netlock at the
beginning and release it at the end.
To be honest, I'm a bit skeptical about the change this creates in
if_umb.c where the NET_LOCK() used to protect two in_ioctl() calls
in a row. After the diff we grab and release it twice. If this is
innocent, I would love to have a better understanding of why.
Index: sys/dev/usb/if_umb.c
===================================================================
RCS file: /var/cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.18
diff -u -p -r1.18 if_umb.c
--- sys/dev/usb/if_umb.c 19 Feb 2018 08:59:52 -0000 1.18
+++ sys/dev/usb/if_umb.c 28 Apr 2018 15:55:45 -0000
@@ -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_s
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, "
Index: sys/netinet/in.c
===================================================================
RCS file: /var/cvs/src/sys/netinet/in.c,v
retrieving revision 1.149
diff -u -p -r1.149 in.c
--- sys/netinet/in.c 24 Apr 2018 19:53:38 -0000 1.149
+++ sys/netinet/in.c 28 Apr 2018 16:44:23 -0000
@@ -187,7 +187,6 @@ in_control(struct socket *so, u_long cmd
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
break;
}
- NET_UNLOCK();
return error;
}
@@ -216,13 +214,13 @@ in_ioctl(u_long cmd, caddr_t data, struc
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, struc
}
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, struc
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, struc
}
}
}
- 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, struc
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, struc
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, struc
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, struc
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, struc
break;
default:
- return (EOPNOTSUPP);
+ error = EOPNOTSUPP;
+ break;
}
- return (0);
+
+err:
+ NET_UNLOCK();
+ return (error);
}
/*
* Delete any existing route for an interface.
Index: sys/netinet/ip_mroute.c
===================================================================
RCS file: /var/cvs/src/sys/netinet/ip_mroute.c,v
retrieving revision 1.121
diff -u -p -r1.121 ip_mroute.c
--- sys/netinet/ip_mroute.c 1 Sep 2017 15:05:31 -0000 1.121
+++ sys/netinet/ip_mroute.c 28 Apr 2018 15:45:32 -0000
@@ -264,12 +264,16 @@ mrt_ioctl(struct socket *so, u_long cmd,
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;
