diff options
author | thompsa <thompsa@FreeBSD.org> | 2007-07-26 20:30:18 +0000 |
---|---|---|
committer | thompsa <thompsa@FreeBSD.org> | 2007-07-26 20:30:18 +0000 |
commit | d74f9301afcad074a999ec6690594c861309632d (patch) | |
tree | 8e4e42ec5c9d3f7af3729e2e4418f7468d24afa9 /sys/net | |
parent | 5ee74f35890568545cf755b8eb8180b7e3e3f6f9 (diff) | |
download | FreeBSD-src-d74f9301afcad074a999ec6690594c861309632d.zip FreeBSD-src-d74f9301afcad074a999ec6690594c861309632d.tar.gz |
Avoid holding the softc lock when using copyout().
Reported by: dfr
Approved by: re (rwatson)
Diffstat (limited to 'sys/net')
-rw-r--r-- | sys/net/if_bridge.c | 86 | ||||
-rw-r--r-- | sys/net/if_bridgevar.h | 1 | ||||
-rw-r--r-- | sys/net/if_lagg.c | 80 |
3 files changed, 109 insertions, 58 deletions
diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c index c5bc0b5..3f0cabf 100644 --- a/sys/net/if_bridge.c +++ b/sys/net/if_bridge.c @@ -668,8 +668,6 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) const struct bridge_control *bc; int error = 0; - BRIDGE_LOCK(sc); - switch (cmd) { case SIOCADDMULTI: @@ -714,7 +712,9 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) break; } + BRIDGE_LOCK(sc); error = (*bc->bc_func)(sc, &args); + BRIDGE_UNLOCK(sc); if (error) break; @@ -730,14 +730,15 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) * If interface is marked down and it is running, * then stop and disable it. */ + BRIDGE_LOCK(sc); bridge_stop(ifp, 1); + BRIDGE_UNLOCK(sc); } else if ((ifp->if_flags & IFF_UP) && !(ifp->if_drv_flags & IFF_DRV_RUNNING)) { /* * If interface is marked up and it is stopped, then * start it. */ - BRIDGE_UNLOCK(sc); (*ifp->if_init)(sc); } break; @@ -752,14 +753,10 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) * drop the lock as ether_ioctl() will call bridge_start() and * cause the lock to be recursed. */ - BRIDGE_UNLOCK(sc); error = ether_ioctl(ifp, cmd, data); break; } - if (BRIDGE_LOCKED(sc)) - BRIDGE_UNLOCK(sc); - return (error); } @@ -1106,7 +1103,8 @@ bridge_ioctl_gifs(struct bridge_softc *sc, void *arg) struct ifbifconf *bifc = arg; struct bridge_iflist *bif; struct ifbreq breq; - int count, len, error = 0; + char *buf, *outbuf; + int count, buflen, len, error = 0; count = 0; LIST_FOREACH(bif, &sc->sc_iflist, bif_next) @@ -1114,13 +1112,18 @@ bridge_ioctl_gifs(struct bridge_softc *sc, void *arg) LIST_FOREACH(bif, &sc->sc_spanlist, bif_next) count++; + buflen = sizeof(breq) * count; if (bifc->ifbic_len == 0) { - bifc->ifbic_len = sizeof(breq) * count; + bifc->ifbic_len = buflen; return (0); } + BRIDGE_UNLOCK(sc); + outbuf = malloc(buflen, M_TEMP, M_WAITOK | M_ZERO); + BRIDGE_LOCK(sc); count = 0; - len = bifc->ifbic_len; + buf = outbuf; + len = min(bifc->ifbic_len, buflen); bzero(&breq, sizeof(breq)); LIST_FOREACH(bif, &sc->sc_iflist, bif_next) { if (len < sizeof(breq)) @@ -1132,10 +1135,9 @@ bridge_ioctl_gifs(struct bridge_softc *sc, void *arg) error = bridge_ioctl_gifflags(sc, &breq); if (error) break; - error = copyout(&breq, bifc->ifbic_req + count, sizeof(breq)); - if (error) - break; + memcpy(buf, &breq, sizeof(breq)); count++; + buf += sizeof(breq); len -= sizeof(breq); } LIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { @@ -1146,14 +1148,17 @@ bridge_ioctl_gifs(struct bridge_softc *sc, void *arg) sizeof(breq.ifbr_ifsname)); breq.ifbr_ifsflags = bif->bif_flags; breq.ifbr_portno = bif->bif_ifp->if_index & 0xfff; - error = copyout(&breq, bifc->ifbic_req + count, sizeof(breq)); - if (error) - break; + memcpy(buf, &breq, sizeof(breq)); count++; + buf += sizeof(breq); len -= sizeof(breq); } + BRIDGE_UNLOCK(sc); bifc->ifbic_len = sizeof(breq) * count; + error = copyout(outbuf, bifc->ifbic_req, bifc->ifbic_len); + BRIDGE_LOCK(sc); + free(outbuf, M_TEMP); return (error); } @@ -1163,12 +1168,24 @@ bridge_ioctl_rts(struct bridge_softc *sc, void *arg) struct ifbaconf *bac = arg; struct bridge_rtnode *brt; struct ifbareq bareq; - int count = 0, error = 0, len; + char *buf, *outbuf; + int count, buflen, len, error = 0; if (bac->ifbac_len == 0) return (0); - len = bac->ifbac_len; + count = 0; + LIST_FOREACH(brt, &sc->sc_rtlist, brt_list) + count++; + buflen = sizeof(bareq) * count; + + BRIDGE_UNLOCK(sc); + outbuf = malloc(buflen, M_TEMP, M_WAITOK | M_ZERO); + BRIDGE_LOCK(sc); + + count = 0; + buf = outbuf; + len = min(bac->ifbac_len, buflen); bzero(&bareq, sizeof(bareq)); LIST_FOREACH(brt, &sc->sc_rtlist, brt_list) { if (len < sizeof(bareq)) @@ -1184,14 +1201,17 @@ bridge_ioctl_rts(struct bridge_softc *sc, void *arg) bareq.ifba_expire = 0; bareq.ifba_flags = brt->brt_flags; - error = copyout(&bareq, bac->ifbac_req + count, sizeof(bareq)); - if (error) - goto out; + memcpy(buf, &bareq, sizeof(bareq)); count++; + buf += sizeof(bareq); len -= sizeof(bareq); } out: + BRIDGE_UNLOCK(sc); bac->ifbac_len = sizeof(bareq) * count; + error = copyout(outbuf, bac->ifbac_req, bac->ifbac_len); + BRIDGE_LOCK(sc); + free(outbuf, M_TEMP); return (error); } @@ -1453,7 +1473,8 @@ bridge_ioctl_gifsstp(struct bridge_softc *sc, void *arg) struct bridge_iflist *bif; struct bstp_port *bp; struct ifbpstpreq bpreq; - int count, len, error = 0; + char *buf, *outbuf; + int count, buflen, len, error = 0; count = 0; LIST_FOREACH(bif, &sc->sc_iflist, bif_next) { @@ -1461,13 +1482,19 @@ bridge_ioctl_gifsstp(struct bridge_softc *sc, void *arg) count++; } + buflen = sizeof(bpreq) * count; if (bifstp->ifbpstp_len == 0) { - bifstp->ifbpstp_len = sizeof(bpreq) * count; + bifstp->ifbpstp_len = buflen; return (0); } + BRIDGE_UNLOCK(sc); + outbuf = malloc(buflen, M_TEMP, M_WAITOK | M_ZERO); + BRIDGE_LOCK(sc); + count = 0; - len = bifstp->ifbpstp_len; + buf = outbuf; + len = min(bifstp->ifbpstp_len, buflen); bzero(&bpreq, sizeof(bpreq)); LIST_FOREACH(bif, &sc->sc_iflist, bif_next) { if (len < sizeof(bpreq)) @@ -1484,16 +1511,17 @@ bridge_ioctl_gifsstp(struct bridge_softc *sc, void *arg) bpreq.ifbp_design_bridge = bp->bp_desg_pv.pv_dbridge_id; bpreq.ifbp_design_root = bp->bp_desg_pv.pv_root_id; - error = copyout(&bpreq, bifstp->ifbpstp_req + count, - sizeof(bpreq)); - if (error != 0) - break; - + memcpy(buf, &bpreq, sizeof(bpreq)); count++; + buf += sizeof(bpreq); len -= sizeof(bpreq); } + BRIDGE_UNLOCK(sc); bifstp->ifbpstp_len = sizeof(bpreq) * count; + error = copyout(outbuf, bifstp->ifbpstp_req, bifstp->ifbpstp_len); + BRIDGE_LOCK(sc); + free(outbuf, M_TEMP); return (error); } diff --git a/sys/net/if_bridgevar.h b/sys/net/if_bridgevar.h index b11c34d..8097cd9 100644 --- a/sys/net/if_bridgevar.h +++ b/sys/net/if_bridgevar.h @@ -272,7 +272,6 @@ struct ifbpstpconf { } while (0) #define BRIDGE_LOCK(_sc) mtx_lock(&(_sc)->sc_mtx) #define BRIDGE_UNLOCK(_sc) mtx_unlock(&(_sc)->sc_mtx) -#define BRIDGE_LOCKED(_sc) mtx_owned(&(_sc)->sc_mtx) #define BRIDGE_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->sc_mtx, MA_OWNED) #define BRIDGE_LOCK2REF(_sc, _err) do { \ mtx_assert(&(_sc)->sc_mtx, MA_OWNED); \ diff --git a/sys/net/if_lagg.c b/sys/net/if_lagg.c index ce8d1d7..2e489eb 100644 --- a/sys/net/if_lagg.c +++ b/sys/net/if_lagg.c @@ -603,15 +603,16 @@ lagg_port_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) switch (cmd) { case SIOCGLAGGPORT: - LAGG_RLOCK(sc); if (rp->rp_portname[0] == '\0' || ifunit(rp->rp_portname) != ifp) { error = EINVAL; break; } - if (lp->lp_softc != sc) { + LAGG_RLOCK(sc); + if ((lp = ifp->if_lagg) == NULL || lp->lp_softc != sc) { error = ENOENT; + LAGG_RUNLOCK(sc); break; } @@ -760,30 +761,45 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) struct lagg_port *lp; struct ifnet *tpif; struct thread *td = curthread; - int i, error = 0, unlock = 1; - - LAGG_WLOCK(sc); + char *buf, *outbuf; + int count, buflen, len, error = 0; bzero(&rpbuf, sizeof(rpbuf)); switch (cmd) { case SIOCGLAGG: + LAGG_RLOCK(sc); + count = 0; + SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) + count++; + buflen = count * sizeof(struct lagg_reqport); + LAGG_RUNLOCK(sc); + + outbuf = malloc(buflen, M_TEMP, M_WAITOK | M_ZERO); + + LAGG_RLOCK(sc); ra->ra_proto = sc->sc_proto; - ra->ra_ports = i = 0; if (sc->sc_req != NULL) (*sc->sc_req)(sc, (caddr_t)&ra->ra_psc); - lp = SLIST_FIRST(&sc->sc_ports); - while (lp && ra->ra_size >= - i + sizeof(struct lagg_reqport)) { - lagg_port2req(lp, &rpbuf); - error = copyout(&rpbuf, (caddr_t)ra->ra_port + i, - sizeof(struct lagg_reqport)); - if (error) + + count = 0; + buf = outbuf; + len = min(ra->ra_size, buflen); + SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) { + if (len < sizeof(rpbuf)) break; - i += sizeof(struct lagg_reqport); - ra->ra_ports++; - lp = SLIST_NEXT(lp, lp_entries); + + lagg_port2req(lp, &rpbuf); + memcpy(buf, &rpbuf, sizeof(rpbuf)); + count++; + buf += sizeof(rpbuf); + len -= sizeof(rpbuf); } + LAGG_RUNLOCK(sc); + ra->ra_ports = count; + ra->ra_size = count * sizeof(rpbuf); + error = copyout(outbuf, ra->ra_port, ra->ra_size); + free(outbuf, M_TEMP); break; case SIOCSLAGG: error = priv_check(td, PRIV_NET_LAGG); @@ -794,6 +810,7 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) break; } if (sc->sc_proto != LAGG_PROTO_NONE) { + LAGG_WLOCK(sc); error = sc->sc_detach(sc); /* Reset protocol and pointers */ sc->sc_proto = LAGG_PROTO_NONE; @@ -808,20 +825,23 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) sc->sc_lladdr = NULL; sc->sc_req = NULL; sc->sc_portreq = NULL; + LAGG_WUNLOCK(sc); } if (error != 0) break; - for (i = 0; i < (sizeof(lagg_protos) / + for (int i = 0; i < (sizeof(lagg_protos) / sizeof(lagg_protos[0])); i++) { if (lagg_protos[i].ti_proto == ra->ra_proto) { if (sc->sc_ifflags & IFF_DEBUG) printf("%s: using proto %u\n", sc->sc_ifname, lagg_protos[i].ti_proto); + LAGG_WLOCK(sc); sc->sc_proto = lagg_protos[i].ti_proto; if (sc->sc_proto != LAGG_PROTO_NONE) error = lagg_protos[i].ti_attach(sc); - goto out; + LAGG_WUNLOCK(sc); + return (error); } } error = EPROTONOSUPPORT; @@ -833,13 +853,16 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) break; } + LAGG_RLOCK(sc); if ((lp = (struct lagg_port *)tpif->if_lagg) == NULL || lp->lp_softc != sc) { error = ENOENT; + LAGG_RUNLOCK(sc); break; } lagg_port2req(lp, rp); + LAGG_RUNLOCK(sc); break; case SIOCSLAGGPORT: error = priv_check(td, PRIV_NET_LAGG); @@ -850,7 +873,9 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) error = EINVAL; break; } + LAGG_WLOCK(sc); error = lagg_port_create(sc, tpif); + LAGG_WUNLOCK(sc); break; case SIOCSLAGGDELPORT: error = priv_check(td, PRIV_NET_LAGG); @@ -862,19 +887,24 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) break; } + LAGG_WLOCK(sc); if ((lp = (struct lagg_port *)tpif->if_lagg) == NULL || lp->lp_softc != sc) { error = ENOENT; + LAGG_WUNLOCK(sc); break; } error = lagg_port_destroy(lp, 1); + LAGG_WUNLOCK(sc); break; case SIOCSIFFLAGS: /* Set flags on ports too */ + LAGG_WLOCK(sc); SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) { lagg_setflags(lp, 1); } + LAGG_WUNLOCK(sc); if (!(ifp->if_flags & IFF_UP) && (ifp->if_drv_flags & IFF_DRV_RUNNING)) { @@ -882,38 +912,32 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) * If interface is marked down and it is running, * then stop and disable it. */ + LAGG_WLOCK(sc); lagg_stop(sc); + LAGG_WUNLOCK(sc); } else if ((ifp->if_flags & IFF_UP) && !(ifp->if_drv_flags & IFF_DRV_RUNNING)) { /* * If interface is marked up and it is stopped, then * start it. */ - LAGG_WUNLOCK(sc); - unlock = 0; (*ifp->if_init)(sc); } break; case SIOCADDMULTI: case SIOCDELMULTI: + LAGG_WLOCK(sc); error = lagg_ether_setmulti(sc); + LAGG_WUNLOCK(sc); break; case SIOCSIFMEDIA: case SIOCGIFMEDIA: - LAGG_WUNLOCK(sc); - unlock = 0; error = ifmedia_ioctl(ifp, ifr, &sc->sc_media, cmd); break; default: - LAGG_WUNLOCK(sc); - unlock = 0; error = ether_ioctl(ifp, cmd, data); break; } - -out: - if (unlock) - LAGG_WUNLOCK(sc); return (error); } |