Lock down parallel router_info list for tracking multicast IGMP

versions of various routers seen:

- Introduce igmp_mtx.
- Protect global variable 'router_info_head' and list fields
  in struct router_info with this mutex, as well as
  igmp_timers_are_running.
- find_rti() asserts that the caller acquires igmp_mtx.
- Annotate a failure to check the return value of
  MALLOC(..., M_NOWAIT).

1 files changed, 27 insertions, 1 deletions

diff --git a/sys/netinet/igmp.c b/sys/netinet/igmp.c
index 2e8ef8d..3746396 100644
--- a/sys/netinet/igmp.c
+++ b/sys/netinet/igmp.c
@@ -80,10 +80,25 @@ static struct igmpstat igmpstat;
 SYSCTL_STRUCT(_net_inet_igmp, IGMPCTL_STATS, stats, CTLFLAG_RW, &igmpstat,
     igmpstat, "");
 
+/*
+ * igmp_mtx protects all mutable global variables in igmp.c, as well as
+ * the data fields in struct router_info.  In general, a router_info
+ * structure will be valid as long as the referencing struct in_multi is
+ * valid, so no reference counting is used.  We allow unlocked reads of
+ * router_info data when accessed via an in_multi read-only.
+ */
+static struct mtx igmp_mtx;
 static SLIST_HEAD(, router_info) router_info_head;
 static int igmp_timers_are_running;
+
+/*
+ * XXXRW: can we define these such that these can be made const?  In any
+ * case, these shouldn't be changed after igmp_init() and therefore don't
+ * need locking.
+ */
 static u_long igmp_all_hosts_group;
 static u_long igmp_all_rtrs_group;
+
 static struct mbuf *router_alert;
 static struct route igmprt;
 
@@ -118,6 +133,7 @@ igmp_init(void)
 	ra->ipopt_list[3] = 0x00;
 	router_alert->m_len = sizeof(ra->ipopt_dst) + ra->ipopt_list[1];
 
+	mtx_init(&igmp_mtx, "igmp_mtx", NULL, MTX_DEF);
 	SLIST_INIT(&router_info_head);
 }
 
@@ -126,6 +142,7 @@ find_rti(struct ifnet *ifp)
 {
 	struct router_info *rti;
 
+	mtx_assert(&igmp_mtx, MA_OWNED);
 	IGMP_PRINTF("[igmp.c, _find_rti] --> entering \n");
 	SLIST_FOREACH(rti, &router_info_head, rti_list) {
 		if (rti->rti_ifp == ifp) {
@@ -134,6 +151,9 @@ find_rti(struct ifnet *ifp)
 			return rti;
 		}
 	}
+	/*
+	 * XXXRW: return value of malloc not checked, despite M_NOWAIT.
+	 */
 	MALLOC(rti, struct router_info *, sizeof *rti, M_IGMP, M_NOWAIT);
 	rti->rti_ifp = ifp;
 	rti->rti_type = IGMP_V2_ROUTER;
@@ -197,7 +217,6 @@ igmp_input(register struct mbuf *m, int off)
 	timer = igmp->igmp_code * PR_FASTHZ / IGMP_TIMER_SCALE;
 	if (timer == 0)
 		timer = 1;
-	rti = find_rti(ifp);
 
 	/*
 	 * In the IGMPv2 specification, there are 3 states and a flag.
@@ -224,8 +243,11 @@ igmp_input(register struct mbuf *m, int off)
 			 * value in RFC 1112.
 			 */
 
+			mtx_lock(&igmp_mtx);
+			rti = find_rti(ifp);
 			rti->rti_type = IGMP_V1_ROUTER;
 			rti->rti_time = 0;
+			mtx_unlock(&igmp_mtx);
 
 			timer = IGMP_MAX_HOST_REPORT_DELAY * PR_FASTHZ;
 
@@ -344,7 +366,9 @@ igmp_joingroup(struct in_multi *inm)
 		inm->inm_timer = 0;
 		inm->inm_state = IGMP_OTHERMEMBER;
 	} else {
+		mtx_lock(&igmp_mtx);
 		inm->inm_rti = find_rti(inm->inm_ifp);
+		mtx_unlock(&igmp_mtx);
 		igmp_sendpkt(inm, inm->inm_rti->rti_type, 0);
 		inm->inm_timer = IGMP_RANDOM_DELAY(
 					IGMP_MAX_HOST_REPORT_DELAY*PR_FASTHZ);
@@ -404,6 +428,7 @@ igmp_slowtimo(void)
 	struct router_info *rti;
 
 	IGMP_PRINTF("[igmp.c,_slowtimo] -- > entering \n");
+	mtx_lock(&igmp_mtx);
 	SLIST_FOREACH(rti, &router_info_head, rti_list) {
 		if (rti->rti_type == IGMP_V1_ROUTER) {
 			rti->rti_time++;
@@ -411,6 +436,7 @@ igmp_slowtimo(void)
 				rti->rti_type = IGMP_V2_ROUTER;
 		}
 	}
+	mtx_unlock(&igmp_mtx);
 	IGMP_PRINTF("[igmp.c,_slowtimo] -- > exiting \n");
 	splx(s);
 }