Interface auto-cloning bug or feature?
Kostik Belousov
kostikbel at gmail.com
Sat Sep 20 13:27:09 UTC 2008
On Fri, Sep 19, 2008 at 03:43:21PM -0700, Maksim Yevmenkin wrote:
> [....]
>
> >> That what has caused me to look into this issue. You can find patch for
> >> security/vpnc to prevent unbounded interface cloning here:
> >>
> >> http://sobomax.sippysoft.com/~sobomax/vpnc.diff
> >>
> > Ok, the patch prevents interface cloning, but I think it doesn't solve
> > the actual problem.
> > Let's wait for Maksim :)
>
> ok, how about attached patch. i put it together *very* quickly and
> only gave it a light testing. its for tap(4), because i could compile
> it as a module and tun(4) is compiled into kernel by default, but the
> idea should identical for tun(4). should be even simpler for tun(4)
> because it does not have to deal with 2 kind of devices (i.e. tap and
> vmnet). give it a try, and see if it works. please try both cloning
> paths, i.e.
>
> 1) cat /dev/tap (/dev/vmnet) with and/or without unit number
>
> and
>
> 2) ifconfig tapX (vmnetX) create/destroy
>
> in the mean time i will prepare something similar for tun(4).
>
> thanks,
> max
> --- if_tap.c.orig 2008-09-08 17:20:57.000000000 -0700
> +++ if_tap.c 2008-09-19 15:35:02.000000000 -0700
> @@ -94,6 +94,7 @@
> static int tapifioctl(struct ifnet *, u_long, caddr_t);
> static void tapifinit(void *);
>
> +static int tap_clone_lookup(struct cdev **, u_short);
> 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);
> @@ -176,6 +177,28 @@
> DEV_MODULE(if_tap, tapmodevent, NULL);
>
> static int
> +tap_clone_lookup(struct cdev **dev, u_short extra)
> +{
> + struct tap_softc *tp;
> +
> + mtx_lock(&tapmtx);
> + SLIST_FOREACH(tp, &taphead, tap_next) {
> + mtx_lock(&tp->tap_mtx);
> + if ((tp->tap_flags & (TAP_OPEN|extra)) == extra) {
> + *dev = tp->tap_dev;
> + mtx_unlock(&tp->tap_mtx);
> + mtx_unlock(&tapmtx);
> +
> + return (1);
> + }
> + mtx_unlock(&tp->tap_mtx);
> + }
> + mtx_unlock(&tapmtx);
> +
> + return (0);
> +}
> +
> +static int
> tap_clone_create(struct if_clone *ifc, int unit, caddr_t params)
> {
> struct cdev *dev;
> @@ -353,8 +376,18 @@
>
> /* We're interested in only tap/vmnet devices. */
> if (strcmp(name, TAP) == 0) {
> + if (tap_clone_lookup(dev, 0)) {
> + dev_ref(*dev);
> + return;
What would prevent two concurrent threads from selecting the same device
there ? First thread could look up the device, unloc tapmtx and be
preempted. Then second thread is put on CPU, do the same selection.
Now you have a problem.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-current/attachments/20080920/e6cf4053/attachment.pgp
More information about the freebsd-current
mailing list