I'd like to protect bridge(4) data structures with a mutex. Using a
mutex is necessary because bridge_output() is called from interrupt
handlers in wireless drivers. This forces us to decouple data gathering
from copying to userland when executing some ioctl(2)s. Diff below does
that by moving the copyout(9)s outside of the loops.
Ok?
Index: net/bridgectl.c
===================================================================
RCS file: /cvs/src/sys/net/bridgectl.c,v
retrieving revision 1.11
diff -u -p -r1.11 bridgectl.c
--- net/bridgectl.c 26 Oct 2018 14:55:27 -0000 1.11
+++ net/bridgectl.c 8 Nov 2018 18:17:37 -0000
@@ -470,40 +470,48 @@ bridge_rtdelete(struct bridge_softc *sc,
int
bridge_rtfind(struct bridge_softc *sc, struct ifbaconf *baconf)
{
- int i, error = 0, onlycnt = 0;
- u_int32_t cnt = 0;
+ struct ifbareq *bareq, *bareqs = NULL;
struct bridge_rtnode *n;
- struct ifbareq bareq;
+ u_int32_t i = 0, total = 0;
+ int k, error = 0;
- if (baconf->ifbac_len == 0)
- onlycnt = 1;
+ for (k = 0; k < BRIDGE_RTABLE_SIZE; k++) {
+ LIST_FOREACH(n, &sc->sc_rts[k], brt_next)
+ total++;
+ }
- for (i = 0, cnt = 0; i < BRIDGE_RTABLE_SIZE; i++) {
- LIST_FOREACH(n, &sc->sc_rts[i], brt_next) {
- if (!onlycnt) {
- if (baconf->ifbac_len < sizeof(struct ifbareq))
- goto done;
- bcopy(sc->sc_if.if_xname, bareq.ifba_name,
- sizeof(bareq.ifba_name));
- bcopy(n->brt_if->if_xname, bareq.ifba_ifsname,
- sizeof(bareq.ifba_ifsname));
- bcopy(&n->brt_addr, &bareq.ifba_dst,
- sizeof(bareq.ifba_dst));
- bridge_copyaddr(&n->brt_tunnel.brtag_peer.sa,
- sstosa(&bareq.ifba_dstsa));
- bareq.ifba_age = n->brt_age;
- bareq.ifba_flags = n->brt_flags;
- error = copyout((caddr_t)&bareq,
- (caddr_t)(baconf->ifbac_req + cnt),
sizeof(bareq));
- if (error)
- goto done;
- baconf->ifbac_len -= sizeof(struct ifbareq);
- }
- cnt++;
+ if (baconf->ifbac_len == 0) {
+ i = total;
+ goto done;
+ }
+
+ bareqs = mallocarray(total, sizeof(*bareqs), M_TEMP, M_WAITOK|M_ZERO);
+ if (bareqs == NULL)
+ goto done;
+
+ for (k = 0; k < BRIDGE_RTABLE_SIZE; k++) {
+ LIST_FOREACH(n, &sc->sc_rts[k], brt_next) {
+ if (baconf->ifbac_len < (i + 1) * sizeof(*bareqs))
+ goto done;
+ bareq = &bareqs[i];
+ bcopy(sc->sc_if.if_xname, bareq->ifba_name,
+ sizeof(bareq->ifba_name));
+ bcopy(n->brt_if->if_xname, bareq->ifba_ifsname,
+ sizeof(bareq->ifba_ifsname));
+ bcopy(&n->brt_addr, &bareq->ifba_dst,
+ sizeof(bareq->ifba_dst));
+ bridge_copyaddr(&n->brt_tunnel.brtag_peer.sa,
+ sstosa(&bareq->ifba_dstsa));
+ bareq->ifba_age = n->brt_age;
+ bareq->ifba_flags = n->brt_flags;
+ i++;
}
}
+
+ error = copyout(bareqs, baconf->ifbac_req, i * sizeof(*bareqs));
done:
- baconf->ifbac_len = cnt * sizeof(struct ifbareq);
+ free(bareqs, M_TEMP, total * sizeof(*bareqs));
+ baconf->ifbac_len = i * sizeof(struct ifbareq);
return (error);
}
@@ -550,7 +558,7 @@ bridge_brlconf(struct bridge_iflist *bif
{
struct bridge_softc *sc = bif->bridge_sc;
struct brl_node *n;
- struct ifbrlreq req;
+ struct ifbrlreq *req, *reqs = NULL;
int error = 0;
u_int32_t i = 0, total = 0;
@@ -566,56 +574,52 @@ bridge_brlconf(struct bridge_iflist *bif
goto done;
}
+ reqs = mallocarray(total, sizeof(*reqs), M_TEMP, M_WAITOK|M_ZERO);
+ if (reqs == NULL)
+ goto done;
+
SIMPLEQ_FOREACH(n, &bif->bif_brlin, brl_next) {
- bzero(&req, sizeof req);
- if (bc->ifbrl_len < sizeof(req))
+ if (bc->ifbrl_len < (i + 1) * sizeof(*reqs))
goto done;
- strlcpy(req.ifbr_name, sc->sc_if.if_xname, IFNAMSIZ);
- strlcpy(req.ifbr_ifsname, bif->ifp->if_xname, IFNAMSIZ);
- req.ifbr_action = n->brl_action;
- req.ifbr_flags = n->brl_flags;
- req.ifbr_src = n->brl_src;
- req.ifbr_dst = n->brl_dst;
- req.ifbr_arpf = n->brl_arpf;
+ req = &reqs[i];
+ strlcpy(req->ifbr_name, sc->sc_if.if_xname, IFNAMSIZ);
+ strlcpy(req->ifbr_ifsname, bif->ifp->if_xname, IFNAMSIZ);
+ req->ifbr_action = n->brl_action;
+ req->ifbr_flags = n->brl_flags;
+ req->ifbr_src = n->brl_src;
+ req->ifbr_dst = n->brl_dst;
+ req->ifbr_arpf = n->brl_arpf;
#if NPF > 0
- req.ifbr_tagname[0] = '\0';
+ req->ifbr_tagname[0] = '\0';
if (n->brl_tag)
- pf_tag2tagname(n->brl_tag, req.ifbr_tagname);
+ pf_tag2tagname(n->brl_tag, req->ifbr_tagname);
#endif
- error = copyout((caddr_t)&req,
- (caddr_t)(bc->ifbrl_buf + (i * sizeof(req))), sizeof(req));
- if (error)
- goto done;
i++;
- bc->ifbrl_len -= sizeof(req);
}
SIMPLEQ_FOREACH(n, &bif->bif_brlout, brl_next) {
- bzero(&req, sizeof req);
- if (bc->ifbrl_len < sizeof(req))
+ if (bc->ifbrl_len < (i + 1) * sizeof(*reqs))
goto done;
- strlcpy(req.ifbr_name, sc->sc_if.if_xname, IFNAMSIZ);
- strlcpy(req.ifbr_ifsname, bif->ifp->if_xname, IFNAMSIZ);
- req.ifbr_action = n->brl_action;
- req.ifbr_flags = n->brl_flags;
- req.ifbr_src = n->brl_src;
- req.ifbr_dst = n->brl_dst;
- req.ifbr_arpf = n->brl_arpf;
+ req = &reqs[i];
+ strlcpy(req->ifbr_name, sc->sc_if.if_xname, IFNAMSIZ);
+ strlcpy(req->ifbr_ifsname, bif->ifp->if_xname, IFNAMSIZ);
+ req->ifbr_action = n->brl_action;
+ req->ifbr_flags = n->brl_flags;
+ req->ifbr_src = n->brl_src;
+ req->ifbr_dst = n->brl_dst;
+ req->ifbr_arpf = n->brl_arpf;
#if NPF > 0
- req.ifbr_tagname[0] = '\0';
+ req->ifbr_tagname[0] = '\0';
if (n->brl_tag)
- pf_tag2tagname(n->brl_tag, req.ifbr_tagname);
+ pf_tag2tagname(n->brl_tag, req->ifbr_tagname);
#endif
- error = copyout((caddr_t)&req,
- (caddr_t)(bc->ifbrl_buf + (i * sizeof(req))), sizeof(req));
- if (error)
- goto done;
i++;
- bc->ifbrl_len -= sizeof(req);
}
+ error = copyout(reqs, bc->ifbrl_buf, i * sizeof(*reqs));
done:
- bc->ifbrl_len = i * sizeof(req);
+ free(reqs, M_TEMP, total * sizeof(*reqs));
+ bc->ifbrl_len = i * sizeof(*reqs);
return (error);
}
Index: net/if_bridge.c
===================================================================
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.312
diff -u -p -r1.312 if_bridge.c
--- net/if_bridge.c 1 Oct 2018 12:38:32 -0000 1.312
+++ net/if_bridge.c 8 Nov 2018 18:16:44 -0000
@@ -594,7 +594,7 @@ bridge_bifconf(struct bridge_softc *sc,
struct bstp_state *bs = sc->sc_stp;
u_int32_t total = 0, i = 0;
int error = 0;
- struct ifbreq *breq = NULL;
+ struct ifbreq *breq, *breqs = NULL;
TAILQ_FOREACH(bif, &sc->sc_iflist, next)
total++;
@@ -605,16 +605,17 @@ bridge_bifconf(struct bridge_softc *sc,
if (bifc->ifbic_len == 0) {
i = total;
goto done;
+
}
- if ((breq = (struct ifbreq *)
- malloc(sizeof(*breq), M_DEVBUF, M_NOWAIT)) == NULL)
+ breqs = mallocarray(total, sizeof(*breqs), M_TEMP, M_WAITOK|M_ZERO);
+ if (breqs == NULL)
goto done;
TAILQ_FOREACH(bif, &sc->sc_iflist, next) {
- bzero(breq, sizeof(*breq));
- if (bifc->ifbic_len < sizeof(*breq))
+ if (bifc->ifbic_len < (i + 1) * sizeof(*breqs))
break;
+ breq = &breqs[i];
strlcpy(breq->ifbr_name, sc->sc_if.if_xname, IFNAMSIZ);
strlcpy(breq->ifbr_ifsname, bif->ifp->if_xname, IFNAMSIZ);
breq->ifbr_ifsflags = bif->bif_flags;
@@ -645,32 +646,22 @@ bridge_bifconf(struct bridge_softc *sc,
if (bp->bp_flags & BSTP_PORT_AUTOPTP)
breq->ifbr_ifsflags |= IFBIF_BSTP_AUTOPTP;
}
- error = copyout((caddr_t)breq,
- (caddr_t)(bifc->ifbic_req + i), sizeof(*breq));
- if (error)
- goto done;
i++;
- bifc->ifbic_len -= sizeof(*breq);
}
TAILQ_FOREACH(bif, &sc->sc_spanlist, next) {
- bzero(breq, sizeof(*breq));
- if (bifc->ifbic_len < sizeof(*breq))
+ if (bifc->ifbic_len < (i + 1) * sizeof(*breqs))
break;
+ breq = &breqs[i];
strlcpy(breq->ifbr_name, sc->sc_if.if_xname, IFNAMSIZ);
strlcpy(breq->ifbr_ifsname, bif->ifp->if_xname, IFNAMSIZ);
breq->ifbr_ifsflags = bif->bif_flags | IFBIF_SPAN;
breq->ifbr_portno = bif->ifp->if_index & 0xfff;
- error = copyout((caddr_t)breq,
- (caddr_t)(bifc->ifbic_req + i), sizeof(*breq));
- if (error)
- goto done;
i++;
- bifc->ifbic_len -= sizeof(*breq);
}
+ error = copyout(breqs, bifc->ifbic_req, i * sizeof(*breqs));
done:
- if (breq != NULL)
- free(breq, M_DEVBUF, sizeof *breq);
+ free(breqs, M_TEMP, total * sizeof(*breq));
bifc->ifbic_len = i * sizeof(*breq);
return (error);
}
