kern/130977: [netgraph][pf] kernel panic trap 12 on user connect to VPN server

[Mikolaj Golub]

The following reply was made to PR kern/130977; it has been noted by GNATS.

From: Mikolaj Golub <to.my.trociny at gmail.com>
To: bug-followup at FreeBSD.org,darkibot at gmail.com
Cc:  
Subject: Re: kern/130977: [netgraph][pf] kernel panic trap 12 on user connect to VPN server
Date: Fri, 10 Apr 2009 14:42:59 +0300

 The problem here (as in kern/131310 and may be in some other reports) is that
 net/if.c:if_attach() when attaching interface adds it to default group ALL
 calling if_addgroup(ifp, IFG_ALL). But when interface is removed (in this case
 ng, but the same thing occurs for other interfaces too, e.g. I checked it for
 tap) the reference to it does not removed from ifgl_group.ifg_members list.
 
 The simple test can be used to confirm this:
 
 1) add interface (e.g. starting mpd);
 
 2) run kgdb and find reference to ng interface in the list 
 ifnet.tqh_first.if_groups->tqh_first.ifgl_group.ifg_members
 
 E.g. in my case it is:
 
 (kgdb) p *ifnet.tqh_first.if_groups->tqh_first.ifgl_group.ifg_members.tqh_first.ifgm_next.tqe_next.ifgm_next.tqe_next.ifgm_next.tqe_next.ifgm_ifp
 $1 = {if_softc = 0xc4e180c0, if_l2com = 0x0, if_link = {tqe_next = 0x0, tqe_prev = 0xc4264808}, 
   if_xname = "ng0", '\0' <repeats 12 times>, if_dname = 0xc4bd60d9 "ng", if_dunit = 0, if_addrhead = {
     tqh_first = 0xc4ba4e00, tqh_last = 0xc4ba4e60}, if_klist = {kl_list = {slh_first = 0x0}, 
     kl_lock = 0xc07abb00 <knlist_mtx_lock>, kl_unlock = 0xc07abb30 <knlist_mtx_unlock>, 
 ...
 
 3) remove ng interface (e.g. stopping mpd). Check that in the list
 ifnet.tqh_first.if_groups->tqh_first.ifgl_group.ifg_members we still have the reference
 to already removed interface:
 
 (kgdb) p *ifnet.tqh_first.if_groups->tqh_first.ifgl_group.ifg_members.tqh_first.ifgm_next.tqe_next.ifgm_next.tqe_next.ifgm_next.tqe_next.ifgm_ifp
 $2 = {if_softc = 0xdeadc0de, if_l2com = 0xdeadc0de, if_link = {tqe_next = 0xdeadc0de, tqe_prev = 0xdeadc0de}, 
   if_xname = "ÞÀ­ÞÞÀ­ÞÞÀ­ÞÞÀ­Þ", if_dname = 0xdeadc0de <Error reading address 0xdeadc0de: Bad address>, 
   if_dunit = -559038242, if_addrhead = {tqh_first = 0xdeadc0de, tqh_last = 0xdeadc0de}, if_klist = {kl_list = {
       slh_first = 0xdeadc0de}, kl_lock = 0xdeadc0de, kl_unlock = 0xdeadc0de, kl_locked = 0xdeadc0de, 
 
 If you repeat this process many times you will have the long least of invalid ifgm_ifp references.
 
 pf traverses the list ifnet.tqh_first.if_groups->tqh_first.ifgl_group.ifg_members in  
 pfi_table_update and calls pfi_instance_add() with nonvalid ifgm_ifp argument
 and the system panics trying to access invalid memory.
 
 I don't know if this correct solution but adding if_delgroup(ifp, IFG_ALL) to 
 sys/net/if.c:if_detach() fixes the problem for me.
 
 --- sys/net/if.c.orig   2009-04-01 10:53:55.000000000 +0300
 +++ sys/net/if.c        2009-04-10 12:38:14.000000000 +0300
 @@ -846,6 +846,7 @@ if_detach(struct ifnet *ifp)
         mtx_destroy(&ifp->if_snd.ifq_mtx);
         IF_AFDATA_DESTROY(ifp);
         splx(s);
 +       if_delgroup(ifp, IFG_ALL);
  }
  
  /*
 
 -- 
 Mikolaj Golub