On Sun, Mar 31, 2019 at 06:03:01PM +0200, Claudio Jeker wrote: > On Fri, Mar 22, 2019 at 09:25:32PM +0100, Denis Fondras wrote: > > (better when the right diff is sent...) > > > > ROV has been broken since the configuration reload changes. > > > > Index: rde.c > > =================================================================== > > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v > > retrieving revision 1.466 > > diff -u -p -r1.466 rde.c > > --- rde.c 13 Mar 2019 14:35:39 -0000 1.466 > > +++ rde.c 22 Mar 2019 15:36:41 -0000 > > @@ -2899,7 +2899,7 @@ rde_reload_done(void) > > roa_old = conf->rde_roa; > > as_sets_old = conf->as_sets; > > > > - copy_config(conf, nconf); > > + memcpy(conf, nconf, sizeof(struct bgpd_config)); > > SIMPLEQ_INIT(&conf->rde_prefixsets); > > SIMPLEQ_INIT(&conf->rde_originsets); > > SIMPLEQ_CONCAT(&conf->rde_prefixsets, &nconf->rde_prefixsets); > > > > Here is a diff that should work better. Your version introduces a use > after free because of nconf->as_sets being freed but copied to conf > beforehands. > > This handles both, as_sets and rde_roa. OK?
OK @denis > -- > :wq Claudio > > Index: rde.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v > retrieving revision 1.467 > diff -u -p -r1.467 rde.c > --- rde.c 23 Mar 2019 13:09:56 -0000 1.467 > +++ rde.c 31 Mar 2019 13:52:01 -0000 > @@ -2899,11 +2899,15 @@ rde_reload_done(void) > roa_old = conf->rde_roa; > as_sets_old = conf->as_sets; > > - memcpy(conf, nconf, sizeof(struct bgpd_config)); > - SIMPLEQ_INIT(&conf->rde_prefixsets); > - SIMPLEQ_INIT(&conf->rde_originsets); > + copy_config(conf, nconf); > + /* need to copy the sets and roa table and clear them in nconf */ > SIMPLEQ_CONCAT(&conf->rde_prefixsets, &nconf->rde_prefixsets); > SIMPLEQ_CONCAT(&conf->rde_originsets, &nconf->rde_originsets); > + conf->rde_roa = nconf->rde_roa; > + conf->as_sets = nconf->as_sets; > + memset(&nconf->rde_roa, 0, sizeof(nconf->rde_roa)); > + nconf->as_sets = NULL; > + > free_config(nconf); > nconf = NULL; >
