From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: ip_mroute: replace and kill mrt_count
To: tech@openbsd.org
Date: Thu, 5 Nov 2026 14:38:39 +0100

The IP mrouter code has this mrt_count that is only used for one thing.
It is to gate MRT_API_CONFIG from being called after some other config was
done to the mrouter state.

MRT_API_CONFIG must be called right after MRT_INIT and it should only work
then. The moment a VIF or MFC is added MRT_API_CONFIG needs to fail.

Doing this the way it is done -- by checking if any VIF or MFC exists for
that mrouter -- is overly complex. Instead just use a flag that is toggled
on the first addition of a VIF / MFC.

This simplifies the locking because the new mrt_api_lock is only accessed
in the setsockopt handler (and that one uses an exclusive socket lock).

-- 
:wq Claudio

Index: ip_mroute.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_mroute.c,v
diff -u -p -r1.155 ip_mroute.c
--- ip_mroute.c	17 Sep 2026 15:56:59 -0000	1.155
+++ ip_mroute.c	5 Nov 2026 10:54:11 -0000
@@ -79,8 +79,8 @@
 /*
  * Locks used to protect data:
  *	I	immutable after creation
- *	N	net lock
  *	R	multicast router lock
+ *	S	socket lock
  */
 
 /* #define MCAST_DEBUG */
@@ -111,7 +111,7 @@ static const u_int32_t mrt_api_support =
 struct rttimer_queue mrt_timer;
 struct socket	*mrt_mrouter[RT_TABLEID_MAX + 1];	/* [R] */
 uint32_t	 mrt_api_config[RT_TABLEID_MAX + 1];	/* [R] */
-uint64_t	 mrt_count[RT_TABLEID_MAX + 1];		/* [N] */
+int		 mrt_api_lock[RT_TABLEID_MAX + 1];	/* [S] */
 int		 ip_mrtproto = IGMP_DVMRP;	/* [I] for netstat only */
 
 struct cpumem *mrtcounters;
@@ -671,12 +671,6 @@ ip_mrouter_done(struct socket *so)
 		rtfree(rt);
 	} while (error == EAGAIN);
 
-	/*
-	 * XXX Should be KASSERT, but there is a leak.  If an interface
-	 * is destroyed, the route is deleted, but mrt_mcast_del() not run.
-	 */
-	mrt_count[rtableid] = 0;
-
 	/* Unregister all interfaces in the domain. */
 	TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
 		if (ifp->if_rdomain != rtableid)
@@ -712,7 +706,6 @@ int
 set_api_config(struct socket *so, struct mbuf *m)
 {
 	struct inpcb *inp = sotoinpcb(so);
-	struct ifnet *ifp;
 	uint32_t *apival;
 	unsigned int rtableid = inp->inp_rtableid;
 
@@ -732,21 +725,9 @@ set_api_config(struct socket *so, struct
 
 	/*
 	 * We can set the API capabilities only if it is the first operation
-	 * after MRT_INIT. I.e.:
-	 *  - there are no vifs installed
-	 *  - the MFC table is empty
+	 * after MRT_INIT.
 	 */
-	TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
-		if (ifp->if_rdomain != rtableid)
-			continue;
-		if (ifp->if_mcast == NULL)
-			continue;
-
-		rw_exit_write(&mrt_routerlock);
-		*apival = 0;
-		return (EPERM);
-	}
-	if (mrt_count[rtableid] > 0) {
+	if (mrt_api_lock[rtableid]) {
 		rw_exit_write(&mrt_routerlock);
 		*apival = 0;
 		return (EPERM);
@@ -754,6 +735,7 @@ set_api_config(struct socket *so, struct
 
 	*apival &= mrt_api_support;
 	mrt_api_config[rtableid] = *apival;
+	mrt_api_lock[rtableid] = 1;
 
 	rw_exit_write(&mrt_routerlock);
 
@@ -873,6 +855,8 @@ add_vif(struct socket *so, struct mbuf *
 	vifp->v_lcl_addr = vifcp->vifc_lcl_addr;
 	vifp->v_rmt_addr = vifcp->vifc_rmt_addr;
 
+	mrt_api_lock[rtableid] = 1;
+
 	return (0);
 }
 
@@ -1161,6 +1145,7 @@ add_mfc(struct socket *so, struct mbuf *
 		    sizeof(mfcctl2) - sizeof(struct mfcctl));
 	}
 
+	mrt_api_lock[rtableid] = 1;
 	return (mfc_add(&mfcctl2, &mfcctl2.mfcc_origin, &mfcctl2.mfcc_mcastgrp,
 	    mfcctl2.mfcc_parent, rtableid, M_WAITOK));
 }
@@ -1505,8 +1490,6 @@ rt_mcast_add(struct ifnet *ifp, struct s
 		return (NULL);
 	}
 
-	mrt_count[rtableid]++;
-
 	return (mfc_find(ifp, &satosin(group)->sin_addr, rtableid));
 }
 
@@ -1521,8 +1504,6 @@ mrt_mcast_del(struct rtentry *rt, unsign
 
 	free(rt->rt_llinfo, M_MRTABLE, sizeof(struct mfc));
 	rt->rt_llinfo = NULL;
-
-	mrt_count[rtableid]--;
 
 	ifp = if_get(rt->rt_ifidx);
 	if (ifp == NULL)