Protect bridge(4) hash table

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Protect bridge(4) hash table

Martin Pieuchot
Diff below uses a mutex to serialize accesses to the hash table.

It doesn't guarantee that pointers returned by a lookup on this table
are still valid.  This will be addressed in a later diff.

I also annotated Immutable fields ([I]) in the bridge_softc structure.

Ok?

Index: net/bridgectl.c
===================================================================
RCS file: /cvs/src/sys/net/bridgectl.c,v
retrieving revision 1.13
diff -u -p -r1.13 bridgectl.c
--- net/bridgectl.c 12 Dec 2018 14:19:15 -0000 1.13
+++ net/bridgectl.c 13 Feb 2019 13:41:17 -0000
@@ -102,7 +102,9 @@ bridgectl_ioctl(struct ifnet *ifp, u_lon
  bparam->ifbrp_csize = sc->sc_brtmax;
  break;
  case SIOCBRDGSCACHE:
+ mtx_enter(&sc->sc_mtx);
  sc->sc_brtmax = bparam->ifbrp_csize;
+ mtx_leave(&sc->sc_mtx);
  break;
  case SIOCBRDGSTO:
  if (bparam->ifbrp_ctime < 0 ||
@@ -195,6 +197,7 @@ bridge_rtupdate(struct bridge_softc *sc,
  }
 
  h = bridge_hash(sc, ea);
+ mtx_enter(&sc->sc_mtx);
  p = LIST_FIRST(&sc->sc_rts[h]);
  if (p == NULL) {
  if (sc->sc_brtcnt >= sc->sc_brtmax)
@@ -285,26 +288,31 @@ bridge_rtupdate(struct bridge_softc *sc,
 done:
  ifp = NULL;
 want:
+ mtx_leave(&sc->sc_mtx);
  return (ifp);
 }
 
 struct bridge_rtnode *
 bridge_rtlookup(struct bridge_softc *sc, struct ether_addr *ea)
 {
- struct bridge_rtnode *p;
+ struct bridge_rtnode *p = NULL;
  u_int32_t h;
  int dir;
 
  h = bridge_hash(sc, ea);
+ mtx_enter(&sc->sc_mtx);
  LIST_FOREACH(p, &sc->sc_rts[h], brt_next) {
  dir = memcmp(ea, &p->brt_addr, sizeof(p->brt_addr));
  if (dir == 0)
- return (p);
- if (dir > 0)
- goto fail;
+ break;
+ if (dir > 0) {
+ p = NULL;
+ break;
+ }
  }
-fail:
- return (NULL);
+ mtx_leave(&sc->sc_mtx);
+
+ return (p);
 }
 
 u_int32_t
@@ -324,8 +332,7 @@ bridge_rtage(void *vsc)
  struct bridge_rtnode *n, *p;
  int i;
 
- KERNEL_ASSERT_LOCKED();
-
+ mtx_enter(&sc->sc_mtx);
  for (i = 0; i < BRIDGE_RTABLE_SIZE; i++) {
  n = LIST_FIRST(&sc->sc_rts[i]);
  while (n != NULL) {
@@ -346,6 +353,7 @@ bridge_rtage(void *vsc)
  }
  }
  }
+ mtx_leave(&sc->sc_mtx);
 
  if (sc->sc_brttimeout != 0)
  timeout_add_sec(&sc->sc_brtimeout, sc->sc_brttimeout);
@@ -373,6 +381,7 @@ bridge_rtagenode(struct ifnet *ifp, int
  if (age == 0)
  bridge_rtdelete(sc, ifp, 1);
  else {
+ mtx_enter(&sc->sc_mtx);
  for (i = 0; i < BRIDGE_RTABLE_SIZE; i++) {
  LIST_FOREACH(n, &sc->sc_rts[i], brt_next) {
  /* Cap the expiry time to 'age' */
@@ -382,6 +391,7 @@ bridge_rtagenode(struct ifnet *ifp, int
  n->brt_age = time_uptime + age;
  }
  }
+ mtx_leave(&sc->sc_mtx);
  }
 }
 
@@ -394,6 +404,7 @@ bridge_rtflush(struct bridge_softc *sc,
  int i;
  struct bridge_rtnode *p, *n;
 
+ mtx_enter(&sc->sc_mtx);
  for (i = 0; i < BRIDGE_RTABLE_SIZE; i++) {
  n = LIST_FIRST(&sc->sc_rts[i]);
  while (n != NULL) {
@@ -408,6 +419,7 @@ bridge_rtflush(struct bridge_softc *sc,
  n = LIST_NEXT(n, brt_next);
  }
  }
+ mtx_leave(&sc->sc_mtx);
 }
 
 /*
@@ -420,14 +432,17 @@ bridge_rtdaddr(struct bridge_softc *sc,
  struct bridge_rtnode *p;
 
  h = bridge_hash(sc, ea);
+ mtx_enter(&sc->sc_mtx);
  LIST_FOREACH(p, &sc->sc_rts[h], brt_next) {
  if (memcmp(ea, &p->brt_addr, sizeof(p->brt_addr)) == 0) {
  LIST_REMOVE(p, brt_next);
  sc->sc_brtcnt--;
+ mtx_leave(&sc->sc_mtx);
  free(p, M_DEVBUF, sizeof *p);
  return (0);
  }
  }
+ mtx_leave(&sc->sc_mtx);
 
  return (ENOENT);
 }
@@ -445,6 +460,7 @@ bridge_rtdelete(struct bridge_softc *sc,
  * Loop through all of the hash buckets and traverse each
  * chain looking for routes to this interface.
  */
+ mtx_enter(&sc->sc_mtx);
  for (i = 0; i < BRIDGE_RTABLE_SIZE; i++) {
  n = LIST_FIRST(&sc->sc_rts[i]);
  while (n != NULL) {
@@ -466,6 +482,7 @@ bridge_rtdelete(struct bridge_softc *sc,
  n = p;
  }
  }
+ mtx_leave(&sc->sc_mtx);
 }
 
 /*
@@ -479,10 +496,12 @@ bridge_rtfind(struct bridge_softc *sc, s
  u_int32_t i = 0, total = 0;
  int k, error = 0;
 
+ mtx_enter(&sc->sc_mtx);
  for (k = 0; k < BRIDGE_RTABLE_SIZE; k++) {
  LIST_FOREACH(n, &sc->sc_rts[k], brt_next)
  total++;
  }
+ mtx_leave(&sc->sc_mtx);
 
  if (baconf->ifbac_len == 0) {
  i = total;
@@ -493,6 +512,7 @@ bridge_rtfind(struct bridge_softc *sc, s
  if (bareqs == NULL)
  goto done;
 
+ mtx_enter(&sc->sc_mtx);
  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))
@@ -511,6 +531,7 @@ bridge_rtfind(struct bridge_softc *sc, s
  i++;
  }
  }
+ mtx_leave(&sc->sc_mtx);
 
  error = copyout(bareqs, baconf->ifbac_req, i * sizeof(*bareqs));
 done:
Index: net/if_bridge.c
===================================================================
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.319
diff -u -p -r1.319 if_bridge.c
--- net/if_bridge.c 29 Jan 2019 17:47:35 -0000 1.319
+++ net/if_bridge.c 13 Feb 2019 13:48:36 -0000
@@ -174,6 +174,7 @@ bridge_clone_create(struct if_clone *ifc
  timeout_set(&sc->sc_brtimeout, bridge_rtage, sc);
  SLIST_INIT(&sc->sc_iflist);
  SLIST_INIT(&sc->sc_spanlist);
+ mtx_init(&sc->sc_mtx, IPL_MPFLOOR);
  for (i = 0; i < BRIDGE_RTABLE_SIZE; i++)
  LIST_INIT(&sc->sc_rts[i]);
  arc4random_buf(&sc->sc_hashkey, sizeof(sc->sc_hashkey));
Index: net/if_bridge.h
===================================================================
RCS file: /cvs/src/sys/net/if_bridge.h,v
retrieving revision 1.60
diff -u -p -r1.60 if_bridge.h
--- net/if_bridge.h 29 Jan 2019 17:47:35 -0000 1.60
+++ net/if_bridge.h 13 Feb 2019 13:41:17 -0000
@@ -242,6 +242,9 @@ struct ifbrlconf {
 };
 
 #ifdef _KERNEL
+
+#include <sys/mutex.h>
+
 /* STP port flags */
 #define BSTP_PORT_CANMIGRATE 0x0001
 #define BSTP_PORT_NEWINFO 0x0002
@@ -464,19 +467,25 @@ struct bridge_rtnode {
 #define BRIDGE_RTABLE_MASK (BRIDGE_RTABLE_SIZE - 1)
 
 /*
+ *  Locks used to protect struct members in this file:
+ * I immutable after creation
+ * m per-softc mutex
+ */
+/*
  * Software state for each bridge
  */
 struct bridge_softc {
  struct ifnet sc_if; /* the interface */
- u_int32_t sc_brtmax; /* max # addresses */
- u_int32_t sc_brtcnt; /* current # addrs */
+ uint32_t sc_brtmax; /* [m] max # addresses */
+ uint32_t sc_brtcnt; /* [m] current # addrs */
  int sc_brttimeout; /* timeout ticks */
- u_int64_t sc_hashkey[2]; /* siphash key */
+ uint64_t sc_hashkey[2]; /* [I] siphash key */
  struct timeout sc_brtimeout; /* timeout state */
  struct bstp_state *sc_stp; /* stp state */
  SLIST_HEAD(, bridge_iflist) sc_iflist; /* interface list */
  SLIST_HEAD(, bridge_iflist) sc_spanlist; /* span ports */
- LIST_HEAD(, bridge_rtnode) sc_rts[BRIDGE_RTABLE_SIZE]; /* hash table */
+ struct mutex sc_mtx; /* mutex */
+ LIST_HEAD(, bridge_rtnode) sc_rts[BRIDGE_RTABLE_SIZE]; /* [m] hash table */
 };
 
 extern const u_int8_t bstp_etheraddr[];