tuntap(4): Use make_dev_s to avoid si_drv1 race
This allows us to avoid some dance in tunopen for dealing with the possibility of dev->si_drv1 being NULL as it's set prior to the devfs node being created in all cases. There's still the possibility that the tun device hasn't been fully initialized, since that's done after the devfs node was created. Alleviate this by returning ENXIO if we're not to that point of tuncreate yet. This work is what sparked r353128, full initialization of cloned devices w/ specified make_dev_args.
This commit is contained in:
Kyle Evans
parent
486c0b2269
commit
6869d530c7
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=353785
@ -209,6 +209,8 @@ SYSCTL_INT(_net_link_tap, OID_AUTO, devfs_cloning, CTLFLAG_RWTUN, &tapdclone, 0,
|
||||
"Enable legacy devfs interface creation");
|
||||
SYSCTL_INT(_net_link_tap, OID_AUTO, debug, CTLFLAG_RW, &tundebug, 0, "");
|
||||
|
||||
static int tun_create_device(struct tuntap_driver *drv, int unit,
|
||||
struct ucred *cr, struct cdev **dev, const char *name);
|
||||
static int tun_busy_locked(struct tuntap_softc *tp);
|
||||
static void tun_unbusy_locked(struct tuntap_softc *tp);
|
||||
static int tun_busy(struct tuntap_softc *tp);
|
||||
@ -217,7 +219,7 @@ static void tun_unbusy(struct tuntap_softc *tp);
|
||||
static int tuntap_name2info(const char *name, int *unit, int *flags);
|
||||
static void tunclone(void *arg, struct ucred *cred, char *name,
|
||||
int namelen, struct cdev **dev);
|
||||
static void tuncreate(struct cdev *dev, struct tuntap_driver *);
|
||||
static void tuncreate(struct cdev *dev);
|
||||
static void tunrename(void *arg, struct ifnet *ifp);
|
||||
static int tunifioctl(struct ifnet *, u_long, caddr_t);
|
||||
static void tuninit(struct ifnet *);
|
||||
@ -544,16 +546,15 @@ tun_clone_create(struct if_clone *ifc, char *name, size_t len, caddr_t params)
|
||||
snprintf(name, IFNAMSIZ, "%s%d", drv->cdevsw.d_name, unit);
|
||||
|
||||
/* find any existing device, or allocate new unit number */
|
||||
dev = NULL;
|
||||
i = clone_create(&drv->clones, &drv->cdevsw, &unit, &dev, 0);
|
||||
if (i) {
|
||||
/* No preexisting struct cdev *, create one */
|
||||
dev = make_dev(&drv->cdevsw, unit, UID_UUCP, GID_DIALER, 0600,
|
||||
"%s%d", drv->cdevsw.d_name, unit);
|
||||
}
|
||||
/* No preexisting struct cdev *, create one */
|
||||
if (i != 0)
|
||||
i = tun_create_device(drv, unit, NULL, &dev, name);
|
||||
if (i == 0)
|
||||
tuncreate(dev);
|
||||
|
||||
tuncreate(dev, drv);
|
||||
|
||||
return (0);
|
||||
return (i);
|
||||
}
|
||||
|
||||
static void
|
||||
@ -608,12 +609,11 @@ tunclone(void *arg, struct ucred *cred, char *name, int namelen,
|
||||
name, u);
|
||||
name = devname;
|
||||
}
|
||||
/* No preexisting struct cdev *, create one */
|
||||
*dev = make_dev_credf(MAKEDEV_REF, &drv->cdevsw, u, cred,
|
||||
UID_UUCP, GID_DIALER, 0600, "%s", name);
|
||||
}
|
||||
|
||||
if_clone_create(name, namelen, NULL);
|
||||
i = tun_create_device(drv, u, cred, dev, name);
|
||||
}
|
||||
if (i == 0)
|
||||
if_clone_create(name, namelen, NULL);
|
||||
out:
|
||||
CURVNET_RESTORE();
|
||||
}
|
||||
@ -791,6 +791,46 @@ MODULE_VERSION(if_tuntap, 1);
|
||||
MODULE_VERSION(if_tun, 1);
|
||||
MODULE_VERSION(if_tap, 1);
|
||||
|
||||
static int
|
||||
tun_create_device(struct tuntap_driver *drv, int unit, struct ucred *cr,
|
||||
struct cdev **dev, const char *name)
|
||||
{
|
||||
struct make_dev_args args;
|
||||
struct tuntap_softc *tp;
|
||||
int error;
|
||||
|
||||
tp = malloc(sizeof(*tp), M_TUN, M_WAITOK | M_ZERO);
|
||||
mtx_init(&tp->tun_mtx, "tun_mtx", NULL, MTX_DEF);
|
||||
cv_init(&tp->tun_cv, "tun_condvar");
|
||||
tp->tun_flags = drv->ident_flags;
|
||||
tp->tun_drv = drv;
|
||||
|
||||
make_dev_args_init(&args);
|
||||
if (cr != NULL)
|
||||
args.mda_flags = MAKEDEV_REF;
|
||||
args.mda_devsw = &drv->cdevsw;
|
||||
args.mda_cr = cr;
|
||||
args.mda_uid = UID_UUCP;
|
||||
args.mda_gid = GID_DIALER;
|
||||
args.mda_mode = 0600;
|
||||
args.mda_unit = unit;
|
||||
args.mda_si_drv1 = tp;
|
||||
error = make_dev_s(&args, dev, "%s", name);
|
||||
if (error != 0) {
|
||||
free(tp, M_TUN);
|
||||
return (error);
|
||||
}
|
||||
|
||||
KASSERT((*dev)->si_drv1 != NULL,
|
||||
("Failed to set si_drv1 at %s creation", name));
|
||||
tp->tun_dev = *dev;
|
||||
knlist_init_mtx(&tp->tun_rsel.si_note, &tp->tun_mtx);
|
||||
mtx_lock(&tunmtx);
|
||||
TAILQ_INSERT_TAIL(&tunhead, tp, tun_list);
|
||||
mtx_unlock(&tunmtx);
|
||||
return (0);
|
||||
}
|
||||
|
||||
static void
|
||||
tunstart(struct ifnet *ifp)
|
||||
{
|
||||
@ -883,49 +923,43 @@ tunstart_l2(struct ifnet *ifp)
|
||||
TUN_UNLOCK(tp);
|
||||
} /* tunstart_l2 */
|
||||
|
||||
|
||||
/* XXX: should return an error code so it can fail. */
|
||||
static void
|
||||
tuncreate(struct cdev *dev, struct tuntap_driver *drv)
|
||||
tuncreate(struct cdev *dev)
|
||||
{
|
||||
struct tuntap_softc *sc;
|
||||
struct tuntap_driver *drv;
|
||||
struct tuntap_softc *tp;
|
||||
struct ifnet *ifp;
|
||||
struct ether_addr eaddr;
|
||||
int iflags;
|
||||
u_char type;
|
||||
|
||||
sc = malloc(sizeof(*sc), M_TUN, M_WAITOK | M_ZERO);
|
||||
mtx_init(&sc->tun_mtx, "tun_mtx", NULL, MTX_DEF);
|
||||
cv_init(&sc->tun_cv, "tun_condvar");
|
||||
sc->tun_flags = drv->ident_flags;
|
||||
sc->tun_dev = dev;
|
||||
sc->tun_drv = drv;
|
||||
mtx_lock(&tunmtx);
|
||||
TAILQ_INSERT_TAIL(&tunhead, sc, tun_list);
|
||||
mtx_unlock(&tunmtx);
|
||||
tp = dev->si_drv1;
|
||||
KASSERT(tp != NULL,
|
||||
("si_drv1 should have been initialized at creation"));
|
||||
|
||||
drv = tp->tun_drv;
|
||||
iflags = IFF_MULTICAST;
|
||||
if ((sc->tun_flags & TUN_L2) != 0) {
|
||||
if ((tp->tun_flags & TUN_L2) != 0) {
|
||||
type = IFT_ETHER;
|
||||
iflags |= IFF_BROADCAST | IFF_SIMPLEX;
|
||||
} else {
|
||||
type = IFT_PPP;
|
||||
iflags |= IFF_POINTOPOINT;
|
||||
}
|
||||
ifp = sc->tun_ifp = if_alloc(type);
|
||||
ifp = tp->tun_ifp = if_alloc(type);
|
||||
if (ifp == NULL)
|
||||
panic("%s%d: failed to if_alloc() interface.\n",
|
||||
drv->cdevsw.d_name, dev2unit(dev));
|
||||
ifp->if_softc = sc;
|
||||
ifp->if_softc = tp;
|
||||
if_initname(ifp, drv->cdevsw.d_name, dev2unit(dev));
|
||||
ifp->if_ioctl = tunifioctl;
|
||||
ifp->if_flags = iflags;
|
||||
IFQ_SET_MAXLEN(&ifp->if_snd, ifqmaxlen);
|
||||
knlist_init_mtx(&sc->tun_rsel.si_note, &sc->tun_mtx);
|
||||
ifp->if_capabilities |= IFCAP_LINKSTATE;
|
||||
ifp->if_capenable |= IFCAP_LINKSTATE;
|
||||
|
||||
if ((sc->tun_flags & TUN_L2) != 0) {
|
||||
if ((tp->tun_flags & TUN_L2) != 0) {
|
||||
ifp->if_mtu = ETHERMTU;
|
||||
ifp->if_init = tunifinit;
|
||||
ifp->if_start = tunstart_l2;
|
||||
@ -943,11 +977,10 @@ tuncreate(struct cdev *dev, struct tuntap_driver *drv)
|
||||
if_attach(ifp);
|
||||
bpfattach(ifp, DLT_NULL, sizeof(u_int32_t));
|
||||
}
|
||||
dev->si_drv1 = sc;
|
||||
|
||||
TUN_LOCK(sc);
|
||||
sc->tun_flags |= TUN_INITED;
|
||||
TUN_UNLOCK(sc);
|
||||
TUN_LOCK(tp);
|
||||
tp->tun_flags |= TUN_INITED;
|
||||
TUN_UNLOCK(tp);
|
||||
|
||||
TUNDEBUG(ifp, "interface %s is created, minor = %#x\n",
|
||||
ifp->if_xname, dev2unit(dev));
|
||||
@ -1008,7 +1041,6 @@ static int
|
||||
tunopen(struct cdev *dev, int flag, int mode, struct thread *td)
|
||||
{
|
||||
struct ifnet *ifp;
|
||||
struct tuntap_driver *drv;
|
||||
struct tuntap_softc *tp;
|
||||
int error, tunflags;
|
||||
|
||||
@ -1031,22 +1063,16 @@ tunopen(struct cdev *dev, int flag, int mode, struct thread *td)
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* XXXRW: Non-atomic test and set of dev->si_drv1 requires
|
||||
* synchronization.
|
||||
*/
|
||||
tp = dev->si_drv1;
|
||||
if (!tp) {
|
||||
drv = tuntap_driver_from_flags(tunflags);
|
||||
if (drv == NULL) {
|
||||
CURVNET_RESTORE();
|
||||
return (ENXIO);
|
||||
}
|
||||
tuncreate(dev, drv);
|
||||
tp = dev->si_drv1;
|
||||
}
|
||||
KASSERT(tp != NULL,
|
||||
("si_drv1 should have been initialized at creation"));
|
||||
|
||||
TUN_LOCK(tp);
|
||||
if ((tp->tun_flags & TUN_INITED) == 0) {
|
||||
TUN_UNLOCK(tp);
|
||||
CURVNET_RESTORE();
|
||||
return (ENXIO);
|
||||
}
|
||||
if ((tp->tun_flags & (TUN_OPEN | TUN_DYING)) != 0) {
|
||||
TUN_UNLOCK(tp);
|
||||
CURVNET_RESTORE();
|
||||
|
||||
Loading…
Reference in New Issue
Block a user