We have some races in pppac(4)
1. malloc(9) can sleep so we must check `sc' presence after malloc(9)
2. we can sleep between `sc' insertion to `sc_entry' list and
`sc_pipex_iface' initialization. Concurrent pppacioctl() can touch
this incomplete `sc'.
Index: sys/net/if_pppx.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.91
diff -u -p -r1.91 if_pppx.c
--- sys/net/if_pppx.c 6 Jul 2020 20:37:51 -0000 1.91
+++ sys/net/if_pppx.c 10 Jul 2020 11:04:53 -0000
@@ -1019,7 +1019,7 @@ RBT_GENERATE(pppx_ifs, pppx_if, pxi_entr
struct pppac_softc {
struct ifnet sc_if;
- unsigned int sc_dead;
+ unsigned int sc_ready;
dev_t sc_dev;
LIST_ENTRY(pppac_softc)
sc_entry;
@@ -1072,8 +1072,12 @@ pppac_lookup(dev_t dev)
struct pppac_softc *sc;
LIST_FOREACH(sc, &pppac_devs, sc_entry) {
- if (sc->sc_dev == dev)
- return (sc);
+ if (sc->sc_dev == dev) {
+ if (sc->sc_ready)
+ return (sc);
+ else
+ break;
+ }
}
return (NULL);
@@ -1088,22 +1092,25 @@ pppacattach(int n)
int
pppacopen(dev_t dev, int flags, int mode, struct proc *p)
{
- struct pppac_softc *sc;
+ struct pppac_softc *sc, *sc_tmp;
struct ifnet *ifp;
- sc = pppac_lookup(dev);
- if (sc != NULL)
- return (EBUSY);
-
sc = malloc(sizeof(*sc), M_DEVBUF, M_WAITOK|M_ZERO);
+
+ LIST_FOREACH(sc_tmp, &pppac_devs, sc_entry) {
+ if (sc_tmp->sc_dev == dev) {
+ free(sc, M_DEVBUF, sizeof(*sc));
+ return (EBUSY);
+ }
+ }
+
sc->sc_dev = dev;
+ LIST_INSERT_HEAD(&pppac_devs, sc, sc_entry);
mtx_init(&sc->sc_rsel_mtx, IPL_SOFTNET);
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));
@@ -1129,6 +1136,7 @@ pppacopen(dev_t dev, int flags, int mode
#endif
pipex_iface_init(&sc->sc_pipex_iface, ifp);
+ sc->sc_ready = 1;
return (0);
}
@@ -1136,12 +1144,14 @@ pppacopen(dev_t dev, int flags, int mode
int
pppacread(dev_t dev, struct uio *uio, int ioflag)
{
- struct pppac_softc *sc = pppac_lookup(dev);
+ struct pppac_softc *sc;
struct ifnet *ifp = &sc->sc_if;
struct mbuf *m0, *m;
int error = 0;
size_t len;
+ if ((sc = pppac_lookup(dev)) == NULL)
+ return (EBADF);
if (!ISSET(ifp->if_flags, IFF_RUNNING))
return (EHOSTDOWN);
@@ -1181,12 +1191,14 @@ pppacread(dev_t dev, struct uio *uio, in
int
pppacwrite(dev_t dev, struct uio *uio, int ioflag)
{
- struct pppac_softc *sc = pppac_lookup(dev);
+ struct pppac_softc *sc;
struct ifnet *ifp = &sc->sc_if;
uint32_t proto;
int error;
struct mbuf *m;
+ if ((sc = pppac_lookup(dev)) == NULL)
+ return (EBADF);
if (!ISSET(ifp->if_flags, IFF_RUNNING))
return (EHOSTDOWN);
@@ -1258,9 +1270,12 @@ pppacwrite(dev_t dev, struct uio *uio, i
int
pppacioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p)
{
- struct pppac_softc *sc = pppac_lookup(dev);
+ struct pppac_softc *sc;
int error = 0;
+ if ((sc = pppac_lookup(dev)) == NULL)
+ return (EBADF);
+
switch (cmd) {
case TUNSIFMODE: /* make npppd happy */
break;
@@ -1282,9 +1297,12 @@ pppacioctl(dev_t dev, u_long cmd, caddr_
int
pppacpoll(dev_t dev, int events, struct proc *p)
{
- struct pppac_softc *sc = pppac_lookup(dev);
+ struct pppac_softc *sc;
int revents = 0;
+ if ((sc = pppac_lookup(dev)) == NULL)
+ goto out;
+
if (events & (POLLIN | POLLRDNORM)) {
if (!mq_empty(&sc->sc_mq))
revents |= events & (POLLIN | POLLRDNORM);
@@ -1296,17 +1314,20 @@ pppacpoll(dev_t dev, int events, struct
if (events & (POLLIN | POLLRDNORM))
selrecord(p, &sc->sc_rsel);
}
-
+out:
return (revents);
}
int
pppackqfilter(dev_t dev, struct knote *kn)
{
- struct pppac_softc *sc = pppac_lookup(dev);
+ struct pppac_softc *sc;
struct mutex *mtx;
struct klist *klist;
+ if ((sc = pppac_lookup(dev)) == NULL)
+ return (EBADF);
+
switch (kn->kn_filter) {
case EVFILT_READ:
mtx = &sc->sc_rsel_mtx;
@@ -1373,12 +1394,15 @@ filt_pppac_write(struct knote *kn, long
int
pppacclose(dev_t dev, int flags, int mode, struct proc *p)
{
- struct pppac_softc *sc = pppac_lookup(dev);
+ struct pppac_softc *sc;
struct ifnet *ifp = &sc->sc_if;
int s;
+ if ((sc = pppac_lookup(dev)) == NULL)
+ return (EBADF);
+
+ sc->sc_ready = 0;
NET_LOCK();
- sc->sc_dead = 1;
CLR(ifp->if_flags, IFF_RUNNING);
NET_UNLOCK();
@@ -1404,7 +1428,7 @@ pppac_ioctl(struct ifnet *ifp, u_long cm
/* struct ifreq *ifr = (struct ifreq *)data; */
int error = 0;
- if (sc->sc_dead)
+ if (sc->sc_ready == 0)
return (ENXIO);
switch (cmd) {
