Prevent concurrent access to incomplete or dying `sc' caused by sleep
authormvs <mvs@openbsd.org>
Thu, 30 Dec 2021 00:49:41 +0000 (00:49 +0000)
committermvs <mvs@openbsd.org>
Thu, 30 Dec 2021 00:49:41 +0000 (00:49 +0000)
points in pppacopen() and pppacclose() paths. Use the same "sc_ready"
logic we use for 'pppx_if' structure.

Reported-by: syzbot+a7ac144b48f7f471f689@syzkaller.appspotmail.com
ok anton@ dlg@

sys/net/if_pppx.c

index 7267ba6..b4f8904 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_pppx.c,v 1.111 2021/07/20 16:44:55 mvs Exp $ */
+/*     $OpenBSD: if_pppx.c,v 1.112 2021/12/30 00:49:41 mvs Exp $ */
 
 /*
  * Copyright (c) 2010 Claudio Jeker <claudio@openbsd.org>
@@ -930,6 +930,7 @@ RBT_GENERATE(pppx_ifs, pppx_if, pxi_entry, pppx_if_cmp);
 struct pppac_softc {
        struct ifnet    sc_if;
        dev_t           sc_dev;         /* [I] */
+       int             sc_ready;       /* [K] */
        LIST_ENTRY(pppac_softc)
                        sc_entry;       /* [K] */
 
@@ -985,8 +986,12 @@ pppac_lookup(dev_t dev)
        struct pppac_softc *sc;
 
        LIST_FOREACH(sc, &pppac_devs, sc_entry) {
-               if (sc->sc_dev == dev)
+               if (sc->sc_dev == dev) {
+                       if (sc->sc_ready == 0)
+                               break;
+
                        return (sc);
+               }
        }
 
        return (NULL);
@@ -1006,10 +1011,13 @@ pppacopen(dev_t dev, int flags, int mode, struct proc *p)
        struct pipex_session *session;
 
        sc = malloc(sizeof(*sc), M_DEVBUF, M_WAITOK|M_ZERO);
-       if (pppac_lookup(dev) != NULL) {
-               free(sc, M_DEVBUF, sizeof(*sc));
-               return (EBUSY);
+       LIST_FOREACH(sc, &pppac_devs, sc_entry) {
+               if (sc->sc_dev == dev) {
+                       free(sc, M_DEVBUF, sizeof(*sc));
+                       return (EBUSY);
+               }
        }
+       LIST_INSERT_HEAD(&pppac_devs, sc, sc_entry);
 
        /* virtual pipex_session entry for multicast */
        session = pool_get(&pipex_session_pool, PR_WAITOK | PR_ZERO);
@@ -1023,8 +1031,6 @@ pppacopen(dev_t dev, int flags, int mode, struct proc *p)
        mtx_init(&sc->sc_wsel_mtx, IPL_SOFTNET);
        mq_init(&sc->sc_mq, IFQ_MAXLEN, IPL_SOFTNET);
 
-       LIST_INSERT_HEAD(&pppac_devs, sc, sc_entry);
-
        ifp = &sc->sc_if;
        snprintf(ifp->if_xname, sizeof(ifp->if_xname), "pppac%u", minor(dev));
 
@@ -1049,6 +1055,8 @@ pppacopen(dev_t dev, int flags, int mode, struct proc *p)
        bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(uint32_t));
 #endif
 
+       sc->sc_ready = 1;
+
        return (0);
 }
 
@@ -1302,6 +1310,8 @@ pppacclose(dev_t dev, int flags, int mode, struct proc *p)
        struct ifnet *ifp = &sc->sc_if;
        int s;
 
+       sc->sc_ready = 0;
+
        NET_LOCK();
        CLR(ifp->if_flags, IFF_RUNNING);
        NET_UNLOCK();