kern/43349: if_detach causes kernel crash

[o.vd.linden%quicknet.nl@localhost]

>Number:         43349
>Category:       kern
>Synopsis:       if_detach causes kernel crash
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon May 24 19:15:00 +0000 2010
>Originator:     Onno van der Linden
>Release:        5.99.29 i386
>Organization:
>Environment:
NetBSD sheep 5.99.29 NetBSD 5.99.29 (SHEEP) #3: Mon May 17 22:50:14 MEST 2010  
root@sheep:/usr/src/sys/arch/i386/compile/SHEEP i386

>Description:
Same crash as described in 
http://mail-index.netbsd.org/tech-kern/2009/10/28/msg006374.html.
Likely to be caused (in my case at least) by a cloned route: removing those by 
hand followed by
a reboot/halt/shutdown avoids the crash in rn_walktree.  The optimizer inlines 
the call to rn_walknext
which is where the the crash happens because  rn=rn->rn_l results in rn getting 
a NULL value
which gets referenced  at the next iteration of the for loop.





>How-To-Repeat:
If your route table has a cloned route it'll probably crash every time you use 
shutdown/reboot/halt.
>Fix:
Based on http://openbsd.monkey.org/tech/200603/msg00007.html
I made a fix myself and a little later I came across 
http://mail-index.netbsd.org/tech-kern/2009/10/28/msg006380.html which 
according to
http://mail-index.netbsd.org/tech-kern/2009/11/04/msg006433.html works fine too
and is a lot shorter than the diff I made.

*** /sys/net/if.c       Thu Jan 28 15:12:11 2010
--- /sys/net/if.c.fixed Mon May 17 22:48:34 2010
***************
*** 792,799 ****
        if_free_sadl(ifp);
  
        /* Walk the routing table looking for stragglers. */
!       for (i = 0; i <= AF_MAX; i++)
!               (void)rt_walktree(i, if_rt_walktree, ifp);
  
        DOMAIN_FOREACH(dp) {
                if (dp->dom_ifdetach != NULL && ifp->if_afdata[dp->dom_family])
--- 792,801 ----
        if_free_sadl(ifp);
  
        /* Walk the routing table looking for stragglers. */
!       for (i = 0; i <= AF_MAX; i++) {
!               while (rt_walktree(i, if_rt_walktree, ifp) == EAGAIN)
!                       ;
!       }
  
        DOMAIN_FOREACH(dp) {
                if (dp->dom_ifdetach != NULL && ifp->if_afdata[dp->dom_family])
***************
*** 884,899 ****
  /*
   * Callback for a radix tree walk to delete all references to an
   * ifnet.
   */
  static int
  if_rt_walktree(struct rtentry *rt, void *v)
  {
        struct ifnet *ifp = (struct ifnet *)v;
!       int error;
  
        if (rt->rt_ifp != ifp)
                return 0;
  
        /* Delete the entry. */
        ++rt->rt_refcnt;
        error = rtrequest(RTM_DELETE, rt_getkey(rt), rt->rt_gateway,
--- 886,906 ----
  /*
   * Callback for a radix tree walk to delete all references to an
   * ifnet.
+  * Note that deleting a RTF_CLONING route can trigger the
+  * deletion of more entries, so we need to cancel the walk
+  * and return EAGAIN.  The caller should restart the walk
+  * as long as EAGAIN is returned.
   */
  static int
  if_rt_walktree(struct rtentry *rt, void *v)
  {
        struct ifnet *ifp = (struct ifnet *)v;
!       int error, cloning;
  
        if (rt->rt_ifp != ifp)
                return 0;
  
+       cloning = (rt->rt_flags & RTF_CLONING);
        /* Delete the entry. */
        ++rt->rt_refcnt;
        error = rtrequest(RTM_DELETE, rt_getkey(rt), rt->rt_gateway,
***************
*** 904,909 ****
--- 911,918 ----
        if (error != 0)
                printf("%s: warning: unable to delete rtentry @ %p, "
                    "error = %d\n", ifp->if_xname, rt, error);
+       else if (cloning)
+               return EAGAIN;
        return 0;
  }