prevent (re)opening of tun/tap interfaces that are being destroyed.

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@

diff --git a/sys/net/if_tun.c b/sys/net/if_tun.c
index 2932a67..c54f51b 100644 (file)
--- a/sys/net/if_tun.c
+++ b/sys/net/if_tun.c
@@ -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 $      */
 
 /*
@@ -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);
@@ -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 */
 
@@ -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 */
@@ -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);
 }
 
 /*