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

[Max Laier]

On Friday 10 April 2009 13:50:03 Mikolaj Golub wrote:
> 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);
>   }
>
>   /*

Good catch!  Thank you very much.  I'll commit your fix shortly after some 
testing and will see that we can get it into 7.2

-- 
/"\  Best regards,                      | mlaier at freebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News