d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

tun/tap: close race between destroy/ioctl handler

Browse Source 
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
This commit is contained in:
kevans 2019-04-25 12:44:08 +00:00
parent 070cf1ede1
commit d61e108233
2 changed files with 36 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
sys/net/if_tap.c
View File

@ -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 */

19
sys/net/if_tun.c
View File

@ -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);
}