On 23.10.2018 07:17, Kristof Provost wrote:
> I think I understand what’s happening here.
>
> With this patch I see a different panic:
>
> |diff --git a/sys/net/if_ipsec.c b/sys/net/if_ipsec.c index
> 91b11a455b3..146cb59aaaa 100644 --- a/sys/net/if_ipsec.c +++
> b/sys/net/if_ipsec.c @@ -134,6 +134,9 @@ ipsec_srchash(const struct
> sockaddr *sa) { uint32_t hval; + KASSERT(V_ipsec4_srchtbl, ("NULL")); +
> KASSERT(V_ipsec6_srchtbl, ("NULL (v6)")); + switch (sa->sa_family) {
> #ifdef INET case AF_INET: @@ -265,17 +274,22 @@ static void
> vnet_ipsec_uninit(const void *unused __unused) {
> if_clone_detach(V_ipsec_cloner); free(V_ipsec_idhtbl, M_IPSEC); #ifdef
> INET if (IS_DEFAULT_VNET(curvnet))
> ip_encap_unregister_srcaddr(ipsec4_srctab); free(V_ipsec4_srchtbl,
> M_IPSEC); + V_ipsec4_srchtbl = NULL; + #endif #ifdef INET6 if
> (IS_DEFAULT_VNET(curvnet)) ip6_encap_unregister_srcaddr(ipsec6_srctab);
> free(V_ipsec6_srchtbl, M_IPSEC); + V_ipsec4_srchtbl = NULL; #endif }
> VNET_SYSUNINIT(vnet_ipsec_uninit, SI_SUB_PROTO_IFATTACHDOMAIN,
> SI_ORDER_ANY, |
>
> That trips the KASSERT() in ipsec_srchash(). Basically, the problem is
> that the V_ipsec4_srchtbl table gets freed in vnet_ipsec_uninit(), and
> later we get a srcaddr_change_event(), which tries to iterate the list
> (in ipsec_srcaddr()). Obviously iterating a freed list doesn’t go well
> for us (hence the 0xdeadc0dedeadc0de softc).
>
> That also explains why this happens during the pf tests, even though
> they don’t actually use IPSec.
>
> I’m not quite sure how to best fix this though. I suppose we could set
> V_ipsec4_srchtbl to NULL (well, copy the pointer first, set it to NULL
> and then free it so we don’t add a race condition) and then check for
> NULL in ipsec_srcaddr().
> It feels like the srcaddr_change_event needs to be per-vnet, so we can
> unregister before we free V_ipsec4_srchtbl.I think the better fix would be adding IPSEC_WAIT() to vnet_ipsec_uninit() before doing free(). I'll try your test scenario and try to fix it. Thanks! -- WBR, Andrey V. Elsukov
signature.asc
Description: OpenPGP digital signature
