d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Avoid holding the softc lock when using copyout().

Browse Source 
Reported by:	dfr
Approved by:	re (rwatson)
This commit is contained in:
Andrew Thompson 2007-07-26 20:30:18 +00:00
parent c4dd9fb67a
commit 82056f42cf
3 changed files with 109 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
sys/net/if_bridge.c
View File

@ -668,8 +668,6 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
const struct bridge_control *bc; const struct bridge_control *bc;
int error = 0; int error = 0;
BRIDGE_LOCK(sc);
switch (cmd) { switch (cmd) {
case SIOCADDMULTI: case SIOCADDMULTI:
@ -714,7 +712,9 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
break; break;
} }
BRIDGE_LOCK(sc);
error = (*bc->bc_func)(sc, &args); error = (*bc->bc_func)(sc, &args);
BRIDGE_UNLOCK(sc);
if (error) if (error)
break; 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, * If interface is marked down and it is running,
* then stop and disable it. * then stop and disable it.
*/ */
BRIDGE_LOCK(sc);
bridge_stop(ifp, 1); bridge_stop(ifp, 1);
BRIDGE_UNLOCK(sc);
} else if ((ifp->if_flags & IFF_UP) && } else if ((ifp->if_flags & IFF_UP) &&
!(ifp->if_drv_flags & IFF_DRV_RUNNING)) { !(ifp->if_drv_flags & IFF_DRV_RUNNING)) {
/* /*
* If interface is marked up and it is stopped, then * If interface is marked up and it is stopped, then
* start it. * start it.
*/ */
BRIDGE_UNLOCK(sc);
(*ifp->if_init)(sc); (*ifp->if_init)(sc);
} }
break; 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 * drop the lock as ether_ioctl() will call bridge_start() and
* cause the lock to be recursed. * cause the lock to be recursed.
*/ */
BRIDGE_UNLOCK(sc);
error = ether_ioctl(ifp, cmd, data); error = ether_ioctl(ifp, cmd, data);
break; break;
} }
if (BRIDGE_LOCKED(sc))
BRIDGE_UNLOCK(sc);
return (error); return (error);
} }
@ -1106,7 +1103,8 @@ bridge_ioctl_gifs(struct bridge_softc *sc, void *arg)
struct ifbifconf *bifc = arg; struct ifbifconf *bifc = arg;
struct bridge_iflist *bif; struct bridge_iflist *bif;
struct ifbreq breq; struct ifbreq breq;
int count, len, error = 0; char *buf, *outbuf;
int count, buflen, len, error = 0;
count = 0; count = 0;
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) 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) LIST_FOREACH(bif, &sc->sc_spanlist, bif_next)
count++; count++;
buflen = sizeof(breq) * count;
if (bifc->ifbic_len == 0) { if (bifc->ifbic_len == 0) {
bifc->ifbic_len = sizeof(breq) * count; bifc->ifbic_len = buflen;
return (0); return (0);
} }
BRIDGE_UNLOCK(sc);
outbuf = malloc(buflen, M_TEMP, M_WAITOK | M_ZERO);
BRIDGE_LOCK(sc);
count = 0; count = 0;
len = bifc->ifbic_len; buf = outbuf;
len = min(bifc->ifbic_len, buflen);
bzero(&breq, sizeof(breq)); bzero(&breq, sizeof(breq));
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) { LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
if (len < sizeof(breq)) if (len < sizeof(breq))
@ -1132,10 +1135,9 @@ bridge_ioctl_gifs(struct bridge_softc *sc, void *arg)
error = bridge_ioctl_gifflags(sc, &breq); error = bridge_ioctl_gifflags(sc, &breq);
if (error) if (error)
break; break;
error = copyout(&breq, bifc->ifbic_req + count, sizeof(breq)); memcpy(buf, &breq, sizeof(breq));
if (error)
break;
count++; count++;
buf += sizeof(breq);
len -= sizeof(breq); len -= sizeof(breq);
} }
LIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { 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)); sizeof(breq.ifbr_ifsname));
breq.ifbr_ifsflags = bif->bif_flags; breq.ifbr_ifsflags = bif->bif_flags;
breq.ifbr_portno = bif->bif_ifp->if_index & 0xfff; breq.ifbr_portno = bif->bif_ifp->if_index & 0xfff;
error = copyout(&breq, bifc->ifbic_req + count, sizeof(breq)); memcpy(buf, &breq, sizeof(breq));
if (error)
break;
count++; count++;
buf += sizeof(breq);
len -= sizeof(breq); len -= sizeof(breq);
} }
BRIDGE_UNLOCK(sc);
bifc->ifbic_len = sizeof(breq) * count; bifc->ifbic_len = sizeof(breq) * count;
error = copyout(outbuf, bifc->ifbic_req, bifc->ifbic_len);
BRIDGE_LOCK(sc);
free(outbuf, M_TEMP);
return (error); return (error);
} }
@ -1163,12 +1168,24 @@ bridge_ioctl_rts(struct bridge_softc *sc, void *arg)
struct ifbaconf *bac = arg; struct ifbaconf *bac = arg;
struct bridge_rtnode *brt; struct bridge_rtnode *brt;
struct ifbareq bareq; struct ifbareq bareq;
int count = 0, error = 0, len; char *buf, *outbuf;
int count, buflen, len, error = 0;
if (bac->ifbac_len == 0) if (bac->ifbac_len == 0)
return (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)); bzero(&bareq, sizeof(bareq));
LIST_FOREACH(brt, &sc->sc_rtlist, brt_list) { LIST_FOREACH(brt, &sc->sc_rtlist, brt_list) {
if (len < sizeof(bareq)) if (len < sizeof(bareq))
@ -1184,14 +1201,17 @@ bridge_ioctl_rts(struct bridge_softc *sc, void *arg)
bareq.ifba_expire = 0; bareq.ifba_expire = 0;
bareq.ifba_flags = brt->brt_flags; bareq.ifba_flags = brt->brt_flags;
error = copyout(&bareq, bac->ifbac_req + count, sizeof(bareq)); memcpy(buf, &bareq, sizeof(bareq));
if (error)
goto out;
count++; count++;
buf += sizeof(bareq);
len -= sizeof(bareq); len -= sizeof(bareq);
} }
out: out:
BRIDGE_UNLOCK(sc);
bac->ifbac_len = sizeof(bareq) * count; bac->ifbac_len = sizeof(bareq) * count;
error = copyout(outbuf, bac->ifbac_req, bac->ifbac_len);
BRIDGE_LOCK(sc);
free(outbuf, M_TEMP);
return (error); return (error);
} }
@ -1453,7 +1473,8 @@ bridge_ioctl_gifsstp(struct bridge_softc *sc, void *arg)
struct bridge_iflist *bif; struct bridge_iflist *bif;
struct bstp_port *bp; struct bstp_port *bp;
struct ifbpstpreq bpreq; struct ifbpstpreq bpreq;
int count, len, error = 0; char *buf, *outbuf;
int count, buflen, len, error = 0;
count = 0; count = 0;
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) { LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
@ -1461,13 +1482,19 @@ bridge_ioctl_gifsstp(struct bridge_softc *sc, void *arg)
count++; count++;
} }
buflen = sizeof(bpreq) * count;
if (bifstp->ifbpstp_len == 0) { if (bifstp->ifbpstp_len == 0) {
bifstp->ifbpstp_len = sizeof(bpreq) * count; bifstp->ifbpstp_len = buflen;
return (0); return (0);
} }
BRIDGE_UNLOCK(sc);
outbuf = malloc(buflen, M_TEMP, M_WAITOK | M_ZERO);
BRIDGE_LOCK(sc);
count = 0; count = 0;
len = bifstp->ifbpstp_len; buf = outbuf;
len = min(bifstp->ifbpstp_len, buflen);
bzero(&bpreq, sizeof(bpreq)); bzero(&bpreq, sizeof(bpreq));
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) { LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
if (len < sizeof(bpreq)) 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_bridge = bp->bp_desg_pv.pv_dbridge_id;
bpreq.ifbp_design_root = bp->bp_desg_pv.pv_root_id; bpreq.ifbp_design_root = bp->bp_desg_pv.pv_root_id;
error = copyout(&bpreq, bifstp->ifbpstp_req + count, memcpy(buf, &bpreq, sizeof(bpreq));
sizeof(bpreq));
if (error != 0)
break;
count++; count++;
buf += sizeof(bpreq);
len -= sizeof(bpreq); len -= sizeof(bpreq);
} }
BRIDGE_UNLOCK(sc);
bifstp->ifbpstp_len = sizeof(bpreq) * count; bifstp->ifbpstp_len = sizeof(bpreq) * count;
error = copyout(outbuf, bifstp->ifbpstp_req, bifstp->ifbpstp_len);
BRIDGE_LOCK(sc);
free(outbuf, M_TEMP);
return (error); return (error);
} }

1
sys/net/if_bridgevar.h
View File

@ -272,7 +272,6 @@ struct ifbpstpconf {
} while (0) } while (0)
#define BRIDGE_LOCK(_sc) mtx_lock(&(_sc)->sc_mtx) #define BRIDGE_LOCK(_sc) mtx_lock(&(_sc)->sc_mtx)
#define BRIDGE_UNLOCK(_sc) mtx_unlock(&(_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_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->sc_mtx, MA_OWNED)
#define BRIDGE_LOCK2REF(_sc, _err) do { \ #define BRIDGE_LOCK2REF(_sc, _err) do { \
mtx_assert(&(_sc)->sc_mtx, MA_OWNED); \ mtx_assert(&(_sc)->sc_mtx, MA_OWNED); \

80
sys/net/if_lagg.c
View File

@ -603,15 +603,16 @@ lagg_port_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
switch (cmd) { switch (cmd) {
case SIOCGLAGGPORT: case SIOCGLAGGPORT:
LAGG_RLOCK(sc);
if (rp->rp_portname[0] == '\0' || if (rp->rp_portname[0] == '\0' ||
ifunit(rp->rp_portname) != ifp) { ifunit(rp->rp_portname) != ifp) {
error = EINVAL; error = EINVAL;
break; break;
} }
if (lp->lp_softc != sc) { LAGG_RLOCK(sc);
if ((lp = ifp->if_lagg) == NULL || lp->lp_softc != sc) {
error = ENOENT; error = ENOENT;
LAGG_RUNLOCK(sc);
break; break;
} }
@ -760,30 +761,45 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
struct lagg_port *lp; struct lagg_port *lp;
struct ifnet *tpif; struct ifnet *tpif;
struct thread *td = curthread; struct thread *td = curthread;
int i, error = 0, unlock = 1; char *buf, *outbuf;
int count, buflen, len, error = 0;
LAGG_WLOCK(sc);
bzero(&rpbuf, sizeof(rpbuf)); bzero(&rpbuf, sizeof(rpbuf));
switch (cmd) { switch (cmd) {
case SIOCGLAGG: 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_proto = sc->sc_proto;
ra->ra_ports = i = 0;
if (sc->sc_req != NULL) if (sc->sc_req != NULL)
(*sc->sc_req)(sc, (caddr_t)&ra->ra_psc); (*sc->sc_req)(sc, (caddr_t)&ra->ra_psc);
lp = SLIST_FIRST(&sc->sc_ports);
while (lp && ra->ra_size >= count = 0;
i + sizeof(struct lagg_reqport)) { buf = outbuf;
lagg_port2req(lp, &rpbuf); len = min(ra->ra_size, buflen);
error = copyout(&rpbuf, (caddr_t)ra->ra_port + i, SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) {
sizeof(struct lagg_reqport)); if (len < sizeof(rpbuf))
if (error)
break; break;
i += sizeof(struct lagg_reqport);
ra->ra_ports++; lagg_port2req(lp, &rpbuf);
lp = SLIST_NEXT(lp, lp_entries); 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; break;
case SIOCSLAGG: case SIOCSLAGG:
error = priv_check(td, PRIV_NET_LAGG); error = priv_check(td, PRIV_NET_LAGG);
@ -794,6 +810,7 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
break; break;
} }
if (sc->sc_proto != LAGG_PROTO_NONE) { if (sc->sc_proto != LAGG_PROTO_NONE) {
LAGG_WLOCK(sc);
error = sc->sc_detach(sc); error = sc->sc_detach(sc);
/* Reset protocol and pointers */ /* Reset protocol and pointers */
sc->sc_proto = LAGG_PROTO_NONE; 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_lladdr = NULL;
sc->sc_req = NULL; sc->sc_req = NULL;
sc->sc_portreq = NULL; sc->sc_portreq = NULL;
LAGG_WUNLOCK(sc);
} }
if (error != 0) if (error != 0)
break; break;
for (i = 0; i < (sizeof(lagg_protos) / for (int i = 0; i < (sizeof(lagg_protos) /
sizeof(lagg_protos[0])); i++) { sizeof(lagg_protos[0])); i++) {
if (lagg_protos[i].ti_proto == ra->ra_proto) { if (lagg_protos[i].ti_proto == ra->ra_proto) {
if (sc->sc_ifflags & IFF_DEBUG) if (sc->sc_ifflags & IFF_DEBUG)
printf("%s: using proto %u\n", printf("%s: using proto %u\n",
sc->sc_ifname, sc->sc_ifname,
lagg_protos[i].ti_proto); lagg_protos[i].ti_proto);
LAGG_WLOCK(sc);
sc->sc_proto = lagg_protos[i].ti_proto; sc->sc_proto = lagg_protos[i].ti_proto;
if (sc->sc_proto != LAGG_PROTO_NONE) if (sc->sc_proto != LAGG_PROTO_NONE)
error = lagg_protos[i].ti_attach(sc); error = lagg_protos[i].ti_attach(sc);
goto out; LAGG_WUNLOCK(sc);
return (error);
} }
} }
error = EPROTONOSUPPORT; error = EPROTONOSUPPORT;
@ -833,13 +853,16 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
break; break;
} }
LAGG_RLOCK(sc);
if ((lp = (struct lagg_port *)tpif->if_lagg) == NULL || if ((lp = (struct lagg_port *)tpif->if_lagg) == NULL ||
lp->lp_softc != sc) { lp->lp_softc != sc) {
error = ENOENT; error = ENOENT;
LAGG_RUNLOCK(sc);
break; break;
} }
lagg_port2req(lp, rp); lagg_port2req(lp, rp);
LAGG_RUNLOCK(sc);
break; break;
case SIOCSLAGGPORT: case SIOCSLAGGPORT:
error = priv_check(td, PRIV_NET_LAGG); error = priv_check(td, PRIV_NET_LAGG);
@ -850,7 +873,9 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
error = EINVAL; error = EINVAL;
break; break;
} }
LAGG_WLOCK(sc);
error = lagg_port_create(sc, tpif); error = lagg_port_create(sc, tpif);
LAGG_WUNLOCK(sc);
break; break;
case SIOCSLAGGDELPORT: case SIOCSLAGGDELPORT:
error = priv_check(td, PRIV_NET_LAGG); error = priv_check(td, PRIV_NET_LAGG);
@ -862,19 +887,24 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
break; break;
} }
LAGG_WLOCK(sc);
if ((lp = (struct lagg_port *)tpif->if_lagg) == NULL || if ((lp = (struct lagg_port *)tpif->if_lagg) == NULL ||
lp->lp_softc != sc) { lp->lp_softc != sc) {
error = ENOENT; error = ENOENT;
LAGG_WUNLOCK(sc);
break; break;
} }
error = lagg_port_destroy(lp, 1); error = lagg_port_destroy(lp, 1);
LAGG_WUNLOCK(sc);
break; break;
case SIOCSIFFLAGS: case SIOCSIFFLAGS:
/* Set flags on ports too */ /* Set flags on ports too */
LAGG_WLOCK(sc);
SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) { SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) {
lagg_setflags(lp, 1); lagg_setflags(lp, 1);
} }
LAGG_WUNLOCK(sc);
if (!(ifp->if_flags & IFF_UP) && if (!(ifp->if_flags & IFF_UP) &&
(ifp->if_drv_flags & IFF_DRV_RUNNING)) { (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, * If interface is marked down and it is running,
* then stop and disable it. * then stop and disable it.
*/ */
LAGG_WLOCK(sc);
lagg_stop(sc); lagg_stop(sc);
LAGG_WUNLOCK(sc);
} else if ((ifp->if_flags & IFF_UP) && } else if ((ifp->if_flags & IFF_UP) &&
!(ifp->if_drv_flags & IFF_DRV_RUNNING)) { !(ifp->if_drv_flags & IFF_DRV_RUNNING)) {
/* /*
* If interface is marked up and it is stopped, then * If interface is marked up and it is stopped, then
* start it. * start it.
*/ */
LAGG_WUNLOCK(sc);
unlock = 0;
(*ifp->if_init)(sc); (*ifp->if_init)(sc);
} }
break; break;
case SIOCADDMULTI: case SIOCADDMULTI:
case SIOCDELMULTI: case SIOCDELMULTI:
LAGG_WLOCK(sc);
error = lagg_ether_setmulti(sc); error = lagg_ether_setmulti(sc);
LAGG_WUNLOCK(sc);
break; break;
case SIOCSIFMEDIA: case SIOCSIFMEDIA:
case SIOCGIFMEDIA: case SIOCGIFMEDIA:
LAGG_WUNLOCK(sc);
unlock = 0;
error = ifmedia_ioctl(ifp, ifr, &sc->sc_media, cmd); error = ifmedia_ioctl(ifp, ifr, &sc->sc_media, cmd);
break; break;
default: default:
LAGG_WUNLOCK(sc);
unlock = 0;
error = ether_ioctl(ifp, cmd, data); error = ether_ioctl(ifp, cmd, data);
break; break;
} }
out:
if (unlock)
LAGG_WUNLOCK(sc);
return (error); return (error);
} }