From: mvs <mvs@openbsd.org>
Date: Thu, 30 Dec 2021 00:49:41 +0000 (+0000)
Subject: Prevent concurrent access to incomplete or dying `sc' caused by sleep
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=188a0a692db52e76194ccf0ec51e41e36f582ba9;p=openbsd

Prevent concurrent access to incomplete or dying `sc' caused by sleep
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@
---

diff --git a/sys/net/if_pppx.c b/sys/net/if_pppx.c
index 7267ba657bc..b4f8904aa30 100644
--- a/sys/net/if_pppx.c
+++ b/sys/net/if_pppx.c
@@ -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();