kern/105228: [PATCH] if_clone support for tun(4) and tap(4)
Landon Fuller
landonf at threerings.net
Mon Nov 6 22:00:32 UTC 2006
>Number: 105228
>Category: kern
>Synopsis: [PATCH] if_clone support for tun(4) and tap(4)
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Mon Nov 06 22:00:30 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator: Landon Fuller
>Release: FreeBSD 7.0-CURRENT i386
>Organization:
Three Rings Design, Inc.
>Environment:
System: FreeBSD freebsd-current.localdomain 7.0-CURRENT FreeBSD 7.0-CURRENT #3: Thu Oct 26 15:45:10 PDT 2006 root at freebsd-current.localdomain:/usr/obj/usr/src/sys/GENERIC i386
>Description:
The attached patch (patch-tuntap_ifclone) adds interface cloning support to the tun(4) and tap(4) drivers.
I maintained backwards-compatible support for devfs cloning, which is now disabled by default and must be
enabled via a sysctl. Tun/tap interfaces that are created via devfs cloning may still be removed via
ifconfig destroy.
Additionally, I stumbled across a bug in the clone_create() function triggered by my changes --
due to an (unanticipated) unsigned integer comparison, devices are added out of order. GCC
promotes the comparison to unsigned due to ORing one of the operands with an unsigned value:
if (u < (unit | extra))
The fix is to simply cast the result as signed. A patch (patch-kern_conf.c) is also attached containing
this fix.
>How-To-Repeat:
To test: ifconfig tun0 create; ifconfig tun0 destroy
>Fix:
See attached patches.
--- patch-tuntap_ifclone begins here ---
--- sys/net/if_tap.c Wed Sep 27 12:57:01 2006
+++ sys/net/if_tap.c Fri Nov 3 15:33:32 2006
@@ -61,6 +61,7 @@
#include <net/bpf.h>
#include <net/ethernet.h>
#include <net/if.h>
+#include <net/if_clone.h>
#include <net/if_dl.h>
#include <net/route.h>
#include <net/if_types.h>
@@ -76,8 +77,8 @@
#define TAP "tap"
#define VMNET "vmnet"
-#define TAPMAXUNIT 0x7fff
#define VMNET_DEV_MASK CLONE_FLAG0
+#define TAPMAXUNIT 0x7fff
/* module */
static int tapmodevent(module_t, int, void *);
@@ -85,13 +86,21 @@
/* device */
static void tapclone(void *, struct ucred *, char *, int,
struct cdev **);
-static void tapcreate(struct cdev *);
+static void tapcreate(struct cdev *, int);
/* network interface */
static void tapifstart(struct ifnet *);
static int tapifioctl(struct ifnet *, u_long, caddr_t);
static void tapifinit(void *);
+static int tap_clone_create(struct if_clone *, int, caddr_t);
+static void tap_clone_destroy(struct ifnet *);
+static int vmnet_clone_create(struct if_clone *, int, caddr_t);
+static void vmnet_clone_destroy(struct ifnet *);
+
+IFC_SIMPLE_DECLARE(tap, 0);
+IFC_SIMPLE_DECLARE(vmnet, 0);
+
/* character device */
static d_open_t tapopen;
static d_close_t tapclose;
@@ -141,6 +150,7 @@
static struct mtx tapmtx;
static int tapdebug = 0; /* debug flag */
static int tapuopen = 0; /* allow user open() */
+static int tapdclone = 0; /* enable devfs cloning support */
static SLIST_HEAD(, tap_softc) taphead; /* first device */
static struct clonedevs *tapclones;
@@ -153,10 +163,93 @@
"Ethernet tunnel software network interface");
SYSCTL_INT(_net_link_tap, OID_AUTO, user_open, CTLFLAG_RW, &tapuopen, 0,
"Allow user to open /dev/tap (based on node permissions)");
+SYSCTL_INT(_net_link_tap, OID_AUTO, devfs_cloning, CTLFLAG_RW, &tapdclone, 0,
+ "Enably legacy devfs interface creation");
SYSCTL_INT(_net_link_tap, OID_AUTO, debug, CTLFLAG_RW, &tapdebug, 0, "");
DEV_MODULE(if_tap, tapmodevent, NULL);
+static int
+tap_clone_create(struct if_clone *ifc, int unit, caddr_t params)
+{
+ struct cdev *dev;
+ int i;
+ u_int extra;
+
+ if (strcmp(ifc->ifc_name, VMNET) == 0)
+ extra = VMNET_DEV_MASK;
+ else
+ extra = 0;
+
+ /* find any existing device, or allocate new unit number */
+ i = clone_create(&tapclones, &tap_cdevsw, &unit, &dev, extra);
+ if (i) {
+ dev = make_dev(&tap_cdevsw, unit2minor(unit | extra),
+ UID_ROOT, GID_WHEEL, 0600, "%s%d", ifc->ifc_name, unit);
+ if (dev != NULL) {
+ dev_ref(dev);
+ dev->si_flags |= SI_CHEAPCLONE;
+ }
+ }
+
+ if (extra == VMNET_DEV_MASK) {
+ /* vmnet device */
+ tapcreate(dev, 1);
+ } else {
+ /* tap device */
+ tapcreate(dev, 0);
+ }
+
+ return (0);
+}
+
+/* vmnet devices are tap devices in disguise */
+static int
+vmnet_clone_create(struct if_clone *ifc, int unit, caddr_t params)
+{
+ return tap_clone_create(ifc, unit, params);
+}
+
+static void
+tap_destroy(struct tap_softc *tp)
+{
+ struct ifnet *ifp = tp->tap_ifp;
+ int s;
+
+ /* Unlocked read. */
+ KASSERT(!(tp->tap_flags & TAP_OPEN),
+ ("%s flags is out of sync", ifp->if_xname));
+
+ knlist_destroy(&tp->tap_rsel.si_note);
+ destroy_dev(tp->tap_dev);
+ s = splimp();
+ ether_ifdetach(ifp);
+ if_free_type(ifp, IFT_ETHER);
+ splx(s);
+
+ mtx_destroy(&tp->tap_mtx);
+ free(tp, M_TAP);
+}
+
+static void
+tap_clone_destroy(struct ifnet *ifp)
+{
+ struct tap_softc *tp = ifp->if_softc;
+
+ mtx_lock(&tapmtx);
+ SLIST_REMOVE(&taphead, tp, tap_softc, tap_next);
+ mtx_unlock(&tapmtx);
+ tap_destroy(tp);
+}
+
+/* vmnet devices are tap devices in disguise */
+static void
+vmnet_clone_destroy(struct ifnet *ifp)
+{
+ tap_clone_destroy(ifp);
+}
+
+
/*
* tapmodevent
*
@@ -168,7 +261,6 @@
static eventhandler_tag eh_tag = NULL;
struct tap_softc *tp = NULL;
struct ifnet *ifp = NULL;
- int s;
switch (type) {
case MOD_LOAD:
@@ -185,6 +277,8 @@
mtx_destroy(&tapmtx);
return (ENOMEM);
}
+ if_clone_attach(&tap_cloner);
+ if_clone_attach(&vmnet_cloner);
return (0);
case MOD_UNLOAD:
@@ -193,6 +287,8 @@
* guarantee that this is race-free since we have to
* release the tap mtx to deregister the clone handler.
*/
+ if_clone_detach(&tap_cloner);
+ if_clone_detach(&vmnet_cloner);
mtx_lock(&tapmtx);
SLIST_FOREACH(tp, &taphead, tap_next) {
mtx_lock(&tp->tap_mtx);
@@ -211,24 +307,10 @@
while ((tp = SLIST_FIRST(&taphead)) != NULL) {
SLIST_REMOVE_HEAD(&taphead, tap_next);
mtx_unlock(&tapmtx);
-
ifp = tp->tap_ifp;
TAPDEBUG("detaching %s\n", ifp->if_xname);
-
- /* Unlocked read. */
- KASSERT(!(tp->tap_flags & TAP_OPEN),
- ("%s flags is out of sync", ifp->if_xname));
-
- knlist_destroy(&tp->tap_rsel.si_note);
- destroy_dev(tp->tap_dev);
- s = splimp();
- ether_ifdetach(ifp);
- if_free_type(ifp, IFT_ETHER);
- splx(s);
-
- mtx_destroy(&tp->tap_mtx);
- free(tp, M_TAP);
+ tap_destroy(tp);
mtx_lock(&tapmtx);
}
mtx_unlock(&tapmtx);
@@ -254,38 +336,63 @@
static void
tapclone(void *arg, struct ucred *cred, char *name, int namelen, struct cdev **dev)
{
+ char devname[SPECNAMELEN + 1];
+ int i, unit, append_unit;
u_int extra;
- int i, unit;
- char *device_name = name;
if (*dev != NULL)
return;
- device_name = TAP;
+ /*
+ * If tap cloning is enabled, only the superuser can create
+ * an interface.
+ */
+ if (!tapdclone || suser_cred(cred, 0) != 0)
+ return;
+
+ unit = 0;
+ append_unit = 0;
extra = 0;
+
+ /* We're interested in only tap/vmnet devices. */
if (strcmp(name, TAP) == 0) {
unit = -1;
} else if (strcmp(name, VMNET) == 0) {
- device_name = VMNET;
- extra = VMNET_DEV_MASK;
unit = -1;
- } else if (dev_stdclone(name, NULL, device_name, &unit) != 1) {
- device_name = VMNET;
extra = VMNET_DEV_MASK;
- if (dev_stdclone(name, NULL, device_name, &unit) != 1)
+ } else if (dev_stdclone(name, NULL, TAP, &unit) != 1) {
+ if (dev_stdclone(name, NULL, VMNET, &unit) != 1) {
return;
+ } else {
+ extra = VMNET_DEV_MASK;
+ }
}
+ if (unit == -1)
+ append_unit = 1;
+
/* find any existing device, or allocate new unit number */
i = clone_create(&tapclones, &tap_cdevsw, &unit, dev, extra);
if (i) {
+ if (append_unit) {
+ /*
+ * We were passed 'tun' or 'tap', with no unit specified
+ * so we'll need to append it now.
+ */
+ namelen = snprintf(devname, sizeof(devname), "%s%d", name,
+ unit);
+ name = devname;
+ }
+
*dev = make_dev(&tap_cdevsw, unit2minor(unit | extra),
- UID_ROOT, GID_WHEEL, 0600, "%s%d", device_name, unit);
+ UID_ROOT, GID_WHEEL, 0600, "%s", name);
if (*dev != NULL) {
dev_ref(*dev);
(*dev)->si_flags |= SI_CHEAPCLONE;
}
}
+
+ if_clone_create(name, namelen, NULL);
} /* tapclone */
@@ -295,7 +402,7 @@
* to create interface
*/
static void
-tapcreate(struct cdev *dev)
+tapcreate(struct cdev *dev, int vmnet)
{
struct ifnet *ifp = NULL;
struct tap_softc *tp = NULL;
@@ -316,7 +423,7 @@
unit = dev2unit(dev);
/* select device: tap or vmnet */
- if (unit & VMNET_DEV_MASK) {
+ if (vmnet) {
name = VMNET;
tp->tap_flags |= TAP_VMNET;
} else
@@ -381,16 +488,7 @@
if ((dev2unit(dev) & CLONE_UNITMASK) > TAPMAXUNIT)
return (ENXIO);
- /*
- * XXXRW: Non-atomic test-and-set of si_drv1. Currently protected
- * by Giant, but the race actually exists under memory pressure as
- * well even when running with Giant, as malloc() may sleep.
- */
tp = dev->si_drv1;
- if (tp == NULL) {
- tapcreate(dev);
- tp = dev->si_drv1;
- }
mtx_lock(&tp->tap_mtx);
if (tp->tap_flags & TAP_OPEN) {
--- sys/net/if_tun.c Thu Oct 26 13:03:10 2006
+++ sys/net/if_tun.c Fri Nov 3 17:00:52 2006
@@ -13,7 +13,7 @@
* UCL. This driver is based much more on read/write/poll mode of
* operation though.
*
- * $FreeBSD: src/sys/net/if_tun.c,v 1.159 2006/10/22 11:52:15 rwatson Exp $
+ * $FreeBSD: src/sys/net/if_tun.c,v 1.158 2006/08/08 19:22:25 rwatson Exp $
*/
#include "opt_atalk.h"
@@ -25,6 +25,7 @@
#include <sys/param.h>
#include <sys/proc.h>
#include <sys/systm.h>
+#include <sys/mac.h>
#include <sys/mbuf.h>
#include <sys/module.h>
#include <sys/socket.h>
@@ -44,6 +45,7 @@
#include <sys/random.h>
#include <net/if.h>
+#include <net/if_clone.h>
#include <net/if_types.h>
#include <net/netisr.h>
#include <net/route.h>
@@ -55,8 +57,6 @@
#include <sys/queue.h>
-#include <security/mac/mac_framework.h>
-
/*
* tun_list is protected by global tunmtx. Other mutable fields are
* protected by tun->tun_mtx, or by their owning subsystem. tun_dev is
@@ -104,13 +104,20 @@
static struct mtx tunmtx;
static MALLOC_DEFINE(M_TUN, TUNNAME, "Tunnel Interface");
static int tundebug = 0;
+static int tundclone = 0;
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, "");
+SYSCTL_DECL(_net_link);
+SYSCTL_NODE(_net_link, OID_AUTO, tun, CTLFLAG_RW, 0,
+ "IP tunnel software network interface.");
+SYSCTL_INT(_net_link_tun, OID_AUTO, devfs_cloning, CTLFLAG_RW, &tundclone, 0,
+ "Enable legacy devfs interface creation.");
+
static void tunclone(void *arg, struct ucred *cred, char *name,
int namelen, struct cdev **dev);
-static void tuncreate(struct cdev *dev);
+static void tuncreate(const char *name, struct cdev *dev);
static int tunifioctl(struct ifnet *, u_long, caddr_t);
static int tuninit(struct ifnet *);
static int tunmodevent(module_t, int, void *);
@@ -118,6 +125,11 @@
struct rtentry *rt);
static void tunstart(struct ifnet *);
+static int tun_clone_create(struct if_clone *, int, caddr_t);
+static void tun_clone_destroy(struct ifnet *);
+
+IFC_SIMPLE_DECLARE(tun, 0);
+
static d_open_t tunopen;
static d_close_t tunclose;
static d_read_t tunread;
@@ -157,15 +169,45 @@
.d_name = TUNNAME,
};
+static int
+tun_clone_create(struct if_clone *ifc, int unit, caddr_t params)
+{
+ struct cdev *dev;
+ int i;
+
+ /* find any existing device, or allocate new unit number */
+ i = clone_create(&tunclones, &tun_cdevsw, &unit, &dev, 0);
+ if (i) {
+ /* No preexisting struct cdev *, create one */
+ dev = make_dev(&tun_cdevsw, unit2minor(unit),
+ UID_UUCP, GID_DIALER, 0600, "%s%d", ifc->ifc_name, unit);
+ if (dev != NULL) {
+ dev_ref(dev);
+ dev->si_flags |= SI_CHEAPCLONE;
+ }
+ }
+ tuncreate(ifc->ifc_name, dev);
+
+ return (0);
+}
+
static void
tunclone(void *arg, struct ucred *cred, char *name, int namelen,
struct cdev **dev)
{
- int u, i;
+ char devname[SPECNAMELEN + 1];
+ int u, i, append_unit;
if (*dev != NULL)
return;
+ /*
+ * If tun cloning is enabled, only the superuser can create an
+ * interface.
+ */
+ if (!tundclone || suser_cred(cred, 0) != 0)
+ return;
+
if (strcmp(name, TUNNAME) == 0) {
u = -1;
} else if (dev_stdclone(name, NULL, TUNNAME, &u) != 1)
@@ -173,17 +215,29 @@
if (u != -1 && u > IF_MAXUNIT)
return; /* Unit number too high */
+ if (u == -1)
+ append_unit = 1;
+ else
+ append_unit = 0;
+
/* find any existing device, or allocate new unit number */
i = clone_create(&tunclones, &tun_cdevsw, &u, dev, 0);
if (i) {
+ if (append_unit) {
+ namelen = snprintf(devname, sizeof(devname), "%s%d", name,
+ u);
+ name = devname;
+ }
/* No preexisting struct cdev *, create one */
*dev = make_dev(&tun_cdevsw, unit2minor(u),
- UID_UUCP, GID_DIALER, 0600, "tun%d", u);
+ UID_UUCP, GID_DIALER, 0600, "%s", name);
if (*dev != NULL) {
dev_ref(*dev);
(*dev)->si_flags |= SI_CHEAPCLONE;
}
}
+
+ if_clone_create(name, namelen, NULL);
}
static void
@@ -205,6 +259,17 @@
free(tp, M_TUN);
}
+static void
+tun_clone_destroy(struct ifnet *ifp)
+{
+ struct tun_softc *tp = ifp->if_softc;
+
+ mtx_lock(&tunmtx);
+ TAILQ_REMOVE(&tunhead, tp, tun_list);
+ mtx_unlock(&tunmtx);
+ tun_destroy(tp);
+}
+
static int
tunmodevent(module_t mod, int type, void *data)
{
@@ -218,8 +283,10 @@
tag = EVENTHANDLER_REGISTER(dev_clone, tunclone, 0, 1000);
if (tag == NULL)
return (ENOMEM);
+ if_clone_attach(&tun_cloner);
break;
case MOD_UNLOAD:
+ if_clone_detach(&tun_cloner);
EVENTHANDLER_DEREGISTER(dev_clone, tag);
mtx_lock(&tunmtx);
@@ -280,7 +347,7 @@
/* XXX: should return an error code so it can fail. */
static void
-tuncreate(struct cdev *dev)
+tuncreate(const char *name, struct cdev *dev)
{
struct tun_softc *sc;
struct ifnet *ifp;
@@ -299,7 +366,7 @@
if (ifp == NULL)
panic("%s%d: failed to if_alloc() interface.\n",
TUNNAME, dev2unit(dev));
- if_initname(ifp, TUNNAME, dev2unit(dev));
+ if_initname(ifp, name, dev2unit(dev));
ifp->if_mtu = TUNMTU;
ifp->if_ioctl = tunifioctl;
ifp->if_output = tunoutput;
@@ -330,7 +397,7 @@
*/
tp = dev->si_drv1;
if (!tp) {
- tuncreate(dev);
+ tuncreate(TUNNAME, dev);
tp = dev->si_drv1;
}
--- patch-tuntap_ifclone ends here ---
--- patch-kern_conf.c begins here ---
--- sys/kern/kern_conf.c Fri Oct 27 08:34:47 2006
+++ sys/kern/kern_conf.c Fri Nov 3 03:27:06 2006
@@ -847,10 +847,10 @@
low++;
de = dev;
continue;
- } else if (u < (unit | extra)) {
+ } else if (u < (int) (unit | extra)) {
de = dev;
continue;
- } else if (u > (unit | extra)) {
+ } else if (u > (int) (unit | extra)) {
dl = dev;
break;
}
--- patch-kern_conf.c ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
More information about the freebsd-bugs
mailing list