Module Name: src Committed By: ozaki-r Date: Wed Nov 22 03:03:18 UTC 2017
Modified Files: src/sys/kern: sys_socket.c src/sys/net: if.c if.h if_media.c if_vlan.c Log Message: Hold KERNEL_LOCK on if_ioctl selectively based on IFEF_MPSAFE If IFEF_MPSAFE is set, hold the lock and otherwise don't hold. This change requires additions of KERNEL_LOCK to subsequence functions from if_ioctl such as ifmedia_ioctl and ifioctl_common to protect non-MP-safe components. Proposed on tech-kern@ and tech-net@ To generate a diff of this commit: cvs rdiff -u -r1.74 -r1.75 src/sys/kern/sys_socket.c cvs rdiff -u -r1.398 -r1.399 src/sys/net/if.c cvs rdiff -u -r1.243 -r1.244 src/sys/net/if.h cvs rdiff -u -r1.34 -r1.35 src/sys/net/if_media.c cvs rdiff -u -r1.108 -r1.109 src/sys/net/if_vlan.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/kern/sys_socket.c diff -u src/sys/kern/sys_socket.c:1.74 src/sys/kern/sys_socket.c:1.75 --- src/sys/kern/sys_socket.c:1.74 Thu Jul 7 06:55:43 2016 +++ src/sys/kern/sys_socket.c Wed Nov 22 03:03:18 2017 @@ -1,4 +1,4 @@ -/* $NetBSD: sys_socket.c,v 1.74 2016/07/07 06:55:43 msaitoh Exp $ */ +/* $NetBSD: sys_socket.c,v 1.75 2017/11/22 03:03:18 ozaki-r Exp $ */ /*- * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc. @@ -61,7 +61,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: sys_socket.c,v 1.74 2016/07/07 06:55:43 msaitoh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: sys_socket.c,v 1.75 2017/11/22 03:03:18 ozaki-r Exp $"); #include <sys/param.h> #include <sys/systm.h> @@ -197,14 +197,18 @@ soo_ioctl(file_t *fp, u_long cmd, void * * 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; } Index: src/sys/net/if.c diff -u src/sys/net/if.c:1.398 src/sys/net/if.c:1.399 --- src/sys/net/if.c:1.398 Sun Nov 19 16:59:25 2017 +++ src/sys/net/if.c Wed Nov 22 03:03:18 2017 @@ -1,4 +1,4 @@ -/* $NetBSD: if.c,v 1.398 2017/11/19 16:59:25 christos Exp $ */ +/* $NetBSD: if.c,v 1.399 2017/11/22 03:03:18 ozaki-r Exp $ */ /*- * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc. @@ -90,7 +90,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.398 2017/11/19 16:59:25 christos Exp $"); +__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.399 2017/11/22 03:03:18 ozaki-r Exp $"); #if defined(_KERNEL_OPT) #include "opt_inet.h" @@ -2766,6 +2766,11 @@ ifioctl_common(struct ifnet *ifp, u_long 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. + */ + KERNEL_LOCK_IF_IFP_MPSAFE(ifp); if (ifp->if_flags & IFF_UP && (ifr->ifr_flags & IFF_UP) == 0) { s = splsoftnet(); if_down(ifp); @@ -2776,6 +2781,7 @@ ifioctl_common(struct ifnet *ifp, u_long if_up(ifp); splx(s); } + KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp); ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) | (ifr->ifr_flags &~ IFF_CANTCHANGE); break; @@ -2847,8 +2853,10 @@ ifioctl_common(struct ifnet *ifp, u_long * If the link MTU changed, do network layer specific procedure. */ #ifdef INET6 + KERNEL_LOCK_UNLESS_NET_MPSAFE(); if (in6_present) nd6_setmtu(ifp); + KERNEL_UNLOCK_UNLESS_NET_MPSAFE(); #endif return ENETRESET; default: @@ -2992,11 +3000,13 @@ doifioctl(struct socket *so, u_long cmd, return error; } } + KERNEL_LOCK_UNLESS_NET_MPSAFE(); 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); + KERNEL_UNLOCK_UNLESS_NET_MPSAFE(); curlwp_bindx(bound); return r; @@ -3054,6 +3064,7 @@ doifioctl(struct socket *so, u_long cmd, oif_flags = ifp->if_flags; + KERNEL_LOCK_UNLESS_IFP_MPSAFE(ifp); mutex_enter(ifp->if_ioctl_lock); error = (*ifp->if_ioctl)(ifp, cmd, data); @@ -3062,6 +3073,7 @@ doifioctl(struct socket *so, u_long cmd, else if (so->so_proto == NULL) error = EOPNOTSUPP; else { + KERNEL_LOCK_IF_IFP_MPSAFE(ifp); #ifdef COMPAT_OSOCK if (vec_compat_ifioctl != NULL) error = (*vec_compat_ifioctl)(so, ocmd, cmd, data, l); @@ -3069,6 +3081,7 @@ doifioctl(struct socket *so, u_long cmd, #endif error = (*so->so_proto->pr_usrreqs->pr_ioctl)(so, cmd, data, ifp); + KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp); } if (((oif_flags ^ ifp->if_flags) & IFF_UP) != 0) { @@ -3084,6 +3097,7 @@ doifioctl(struct socket *so, u_long cmd, #endif mutex_exit(ifp->if_ioctl_lock); + KERNEL_UNLOCK_UNLESS_IFP_MPSAFE(ifp); out: if_put(ifp, &psref); curlwp_bindx(bound); Index: src/sys/net/if.h diff -u src/sys/net/if.h:1.243 src/sys/net/if.h:1.244 --- src/sys/net/if.h:1.243 Fri Nov 17 07:37:12 2017 +++ src/sys/net/if.h Wed Nov 22 03:03:18 2017 @@ -1,4 +1,4 @@ -/* $NetBSD: if.h,v 1.243 2017/11/17 07:37:12 ozaki-r Exp $ */ +/* $NetBSD: if.h,v 1.244 2017/11/22 03:03:18 ozaki-r Exp $ */ /*- * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc. @@ -391,9 +391,11 @@ typedef struct ifnet { #define IFEF_NO_LINK_STATE_CHANGE __BIT(1) /* doesn't use link state interrupts */ /* - * if_XXX() handlers that IFEF_MPSAFE suppresses KERNEL_LOCK: + * The following if_XXX() handlers don't take KERNEL_LOCK if the interface + * is set IFEF_MPSAFE: * - if_start * - if_output + * - if_ioctl */ #ifdef _KERNEL @@ -441,6 +443,16 @@ if_is_link_state_changeable(struct ifnet return ((ifp->if_extflags & IFEF_NO_LINK_STATE_CHANGE) == 0); } +#define KERNEL_LOCK_IF_IFP_MPSAFE(ifp) \ + do { if (if_is_mpsafe(ifp)) { KERNEL_LOCK(1, NULL); } } while (0) +#define KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp) \ + do { if (if_is_mpsafe(ifp)) { KERNEL_UNLOCK_ONE(NULL); } } while (0) + +#define KERNEL_LOCK_UNLESS_IFP_MPSAFE(ifp) \ + do { if (!if_is_mpsafe(ifp)) { KERNEL_LOCK(1, NULL); } } while (0) +#define KERNEL_UNLOCK_UNLESS_IFP_MPSAFE(ifp) \ + do { if (!if_is_mpsafe(ifp)) { KERNEL_UNLOCK_ONE(NULL); } } while (0) + #ifdef _KERNEL_OPT #include "opt_net_mpsafe.h" #endif Index: src/sys/net/if_media.c diff -u src/sys/net/if_media.c:1.34 src/sys/net/if_media.c:1.35 --- src/sys/net/if_media.c:1.34 Mon Oct 23 03:54:40 2017 +++ src/sys/net/if_media.c Wed Nov 22 03:03:18 2017 @@ -1,4 +1,4 @@ -/* $NetBSD: if_media.c,v 1.34 2017/10/23 03:54:40 msaitoh Exp $ */ +/* $NetBSD: if_media.c,v 1.35 2017/11/22 03:03:18 ozaki-r Exp $ */ /*- * Copyright (c) 1998 The NetBSD Foundation, Inc. @@ -76,7 +76,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: if_media.c,v 1.34 2017/10/23 03:54:40 msaitoh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: if_media.c,v 1.35 2017/11/22 03:03:18 ozaki-r Exp $"); #include <sys/param.h> #include <sys/systm.h> @@ -225,8 +225,8 @@ ifmedia_set(struct ifmedia *ifm, int tar /* * 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,22 @@ ifmedia_ioctl(struct ifnet *ifp, struct 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. + */ + KERNEL_LOCK_IF_IFP_MPSAFE(ifp); + e = _ifmedia_ioctl(ifp, ifr, ifm, cmd); + KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp); + return e; +} + /* * Find media entry matching a given ifm word. */ Index: src/sys/net/if_vlan.c diff -u src/sys/net/if_vlan.c:1.108 src/sys/net/if_vlan.c:1.109 --- src/sys/net/if_vlan.c:1.108 Wed Nov 22 02:35:24 2017 +++ src/sys/net/if_vlan.c Wed Nov 22 03:03:18 2017 @@ -1,4 +1,4 @@ -/* $NetBSD: if_vlan.c,v 1.108 2017/11/22 02:35:24 msaitoh Exp $ */ +/* $NetBSD: if_vlan.c,v 1.109 2017/11/22 03:03:18 ozaki-r Exp $ */ /*- * Copyright (c) 2000, 2001 The NetBSD Foundation, Inc. @@ -78,7 +78,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.108 2017/11/22 02:35:24 msaitoh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.109 2017/11/22 03:03:18 ozaki-r Exp $"); #ifdef _KERNEL_OPT #include "opt_inet.h" @@ -245,6 +245,16 @@ 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; + KERNEL_LOCK_UNLESS_NET_MPSAFE(); + e = ifpromisc(ifp, pswitch); + KERNEL_UNLOCK_UNLESS_NET_MPSAFE(); + return e; +} + void vlanattach(int n) { @@ -612,13 +622,15 @@ vlan_unconfig_locked(struct ifvlan *ifv, kmem_free(omib, sizeof(*omib)); #ifdef INET6 + KERNEL_LOCK_UNLESS_NET_MPSAFE(); /* To delete v6 link local addresses */ if (in6_present) in6_ifdetach(ifp); + KERNEL_UNLOCK_UNLESS_NET_MPSAFE(); #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; @@ -834,13 +846,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; } @@ -917,7 +929,7 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd 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); @@ -1114,7 +1126,10 @@ vlan_ether_addmulti(struct ifvlan *ifv, LIST_INSERT_HEAD(&ifv->ifv_mc_listhead, mc, mc_entries); mib = ifv->ifv_mib; + + KERNEL_LOCK_UNLESS_IFP_MPSAFE(mib->ifvm_p); error = if_mcast_op(mib->ifvm_p, SIOCADDMULTI, sa); + KERNEL_UNLOCK_UNLESS_IFP_MPSAFE(mib->ifvm_p); if (error != 0) goto ioctl_failed;