Route attributes overwritten during reconfiguration

[Lennert Buytenhek]

On Fri, Feb 17, 2017 at 10:00:05PM +0100, Ondrej Zajicek wrote:

> > > > > I think that the reconfiguration code finds the old and the new implicit
> > > > > filter expression to be the same code, and therefore hangs onto the old
> > > > > version, but then the underlying memory for some of the old objects gets
> > > > > freed.  Or something like that.
> > > > 
> > > > Well, the comparison in static_same_rte() is just for decision whether
> > > > the route should be propagated again to the (main) routing table.
> > > > 
> > > > Internal structures of static protocols (like struct static_route) are
> > > > replaced anyways.
> > > 
> > > I see.  Well, I was hoping to avoid going too deeply into bird internals
> > > (I've already been labeled our resident ornithologist), but I can dig
> > > deeper if the problem doesn't reproduce on your end for some reason.
> > 
> > I could not reproduce the crash, but i can reproduce the valgrind warnings.
> > I will check it.
> 
> Hi

Hello!


> Here is the bugfix. The bug was caused by some arcane rewriting behavior
> in i_same() that works when called with argument order (new, old),
> but static protocol called it as (old, new).
> 
> Note that the bug is specific to using functions in static routes.
> 
> -- 
> Elen sila lumenn' omentielvo
> 
> Ondrej 'Santiago' Zajicek (email: santiago at crfreenet.org)
> OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
> "To err is human -- to blame it on a computer is even more so."

> diff --git a/proto/static/static.c b/proto/static/static.c
> index 0c088cd..849067b 100644
> --- a/proto/static/static.c
> +++ b/proto/static/static.c
> @@ -498,7 +498,8 @@ static_same_dest(struct static_route *x, struct static_route *y)
>  static inline int
>  static_same_rte(struct static_route *x, struct static_route *y)
>  {
> -  return static_same_dest(x, y) && i_same(x->cmds, y->cmds);
> +  /* Note that i_same() requires arguments in (new, old) order */
> +  return static_same_dest(x, y) && i_same(y->cmds, x->cmds);
>  }
>  

I can confirm that this fixes the issue for me.  Many thanks!