if an open tun (or tap) device is destroyed via ifconfig destroy, 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.
i think this diff fixes it by having the open code refuse to work with a device that's in the process of being destroyed. this was found by syzkaller. the detail is at https://syzkaller.appspot.com/bug?id=1d1ea7860c36e5066edea1145cf2d56715d5042b. i was able to reproduce the problem with this code. it was pretty hard to hit, but ive had no luck since adding the TUN_DEAD handling to tun_dev_open. #include <sys/socket.h> #include <sys/ioctl.h> #include <sys/sockio.h> #include <net/if.h> #include <pthread.h> #include <stdlib.h> #include <stdio.h> #include <string.h> #include <unistd.h> #include <fcntl.h> #include <errno.h> #include <err.h> static void * ifioctl(void *arg) { const char *name = arg; struct ifreq ifr; size_t len; int s; len = strlen(name); if (len >= sizeof(ifr.ifr_name)) errx(1, "interface name is too long"); s = socket(AF_INET, SOCK_DGRAM, 0); if (s == -1) err(1, "%s socket", __func__); for (;;) { memset(&ifr, 0, sizeof(ifr)); memcpy(ifr.ifr_name, name, len); if (ioctl(s, SIOCIFCREATE, &ifr) == -1) warn("SIOCIFCREATE %s", name); memset(&ifr, 0, sizeof(ifr)); memcpy(ifr.ifr_name, name, len); if (ioctl(s, SIOCIFDESTROY, &ifr) == -1) warn("SIOCIFDESTROY %s", name); } return (NULL); } static void * dvthing(void *arg) { const char *name = arg; int fd; unsigned int yay = 0, nay = 0; ssize_t rv; uint8_t buf[65536]; for (;;) { fd = open(name, O_RDWR); if (fd == -1) { nay++; if (errno != EBUSY || (nay % 1000) == 0) { warn("open %s (yay %u vs nay %u)", name, yay, nay); } continue; } yay++; pain: rv = read(fd, buf, sizeof(buf)); if (rv == -1) { warn("read"); if (errno == EIO) { int nfd = open(name, O_RDWR); if (nfd == -1) warn("open new %s", name); else { close(fd); fd = nfd; goto pain; } } } close(fd); } return (NULL); } __dead static void usage(void) { extern char *__progname; fprintf(stderr, "usage: %s ifname\n", __progname); exit(1); } int main(int argc, char *argv[]) { int ecode; pthread_t if_tid, dv_tid; void *ret; if (argc != 2) usage(); if (chdir("/dev") == -1) err(1, "/dev"); ecode = pthread_create(&if_tid, NULL, ifioctl, argv[1]); if (ecode != 0) errc(1, ecode, "create ifioctl thread"); ecode = pthread_create(&dv_tid, NULL, dvthing, argv[1]); if (ecode != 0) errc(1, ecode, "create dvthing thread"); ecode = pthread_join(if_tid, &ret); if (ecode != 0) errc(1, ecode, "join ifioctl thread"); ecode = pthread_join(dv_tid, &ret); if (ecode != 0) errc(1, ecode, "join dvthing thread"); exit(0); } Index: if_tun.c =================================================================== RCS file: /cvs/src/sys/net/if_tun.c,v retrieving revision 1.233 diff -u -p -r1.233 if_tun.c --- if_tun.c 15 Feb 2022 04:19:52 -0000 1.233 +++ if_tun.c 15 Feb 2022 04:21:53 -0000 @@ -218,6 +218,8 @@ tun_create(struct if_clone *ifc, int uni 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 uni } 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_ 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_ } } - 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); } /*
