Module Name: src
Committed By: ozaki-r
Date: Wed Apr 18 04:01:58 UTC 2018
Modified Files:
src/sys/net: if_bridge.c if_bridgevar.h
Log Message:
bridge: use pslist(9) for rtlist and rthash
The change fixes race conditions on list operations. One example is that a
reader may see invalid pointers on a looking item in a list due to lack of
membar_producer.
To generate a diff of this commit:
cvs rdiff -u -r1.151 -r1.152 src/sys/net/if_bridge.c
cvs rdiff -u -r1.31 -r1.32 src/sys/net/if_bridgevar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/net/if_bridge.c
diff -u src/sys/net/if_bridge.c:1.151 src/sys/net/if_bridge.c:1.152
--- src/sys/net/if_bridge.c:1.151 Wed Apr 18 03:49:44 2018
+++ src/sys/net/if_bridge.c Wed Apr 18 04:01:58 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: if_bridge.c,v 1.151 2018/04/18 03:49:44 ozaki-r Exp $ */
+/* $NetBSD: if_bridge.c,v 1.152 2018/04/18 04:01:58 ozaki-r Exp $ */
/*
* Copyright 2001 Wasabi Systems, Inc.
@@ -80,7 +80,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.151 2018/04/18 03:49:44 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.152 2018/04/18 04:01:58 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_bridge_ipf.h"
@@ -191,6 +191,29 @@ __CTASSERT(offsetof(struct ifbifconf, if
#define BRIDGE_RT_RENTER(__s) do { __s = pserialize_read_enter(); } while (0)
#define BRIDGE_RT_REXIT(__s) do { pserialize_read_exit(__s); } while (0)
+#define BRIDGE_RTLIST_READER_FOREACH(_brt, _sc) \
+ PSLIST_READER_FOREACH((_brt), &((_sc)->sc_rtlist), \
+ struct bridge_rtnode, brt_list)
+#define BRIDGE_RTLIST_WRITER_FOREACH(_brt, _sc) \
+ PSLIST_WRITER_FOREACH((_brt), &((_sc)->sc_rtlist), \
+ struct bridge_rtnode, brt_list)
+#define BRIDGE_RTLIST_WRITER_INSERT_HEAD(_sc, _brt) \
+ PSLIST_WRITER_INSERT_HEAD(&(_sc)->sc_rtlist, brt, brt_list)
+#define BRIDGE_RTLIST_WRITER_REMOVE(_brt) \
+ PSLIST_WRITER_REMOVE((_brt), brt_list)
+
+#define BRIDGE_RTHASH_READER_FOREACH(_brt, _sc, _hash) \
+ PSLIST_READER_FOREACH((_brt), &(_sc)->sc_rthash[(_hash)], \
+ struct bridge_rtnode, brt_hash)
+#define BRIDGE_RTHASH_WRITER_FOREACH(_brt, _sc, _hash) \
+ PSLIST_WRITER_FOREACH((_brt), &(_sc)->sc_rthash[(_hash)], \
+ struct bridge_rtnode, brt_hash)
+#define BRIDGE_RTHASH_WRITER_INSERT_HEAD(_sc, _hash, _brt) \
+ PSLIST_WRITER_INSERT_HEAD(&(_sc)->sc_rthash[(_hash)], brt, brt_hash)
+#define BRIDGE_RTHASH_WRITER_INSERT_AFTER(_brt, _new) \
+ PSLIST_WRITER_INSERT_AFTER((_brt), (_new), brt_hash)
+#define BRIDGE_RTHASH_WRITER_REMOVE(_brt) \
+ PSLIST_WRITER_REMOVE((_brt), brt_hash)
#ifdef NET_MPSAFE
#define DECLARE_LOCK_VARIABLE
@@ -1039,7 +1062,7 @@ bridge_ioctl_rts(struct bridge_softc *sc
BRIDGE_RT_LOCK(sc);
len = bac->ifbac_len;
- LIST_FOREACH(brt, &sc->sc_rtlist, brt_list) {
+ BRIDGE_RTLIST_WRITER_FOREACH(brt, sc) {
if (len < sizeof(bareq))
goto out;
memset(&bareq, 0, sizeof(bareq));
@@ -2105,7 +2128,7 @@ typedef bool (*bridge_iterate_cb_t)
static void
bridge_rtlist_iterate_remove(struct bridge_softc *sc, bridge_iterate_cb_t func, void *arg)
{
- struct bridge_rtnode *brt, *nbrt;
+ struct bridge_rtnode *brt;
struct bridge_rtnode **brt_list;
int i, count;
@@ -2124,7 +2147,12 @@ retry:
}
i = 0;
- LIST_FOREACH_SAFE(brt, &sc->sc_rtlist, brt_list, nbrt) {
+ /*
+ * We don't need to use a _SAFE variant here because we know
+ * that a removed item keeps its next pointer as-is thanks to
+ * pslist(9) and isn't freed in the loop.
+ */
+ BRIDGE_RTLIST_WRITER_FOREACH(brt, sc) {
bool need_break = false;
if (func(sc, brt, &need_break, arg)) {
bridge_rtnode_remove(sc, brt);
@@ -2298,7 +2326,7 @@ bridge_rtdelete(struct bridge_softc *sc,
/* XXX pserialize_perform for each entry is slow */
again:
BRIDGE_RT_LOCK(sc);
- LIST_FOREACH(brt, &sc->sc_rtlist, brt_list) {
+ BRIDGE_RTLIST_WRITER_FOREACH(brt, sc) {
if (brt->brt_ifp == ifp)
break;
}
@@ -2329,11 +2357,11 @@ bridge_rtable_init(struct bridge_softc *
KM_SLEEP);
for (i = 0; i < BRIDGE_RTHASH_SIZE; i++)
- LIST_INIT(&sc->sc_rthash[i]);
+ PSLIST_INIT(&sc->sc_rthash[i]);
sc->sc_rthash_key = cprng_fast32();
- LIST_INIT(&sc->sc_rtlist);
+ PSLIST_INIT(&sc->sc_rtlist);
sc->sc_rtlist_psz = pserialize_create();
sc->sc_rtlist_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_SOFTNET);
@@ -2402,7 +2430,7 @@ bridge_rtnode_lookup(struct bridge_softc
int dir;
hash = bridge_rthash(sc, addr);
- LIST_FOREACH(brt, &sc->sc_rthash[hash], brt_hash) {
+ BRIDGE_RTHASH_READER_FOREACH(brt, sc, hash) {
dir = memcmp(addr, brt->brt_addr, ETHER_ADDR_LEN);
if (dir == 0)
return brt;
@@ -2428,7 +2456,7 @@ bridge_rtnode_insert(struct bridge_softc
KASSERT(BRIDGE_RT_LOCKED(sc));
hash = bridge_rthash(sc, brt->brt_addr);
- LIST_FOREACH(lbrt, &sc->sc_rthash[hash], brt_hash) {
+ BRIDGE_RTHASH_WRITER_FOREACH(lbrt, sc, hash) {
int dir = memcmp(brt->brt_addr, lbrt->brt_addr, ETHER_ADDR_LEN);
if (dir == 0)
return EEXIST;
@@ -2437,11 +2465,11 @@ bridge_rtnode_insert(struct bridge_softc
prev = lbrt;
}
if (prev == NULL)
- LIST_INSERT_HEAD(&sc->sc_rthash[hash], brt, brt_hash);
+ BRIDGE_RTHASH_WRITER_INSERT_HEAD(sc, hash, brt);
else
- LIST_INSERT_AFTER(prev, brt, brt_hash);
+ BRIDGE_RTHASH_WRITER_INSERT_AFTER(prev, brt);
- LIST_INSERT_HEAD(&sc->sc_rtlist, brt, brt_list);
+ BRIDGE_RTLIST_WRITER_INSERT_HEAD(sc, brt);
sc->sc_brtcnt++;
return 0;
@@ -2458,8 +2486,8 @@ bridge_rtnode_remove(struct bridge_softc
KASSERT(BRIDGE_RT_LOCKED(sc));
- LIST_REMOVE(brt, brt_hash);
- LIST_REMOVE(brt, brt_list);
+ BRIDGE_RTHASH_WRITER_REMOVE(brt);
+ BRIDGE_RTLIST_WRITER_REMOVE(brt);
sc->sc_brtcnt--;
}
Index: src/sys/net/if_bridgevar.h
diff -u src/sys/net/if_bridgevar.h:1.31 src/sys/net/if_bridgevar.h:1.32
--- src/sys/net/if_bridgevar.h:1.31 Thu Apr 28 00:16:56 2016
+++ src/sys/net/if_bridgevar.h Wed Apr 18 04:01:58 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: if_bridgevar.h,v 1.31 2016/04/28 00:16:56 ozaki-r Exp $ */
+/* $NetBSD: if_bridgevar.h,v 1.32 2018/04/18 04:01:58 ozaki-r Exp $ */
/*
* Copyright 2001 Wasabi Systems, Inc.
@@ -275,8 +275,8 @@ struct bridge_iflist {
* Bridge route node.
*/
struct bridge_rtnode {
- LIST_ENTRY(bridge_rtnode) brt_hash; /* hash table linkage */
- LIST_ENTRY(bridge_rtnode) brt_list; /* list linkage */
+ struct pslist_entry brt_hash; /* hash table linkage */
+ struct pslist_entry brt_list; /* list linkage */
struct ifnet *brt_ifp; /* destination if */
time_t brt_expire; /* expiration time */
uint8_t brt_flags; /* address flags */
@@ -319,8 +319,8 @@ struct bridge_softc {
callout_t sc_brcallout; /* bridge callout */
callout_t sc_bstpcallout; /* STP callout */
struct bridge_iflist_psref sc_iflist_psref;
- LIST_HEAD(, bridge_rtnode) *sc_rthash; /* our forwarding table */
- LIST_HEAD(, bridge_rtnode) sc_rtlist; /* list version of above */
+ struct pslist_head *sc_rthash; /* our forwarding table */
+ struct pslist_head sc_rtlist; /* list version of above */
kmutex_t *sc_rtlist_lock;
pserialize_t sc_rtlist_psz;
struct workqueue *sc_rtage_wq;
