Author: kevans Date: Thu Apr 25 12:44:08 2019 New Revision: 346670 URL: https://svnweb.freebsd.org/changeset/base/346670
Log: tun/tap: close race between destroy/ioctl handler It seems that there should be a better way to handle this, but this seems to be the more common approach and it should likely get replaced in all of the places it happens... Basically, thread 1 is in the process of destroying the tun/tap while thread 2 is executing one of the ioctls that requires the tun/tap mutex and the mutex is destroyed before the ioctl handler can acquire it. This is only one of the races described/found in PR 233955. PR: 233955 Reviewed by: ae MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D20027 Modified: head/sys/net/if_tap.c head/sys/net/if_tun.c Modified: head/sys/net/if_tap.c ============================================================================== --- head/sys/net/if_tap.c Thu Apr 25 12:02:17 2019 (r346669) +++ head/sys/net/if_tap.c Thu Apr 25 12:44:08 2019 (r346670) @@ -41,6 +41,7 @@ #include <sys/param.h> #include <sys/conf.h> +#include <sys/lock.h> #include <sys/fcntl.h> #include <sys/filio.h> #include <sys/jail.h> @@ -55,6 +56,7 @@ #include <sys/signalvar.h> #include <sys/socket.h> #include <sys/sockio.h> +#include <sys/sx.h> #include <sys/sysctl.h> #include <sys/systm.h> #include <sys/ttycom.h> @@ -163,6 +165,9 @@ MALLOC_DECLARE(M_TAP); MALLOC_DEFINE(M_TAP, CDEV_NAME, "Ethernet tunnel interface"); SYSCTL_INT(_debug, OID_AUTO, if_tap_debug, CTLFLAG_RW, &tapdebug, 0, ""); +static struct sx tap_ioctl_sx; +SX_SYSINIT(tap_ioctl_sx, &tap_ioctl_sx, "tap_ioctl"); + SYSCTL_DECL(_net_link); static SYSCTL_NODE(_net_link, OID_AUTO, tap, CTLFLAG_RW, 0, "Ethernet tunnel software network interface"); @@ -217,6 +222,10 @@ tap_destroy(struct tap_softc *tp) struct ifnet *ifp = tp->tap_ifp; CURVNET_SET(ifp->if_vnet); + sx_xlock(&tap_ioctl_sx); + ifp->if_softc = NULL; + sx_xunlock(&tap_ioctl_sx); + destroy_dev(tp->tap_dev); seldrain(&tp->tap_rsel); knlist_clear(&tp->tap_rsel.si_note, 0); @@ -600,12 +609,18 @@ tapifinit(void *xtp) static int tapifioctl(struct ifnet *ifp, u_long cmd, caddr_t data) { - struct tap_softc *tp = ifp->if_softc; + struct tap_softc *tp; struct ifreq *ifr = (struct ifreq *)data; struct ifstat *ifs = NULL; struct ifmediareq *ifmr = NULL; int dummy, error = 0; + sx_xlock(&tap_ioctl_sx); + tp = ifp->if_softc; + if (tp == NULL) { + error = ENXIO; + goto bad; + } switch (cmd) { case SIOCSIFFLAGS: /* XXX -- just like vmnet does */ case SIOCADDMULTI: @@ -648,6 +663,8 @@ tapifioctl(struct ifnet *ifp, u_long cmd, caddr_t data break; } +bad: + sx_xunlock(&tap_ioctl_sx); return (error); } /* tapifioctl */ Modified: head/sys/net/if_tun.c ============================================================================== --- head/sys/net/if_tun.c Thu Apr 25 12:02:17 2019 (r346669) +++ head/sys/net/if_tun.c Thu Apr 25 12:44:08 2019 (r346670) @@ -20,6 +20,7 @@ #include "opt_inet6.h" #include <sys/param.h> +#include <sys/lock.h> #include <sys/priv.h> #include <sys/proc.h> #include <sys/systm.h> @@ -30,6 +31,7 @@ #include <sys/fcntl.h> #include <sys/filio.h> #include <sys/sockio.h> +#include <sys/sx.h> #include <sys/ttycom.h> #include <sys/poll.h> #include <sys/selinfo.h> @@ -115,6 +117,9 @@ static struct clonedevs *tunclones; static TAILQ_HEAD(,tun_softc) tunhead = TAILQ_HEAD_INITIALIZER(tunhead); SYSCTL_INT(_debug, OID_AUTO, if_tun_debug, CTLFLAG_RW, &tundebug, 0, ""); +static struct sx tun_ioctl_sx; +SX_SYSINIT(tun_ioctl_sx, &tun_ioctl_sx, "tun_ioctl"); + SYSCTL_DECL(_net_link); static SYSCTL_NODE(_net_link, OID_AUTO, tun, CTLFLAG_RW, 0, "IP tunnel software network interface."); @@ -278,6 +283,10 @@ tun_destroy(struct tun_softc *tp) mtx_unlock(&tp->tun_mtx); CURVNET_SET(TUN2IFP(tp)->if_vnet); + sx_xlock(&tun_ioctl_sx); + TUN2IFP(tp)->if_softc = NULL; + sx_xunlock(&tun_ioctl_sx); + dev = tp->tun_dev; bpfdetach(TUN2IFP(tp)); if_detach(TUN2IFP(tp)); @@ -588,10 +597,16 @@ static int tunifioctl(struct ifnet *ifp, u_long cmd, caddr_t data) { struct ifreq *ifr = (struct ifreq *)data; - struct tun_softc *tp = ifp->if_softc; + struct tun_softc *tp; struct ifstat *ifs; int error = 0; + sx_xlock(&tun_ioctl_sx); + tp = ifp->if_softc; + if (tp == NULL) { + error = ENXIO; + goto bad; + } switch(cmd) { case SIOCGIFSTATUS: ifs = (struct ifstat *)data; @@ -618,6 +633,8 @@ tunifioctl(struct ifnet *ifp, u_long cmd, caddr_t data default: error = EINVAL; } +bad: + sx_xunlock(&tun_ioctl_sx); return (error); } _______________________________________________ [email protected] mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "[email protected]"
