From 156bbf72d5e4755c10d0eb871b7abd29d5cf1b28 Mon Sep 17 00:00:00 2001
From: dlg <dlg@openbsd.org>
Date: Wed, 16 Feb 2022 02:22:39 +0000
Subject: [PATCH] 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@
---
 sys/net/if_tun.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/sys/net/if_tun.c b/sys/net/if_tun.c
index 2932a67c42e..c54f51b0d54 100644
--- 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);
 }
 
 /*
-- 
2.20.1