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?
--
: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;
