svn commit: r353785 - head/sys/net
Kyle Evans
kevans at FreeBSD.org
Sun Oct 20 22:39:41 UTC 2019
Author: kevans
Date: Sun Oct 20 22:39:40 2019
New Revision: 353785
URL: https://svnweb.freebsd.org/changeset/base/353785
Log:
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.
Modified:
head/sys/net/if_tuntap.c
Modified: head/sys/net/if_tuntap.c
==============================================================================
--- head/sys/net/if_tuntap.c Sun Oct 20 22:05:57 2019 (r353784)
+++ head/sys/net/if_tuntap.c Sun Oct 20 22:39:40 2019 (r353785)
@@ -209,6 +209,8 @@ SYSCTL_INT(_net_link_tap, OID_AUTO, devfs_cloning, CTL
"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, siz
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, in
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 t
}
}
- /*
- * 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();
More information about the svn-src-head
mailing list