Skip to content

Commit

Permalink
prevent (re)opening of tun/tap interfaces that are being destroyed.
Browse files Browse the repository at this point in the history
if an open tun (or tap) device is destroyed via the clone destroy
ioctl (eg, like what ifconfig destroy does), there is a window while
the open device is being revoked on the vfs side that a third thread
can come and open it again. this in turn triggers a kassert in the
ifconfig destroy path where it expects the
device to be closed.

fix this by having tun_dev_open check for the TUN_DEAD flag that
the destroy function sets. this still relies on the kernel lock for
serialisation.

Reported-by: syzbot+5df2ad232f5f8b671442@syzkaller.appspotmail.com
ok visa@
  • Loading branch information
dlg committed Feb 16, 2022
1 parent 3271a54 commit 156bbf7
Showing 1 changed file with 23 additions and 11 deletions.
34 changes: 23 additions & 11 deletions sys/net/if_tun.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: if_tun.c,v 1.233 2022/02/15 04:19:52 dlg Exp $ */
/* $OpenBSD: if_tun.c,v 1.234 2022/02/16 02:22:39 dlg Exp $ */
/* $NetBSD: if_tun.c,v 1.24 1996/05/07 02:40:48 thorpej Exp $ */

/*
Expand Down Expand Up @@ -218,6 +218,8 @@ tun_create(struct if_clone *ifc, int unit, int flags)
KERNEL_ASSERT_LOCKED();

sc = malloc(sizeof(*sc), M_DEVBUF, M_WAITOK|M_ZERO);
refcnt_init(&sc->sc_refs);

ifp = &sc->sc_if;
snprintf(ifp->if_xname, sizeof(ifp->if_xname),
"%s%d", ifc->ifc_name, unit);
Expand Down Expand Up @@ -267,7 +269,6 @@ tun_create(struct if_clone *ifc, int unit, int flags)
}

sigio_init(&sc->sc_sigio);
refcnt_init(&sc->sc_refs);

/* tell tun_dev_open we're initialised */

Expand Down Expand Up @@ -381,7 +382,7 @@ tun_dev_open(dev_t dev, const struct if_clone *ifc, int mode, struct proc *p)
rdomain = rtable_l2(p->p_p->ps_rtableid);

/* let's find or make an interface to work with */
while ((ifp = if_unit(name)) == NULL) {
while ((sc = tun_name_lookup(name)) == NULL) {
error = if_clone_create(name, rdomain);
switch (error) {
case 0: /* it's probably ours */
Expand All @@ -394,34 +395,45 @@ tun_dev_open(dev_t dev, const struct if_clone *ifc, int mode, struct proc *p)
}
}

sc = ifp->if_softc;
refcnt_take(&sc->sc_refs);

/* wait for it to be fully constructed before we use it */
while (!ISSET(sc->sc_flags, TUN_INITED)) {
for (;;) {
if (ISSET(sc->sc_flags, TUN_DEAD)) {
error = ENXIO;
goto done;
}

if (ISSET(sc->sc_flags, TUN_INITED))
break;

error = tsleep_nsec(sc, PCATCH, "tuninit", INFSLP);
if (error != 0) {
/* XXX if_clone_destroy if stayup? */
if_put(ifp);
return (error);
goto done;
}
}

if (sc->sc_dev != 0) {
/* aww, we lost */
if_put(ifp);
return (EBUSY);
error = EBUSY;
goto done;
}
/* it's ours now */
sc->sc_dev = dev;
CLR(sc->sc_flags, stayup);

/* automatically mark the interface running on open */
ifp = &sc->sc_if;
NET_LOCK();
SET(ifp->if_flags, IFF_UP | IFF_RUNNING);
NET_UNLOCK();
tun_link_state(ifp, LINK_STATE_FULL_DUPLEX);
if_put(ifp);
error = 0;

return (0);
done:
tun_put(sc);
return (error);
}

/*
Expand Down

0 comments on commit 156bbf7

Please sign in to comment.