i386/82882: [ PATCH ] ip_mroute abends kernel when interface detached

[Dan Lukes]

>Number:         82882
>Category:       i386
>Synopsis:       [ PATCH ] ip_mroute abends kernel when interface detached
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-i386
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Jul 02 02:40:14 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Dan Lukes
>Release:        FreeBSD 5.4-STABLE i386
>Organization:
Obludarium
>Environment:
System: FreeBSD 5.4-STABLE i386
src/sys/netinet/ip_mroute.c,v 1.106.2.3 2004/10/09 15:12:04 rwatson

>Description:
	MROUTE code didn't care about interface deletion. It's maintain it's
own copy of ifp within it's table. Using it copy after interface detached
may cause kernel abend or other malfunction.

>How-To-Repeat:
	ifconfig vlan666 create
	ifconfig vlan666 127.0.0.5/32 vlan 666 vlandev <a parent interface>
	run a mroute daemon which add vlan666 as vif to kernel
	ifconfig vlan666 destroy
	now shutdown the mrouted - kernel abend during MRT_DEL_VIF (or MRT_DONE)

>Fix:

	ip_mroute should monitor if_detach interface event invoked by if.c:if_detach()
	It should delete delte destryed interface from it's tables

--- ip_mroute.patch begins here ---
--- sys/netinet/ip_mroute.c.ORIG	Tue Nov 30 14:58:31 2004
+++ sys/netinet/ip_mroute.c	Sat Jul  2 04:11:13 2005
@@ -124,6 +124,8 @@
 
 static u_char		nexpire[MFCTBLSIZ];
 
+static eventhandler_tag if_detach_event_tag = NULL;
+
 static struct callout expire_upcalls_ch;
 
 #define		EXPIRE_TIMEOUT	(hz / 4)	/* 4x / second		*/
@@ -268,8 +270,10 @@
 
 static int get_sg_cnt(struct sioc_sg_req *);
 static int get_vif_cnt(struct sioc_vif_req *);
+static void if_detached_event(void *arg __unused, struct ifnet *);
 static int ip_mrouter_init(struct socket *, int);
 static int add_vif(struct vifctl *);
+static int del_vif_locked(vifi_t);
 static int del_vif(vifi_t);
 static int add_mfc(struct mfcctl2 *);
 static int del_mfc(struct mfcctl2 *);
@@ -619,6 +623,29 @@
 
 static struct mtx mrouter_mtx;		/* used to synch init/done work */
 
+static void
+if_detached_event(void *arg __unused, struct ifnet *ifp)
+{
+    vifi_t vifi;
+
+    mtx_lock(&mrouter_mtx);
+
+    if (ip_mrouter == NULL) {
+	mtx_unlock(&mrouter_mtx);
+    }
+
+    VIF_LOCK();
+    /*
+     * For each phyint in use, disable promiscuous reception of all IP
+     * multicasts.
+     */
+    for (vifi = 0; vifi < numvifs; vifi++)
+	if (viftable[vifi].v_ifp == ifp)
+	    del_vif_locked(vifi);
+    VIF_UNLOCK();
+    mtx_unlock(&mrouter_mtx);
+}
+                        
 /*
  * Enable multicast routing
  */
@@ -642,6 +669,11 @@
 	return EADDRINUSE;
     }
 
+    if_detach_event_tag = EVENTHANDLER_REGISTER(ifnet_departure_event, 
+        if_detached_event, NULL, EVENTHANDLER_PRI_ANY);
+    if (if_detach_event_tag == NULL)
+	return (ENOMEM);
+
     callout_reset(&expire_upcalls_ch, EXPIRE_TIMEOUT, expire_upcalls, NULL);
 
     callout_reset(&bw_upcalls_ch, BW_UPCALLS_PERIOD,
@@ -716,6 +748,7 @@
     numvifs = 0;
     pim_assert = 0;
     VIF_UNLOCK();
+    EVENTHANDLER_DEREGISTER(ifnet_departure_event, if_detach_event_tag);
 
     /*
      * Free all multicast forwarding cache entries.
@@ -1050,19 +1083,17 @@
  * Delete a vif from the vif table
  */
 static int
-del_vif(vifi_t vifi)
+del_vif_locked(vifi_t vifi)
 {
     struct vif *vifp;
 
-    VIF_LOCK();
+    VIF_LOCK_ASSERT();
 
     if (vifi >= numvifs) {
-	VIF_UNLOCK();
 	return EINVAL;
     }
     vifp = &viftable[vifi];
     if (vifp->v_lcl_addr.s_addr == INADDR_ANY) {
-	VIF_UNLOCK();
 	return EADDRNOTAVAIL;
     }
 
@@ -1101,9 +1132,19 @@
 	    break;
     numvifs = vifi;
 
+    return 0;
+}
+
+static int
+del_vif(vifi_t vifi)
+{
+    int cc;
+
+    VIF_LOCK();
+    cc = del_vif_locked(vifi);
     VIF_UNLOCK();
 
-    return 0;
+    return cc;
 }
 
 /*
--- ip_mroute.patch ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted: