Lock down global variables in if_gre:

- Add gre_mtx to protect global softc list.
- Hold gre_mtx over various list operations (insert, delete).
- Centralize if_gre interface teardown in gre_destroy(), and call this
  from modevent unload and gre_clone_destroy().
- Export gre_mtx to ip_gre.c, which walks the gre list to look up gre
  interfaces during encapsulation.  Add a wonking comment on how we need
  some sort of drain/reference count mechanism to keep gre references
  alive while in use and simultaneous destroy.

This commit does not lockdown softc data, which follows in a future
commit.

3 files changed, 46 insertions, 9 deletions

diff --git a/sys/net/if_gre.c b/sys/net/if_gre.c
index 254d9a3..a2f731e 100644
--- a/sys/net/if_gre.c
+++ b/sys/net/if_gre.c
@@ -91,6 +91,11 @@
 
 #define GRENAME	"gre"
 
+/*
+ * gre_mtx protects all global variables in if_gre.c.
+ * XXX: gre_softc data not protected yet.
+ */
+struct mtx gre_mtx;
 static MALLOC_DEFINE(M_GRE, GRENAME, "Generic Routing Encapsulation");
 
 struct gre_softc_head gre_softc_list;
@@ -149,6 +154,7 @@ static void
 greattach(void)
 {
 
+	mtx_init(&gre_mtx, "gre_mtx", NULL, MTX_DEF);
 	LIST_INIT(&gre_softc_list);
 	if_clone_attach(&gre_cloner);
 }
@@ -181,26 +187,37 @@ gre_clone_create(ifc, unit)
 	sc->wccp_ver = WCCP_V1;
 	if_attach(&sc->sc_if);
 	bpfattach(&sc->sc_if, DLT_NULL, sizeof(u_int32_t));
+	mtx_lock(&gre_mtx);
 	LIST_INSERT_HEAD(&gre_softc_list, sc, sc_list);
+	mtx_unlock(&gre_mtx);
 	return (0);
 }
 
 static void
-gre_clone_destroy(ifp)
-	struct ifnet *ifp;
+gre_destroy(struct gre_softc *sc)
 {
-	struct gre_softc *sc = ifp->if_softc;
 
 #ifdef INET
 	if (sc->encap != NULL)
 		encap_detach(sc->encap);
 #endif
-	LIST_REMOVE(sc, sc_list);
-	bpfdetach(ifp);
-	if_detach(ifp);
+	bpfdetach(&sc->sc_if);
+	if_detach(&sc->sc_if);
 	free(sc, M_GRE);
 }
 
+static void
+gre_clone_destroy(ifp)
+	struct ifnet *ifp;
+{
+	struct gre_softc *sc = ifp->if_softc;
+
+	mtx_lock(&gre_mtx);
+	LIST_REMOVE(sc, sc_list);
+	mtx_unlock(&gre_mtx);
+	gre_destroy(sc);
+}
+
 /*
  * The output routine. Takes a packet and encapsulates it in the protocol
  * given by sc->g_proto. See also RFC 1701 and RFC 2004
@@ -727,6 +744,7 @@ gre_in_cksum(u_int16_t *p, u_int len)
 static int
 gremodevent(module_t mod, int type, void *data)
 {
+	struct gre_softc *sc;
 
 	switch (type) {
 	case MOD_LOAD:
@@ -735,8 +753,15 @@ gremodevent(module_t mod, int type, void *data)
 	case MOD_UNLOAD:
 		if_clone_detach(&gre_cloner);
 
-		while (!LIST_EMPTY(&gre_softc_list))
-			gre_clone_destroy(&LIST_FIRST(&gre_softc_list)->sc_if);
+		mtx_lock(&gre_mtx);
+		while ((sc = LIST_FIRST(&gre_softc_list)) != NULL) {
+			LIST_REMOVE(sc, sc_list);
+			mtx_unlock(&gre_mtx);
+			gre_destroy(sc);
+			mtx_lock(&gre_mtx);
+		}
+		mtx_unlock(&gre_mtx);
+		mtx_destroy(&gre_mtx);
 		break;
 	}
 	return 0;
diff --git a/sys/net/if_gre.h b/sys/net/if_gre.h
index 21fab39..2910f2f 100644
--- a/sys/net/if_gre.h
+++ b/sys/net/if_gre.h
@@ -176,6 +176,7 @@ struct mobip_h {
 
 #ifdef _KERNEL
 LIST_HEAD(gre_softc_head, gre_softc);
+extern struct mtx gre_mtx;
 extern struct gre_softc_head gre_softc_list;
 
 u_int16_t	gre_in_cksum(u_int16_t *, u_int);
diff --git a/sys/netinet/ip_gre.c b/sys/netinet/ip_gre.c
index 960b8a8..c11317d 100644
--- a/sys/netinet/ip_gre.c
+++ b/sys/netinet/ip_gre.c
@@ -314,6 +314,13 @@ gre_mobile_input(m, va_alist)
 
 /*
  * Find the gre interface associated with our src/dst/proto set.
+ *
+ * XXXRW: Need some sort of drain/refcount mechanism so that the softc
+ * reference remains valid after it's returned from gre_lookup().  Right
+ * now, I'm thinking it should be reference-counted with a gre_dropref()
+ * when the caller is done with the softc.  This is complicated by how
+ * to handle destroying the gre softc; probably using a gre_drain() in
+ * in_gre.c during destroy.
  */
 static struct gre_softc *
 gre_lookup(m, proto)
@@ -323,14 +330,18 @@ gre_lookup(m, proto)
 	struct ip *ip = mtod(m, struct ip *);
 	struct gre_softc *sc;
 
+	mtx_lock(&gre_mtx);
 	for (sc = LIST_FIRST(&gre_softc_list); sc != NULL;
 	     sc = LIST_NEXT(sc, sc_list)) {
 		if ((sc->g_dst.s_addr == ip->ip_src.s_addr) &&
 		    (sc->g_src.s_addr == ip->ip_dst.s_addr) &&
 		    (sc->g_proto == proto) &&
-		    ((sc->sc_if.if_flags & IFF_UP) != 0))
+		    ((sc->sc_if.if_flags & IFF_UP) != 0)) {
+			mtx_unlock(&gre_mtx);
 			return (sc);
+		}
 	}
+	mtx_unlock(&gre_mtx);
 
 	return (NULL);
 }