On Wed, May 27, 2015 at 10:39 +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> > > - if (pf_state_insert(BOUND_IFACE(r, pd->kif), skw, sks, s)) {
> > > - pf_state_key_detach(s, PF_SK_STACK);
> > > - pf_state_key_detach(s, PF_SK_WIRE);
> >
> > This bug is not yours, but doing two pf_state_key_detach is wrong
> > and results in all kinds of protection fault fireworks. The right
> > thing is to do pf_detach_state that would handle the case where
> > PF_SK_STACK == PF_SK_WIRE (i.e. we need only one invocation).
> good catch.
>
> >
> > In csfailed case we do pf_remove_src_node and then call
> > pf_src_tree_remove_state. With your diff this means that
> > pf_src_tree_remove_state will be dereferencing sni->sn that
> > might be pool_put by the pf_remove_src_node. Therefore their
> > order needs to be reversed.
>
> I see. Another option to fix it would be to do:
>
> sni->sn = sns[i];
> sns[i] = NULL;
>
I'm not sure I follow. Where do you want to do it?
If it's when we pool_get sni's, then this would mean
we won't run pf_remove_src_node and cleanup source
nodes that might have been created.
> since your fix works my comment is kind of bikeshedding.
>
> regards
> sasha
>
