commit 2ec577b5da1e7ce196c3ec8101d8feb0ca61211e Author: Ryota Ozaki Date: Wed Nov 15 10:20:01 2017 +0900 Hold KERNEL_LOCK on if_ioctl seclectively based on IFEF_MPSAFE diff --git a/sys/kern/sys_socket.c b/sys/kern/sys_socket.c index 595fe89f2af..e4271ce4c07 100644 --- a/sys/kern/sys_socket.c +++ b/sys/kern/sys_socket.c @@ -197,14 +197,18 @@ soo_ioctl(file_t *fp, u_long cmd, void *data) * interface and routing ioctls should have a * different entry since a socket's unnecessary */ - KERNEL_LOCK(1, NULL); if (IOCGROUP(cmd) == 'i') + /* + * KERNEL_LOCK will be held later if if_ioctl() of the + * interface isn't MP-safe. + */ error = ifioctl(so, cmd, data, curlwp); else { + KERNEL_LOCK(1, NULL); error = (*so->so_proto->pr_usrreqs->pr_ioctl)(so, cmd, data, NULL); + KERNEL_UNLOCK_ONE(NULL); } - KERNEL_UNLOCK_ONE(NULL); break; } diff --git a/sys/net/if.c b/sys/net/if.c index 370511f9859..4d554b6848f 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -2771,6 +2771,12 @@ ifioctl_common(struct ifnet *ifp, u_long cmd, void *data) return 0; case SIOCSIFFLAGS: ifr = data; + /* + * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here, but if_up + * and if_down aren't MP-safe yet, so we must hold the lock. + */ + if (if_is_mpsafe(ifp)) + KERNEL_LOCK(1, NULL); if (ifp->if_flags & IFF_UP && (ifr->ifr_flags & IFF_UP) == 0) { s = splsoftnet(); if_down(ifp); @@ -2781,6 +2787,8 @@ ifioctl_common(struct ifnet *ifp, u_long cmd, void *data) if_up(ifp); splx(s); } + if (if_is_mpsafe(ifp)) + KERNEL_UNLOCK_ONE(NULL); ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) | (ifr->ifr_flags &~ IFF_CANTCHANGE); break; @@ -2852,9 +2860,15 @@ ifioctl_common(struct ifnet *ifp, u_long cmd, void *data) * If the link MTU changed, do network layer specific procedure. */ #ifdef INET6 +#ifndef NET_MPSAFE + KERNEL_LOCK(1, NULL); +#endif if (in6_present) nd6_setmtu(ifp); +#ifndef NET_MPSAFE + KERNEL_UNLOCK_ONE(NULL); #endif +#endif /* INET6 */ return ENETRESET; default: return ENOTTY; @@ -2997,11 +3011,17 @@ doifioctl(struct socket *so, u_long cmd, void *data, struct lwp *l) return error; } } +#ifndef NET_MPSAFE + KERNEL_LOCK(1, NULL); +#endif mutex_enter(&if_clone_mtx); r = (cmd == SIOCIFCREATE) ? if_clone_create(ifr->ifr_name) : if_clone_destroy(ifr->ifr_name); mutex_exit(&if_clone_mtx); +#ifndef NET_MPSAFE + KERNEL_UNLOCK_ONE(NULL); +#endif curlwp_bindx(bound); return r; @@ -3059,6 +3079,8 @@ doifioctl(struct socket *so, u_long cmd, void *data, struct lwp *l) oif_flags = ifp->if_flags; + if (!if_is_mpsafe(ifp)) + KERNEL_LOCK(1, NULL); mutex_enter(ifp->if_ioctl_lock); error = (*ifp->if_ioctl)(ifp, cmd, data); @@ -3089,6 +3111,8 @@ doifioctl(struct socket *so, u_long cmd, void *data, struct lwp *l) #endif mutex_exit(ifp->if_ioctl_lock); + if (!if_is_mpsafe(ifp)) + KERNEL_UNLOCK_ONE(NULL); out: if_put(ifp, &psref); curlwp_bindx(bound); diff --git a/sys/net/if_media.c b/sys/net/if_media.c index 5b5718de069..98b63a153f1 100644 --- a/sys/net/if_media.c +++ b/sys/net/if_media.c @@ -225,8 +225,8 @@ ifmedia_set(struct ifmedia *ifm, int target) /* * Device-independent media ioctl support function. */ -int -ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm, +static int +_ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm, u_long cmd) { struct ifmedia_entry *match; @@ -360,6 +360,24 @@ ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm, return error; } +int +ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm, + u_long cmd) +{ + int e; + + /* + * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here, but _ifmedia_ioctl + * isn't MP-safe yet, so we must hold the lock. + */ + if (if_is_mpsafe(ifp)) + KERNEL_LOCK(1, NULL); + e = _ifmedia_ioctl(ifp, ifr, ifm, cmd); + if (if_is_mpsafe(ifp)) + KERNEL_UNLOCK_ONE(NULL); + return e; +} + /* * Find media entry matching a given ifm word. */ diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c index b69bd1b4370..82f67d5b2bc 100644 --- a/sys/net/if_vlan.c +++ b/sys/net/if_vlan.c @@ -245,6 +245,20 @@ struct if_clone vlan_cloner = /* Used to pad ethernet frames with < ETHER_MIN_LEN bytes */ static char vlan_zero_pad_buff[ETHER_MIN_LEN]; +static inline int +vlan_safe_ifpromisc(struct ifnet *ifp, int pswitch) +{ + int e; +#ifndef NET_MPSAFE + KERNEL_LOCK(1, NULL); +#endif + e = ifpromisc(ifp, pswitch); +#ifndef NET_MPSAFE + KERNEL_UNLOCK_ONE(NULL); +#endif + return e; +} + void vlanattach(int n) { @@ -611,13 +625,19 @@ vlan_unconfig_locked(struct ifvlan *ifv, struct ifvlan_linkmib *nmib) kmem_free(omib, sizeof(*omib)); #ifdef INET6 +#ifndef NET_MPSAFE + KERNEL_LOCK(1, NULL); +#endif /* To delete v6 link local addresses */ if (in6_present) in6_ifdetach(ifp); +#ifndef NET_MPSAFE + KERNEL_UNLOCK_ONE(NULL); +#endif #endif if ((ifp->if_flags & IFF_PROMISC) != 0) - ifpromisc(ifp, 0); + vlan_safe_ifpromisc(ifp, 0); if_down(ifp); ifp->if_flags &= ~(IFF_UP|IFF_RUNNING); ifp->if_capabilities = 0; @@ -833,13 +853,13 @@ vlan_set_promisc(struct ifnet *ifp) if ((ifp->if_flags & IFF_PROMISC) != 0) { if ((ifv->ifv_flags & IFVF_PROMISC) == 0) { - error = ifpromisc(mib->ifvm_p, 1); + error = vlan_safe_ifpromisc(mib->ifvm_p, 1); if (error == 0) ifv->ifv_flags |= IFVF_PROMISC; } } else { if ((ifv->ifv_flags & IFVF_PROMISC) != 0) { - error = ifpromisc(mib->ifvm_p, 0); + error = vlan_safe_ifpromisc(mib->ifvm_p, 0); if (error == 0) ifv->ifv_flags &= ~IFVF_PROMISC; } @@ -916,7 +936,7 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd, void *data) if (mib->ifvm_p != NULL && (ifp->if_flags & IFF_PROMISC) != 0) - error = ifpromisc(mib->ifvm_p, 0); + error = vlan_safe_ifpromisc(mib->ifvm_p, 0); vlan_putref_linkmib(mib, &psref); curlwp_bindx(bound); @@ -1113,7 +1133,12 @@ vlan_ether_addmulti(struct ifvlan *ifv, struct ifreq *ifr) LIST_INSERT_HEAD(&ifv->ifv_mc_listhead, mc, mc_entries); mib = ifv->ifv_mib; + + if (!if_is_mpsafe(mib->ifvm_p)) + KERNEL_LOCK(1, NULL); error = if_mcast_op(mib->ifvm_p, SIOCADDMULTI, sa); + if (!if_is_mpsafe(mib->ifvm_p)) + KERNEL_UNLOCK_ONE(NULL); if (error != 0) goto ioctl_failed;