Conf: Fixes bug in symbol lookup during reconfiguration

Symbol lookup by cf_find_symbol() not only did the lookup but also added
new void symbols allocated from cfg_mem linpool, which gets broken when
lookups are done outside of config parsing, which may lead to crashes
during reconfiguration.

The patch separates lookup-only cf_find_symbol() and config-modifying
cf_get_symbol(), while the later is called only during parsing. Also
new_config and cfg_mem global variables are NULLed outside of parsing.

1 files changed, 39 insertions, 26 deletions

diff --git a/conf/conf.c b/conf/conf.c
index a907402d..825a8e9f 100644
--- a/conf/conf.c
+++ b/conf/conf.c
@@ -20,19 +20,19 @@
  *
  * There can exist up to four different configurations at one time: an active
  * one (pointed to by @config), configuration we are just switching from
- * (@old_config), one queued for the next reconfiguration (@future_config;
- * if there is one and the user wants to reconfigure once again, we just
- * free the previous queued config and replace it with the new one) and
- * finally a config being parsed (@new_config). The stored @old_config 
- * is also used for undo reconfiguration, which works in a similar way.
- * Reconfiguration could also have timeout (using @config_timer) and undo
- * is automatically called if the new configuration is not confirmed later.
+ * (@old_config), one queued for the next reconfiguration (@future_config; if
+ * there is one and the user wants to reconfigure once again, we just free the
+ * previous queued config and replace it with the new one) and finally a config
+ * being parsed (@new_config). The stored @old_config is also used for undo
+ * reconfiguration, which works in a similar way. Reconfiguration could also
+ * have timeout (using @config_timer) and undo is automatically called if the
+ * new configuration is not confirmed later. The new config (@new_config) and
+ * associated linear pool (@cfg_mem) is non-NULL only during parsing.
  *
- * Loading of new configuration is very simple: just call config_alloc()
- * to get a new &config structure, then use config_parse() to parse a
- * configuration file and fill all fields of the structure
- * and finally ask the config manager to switch to the new
- * config by calling config_commit().
+ * Loading of new configuration is very simple: just call config_alloc() to get
+ * a new &config structure, then use config_parse() to parse a configuration
+ * file and fill all fields of the structure and finally ask the config manager
+ * to switch to the new config by calling config_commit().
  *
  * CLI commands are parsed in a very similar way -- there is also a stripped-down
  * &config structure associated with them and they are lex-ed and parsed by the
@@ -91,10 +91,15 @@ config_alloc(byte *name)
   linpool *l = lp_new(p, 4080);
   struct config *c = lp_allocz(l, sizeof(struct config));
 
+  /* Duplication of name string in local linear pool */
+  uint nlen = strlen(name) + 1;
+  char *ndup = lp_allocu(l, nlen);
+  memcpy(ndup, name, nlen);
+
   c->mrtdump_file = -1; /* Hack, this should be sysdep-specific */
   c->pool = p;
-  cfg_mem = c->mem = l;
-  c->file_name = cfg_strdup(name);
+  c->mem = l;
+  c->file_name = ndup;
   c->load_time = now;
   c->tf_route = c->tf_proto = (struct timeformat){"%T", "%F", 20*3600};
   c->tf_base = c->tf_log = (struct timeformat){"%F %T", NULL, 0};
@@ -119,11 +124,13 @@ config_alloc(byte *name)
 int
 config_parse(struct config *c)
 {
+  int done = 0;
   DBG("Parsing configuration file `%s'\n", c->file_name);
   new_config = c;
   cfg_mem = c->mem;
   if (setjmp(conf_jmpbuf))
-    return 0;
+    goto cleanup;
+
   cf_lex_init(0, c);
   sysdep_preconfig(c);
   protos_preconfig(c);
@@ -137,7 +144,12 @@ config_parse(struct config *c)
   if (!c->router_id)
     cf_error("Router ID must be configured manually on IPv6 routers");
 #endif
-  return 1;
+  done = 1;
+
+cleanup:
+  new_config = NULL;
+  cfg_mem = NULL;
+  return done;
 }
 
 /**
@@ -150,14 +162,22 @@ config_parse(struct config *c)
 int
 cli_parse(struct config *c)
 {
-  new_config = c;
+  int done = 0;
   c->sym_fallback = config->sym_hash;
+  new_config = c;
   cfg_mem = c->mem;
   if (setjmp(conf_jmpbuf))
-    return 0;
+    goto cleanup;
+
   cf_lex_init(1, c);
   cf_parse();
-  return 1;
+  done = 1;
+
+cleanup:
+  c->sym_fallback = NULL;
+  new_config = NULL;
+  cfg_mem = NULL;
+  return done;
 }
 
 /**
@@ -237,10 +257,6 @@ config_do_commit(struct config *c, int type)
   if (old_config && !config->shutdown)
     log(L_INFO "Reconfiguring");
 
-  /* This should not be necessary, but it seems there are some
-     functions that access new_config instead of config */
-  new_config = config;
-
   if (old_config)
     old_config->obstacle_count++;
 
@@ -254,9 +270,6 @@ config_do_commit(struct config *c, int type)
   DBG("protos_commit\n");
   protos_commit(c, old_config, force_restart, type);
 
-  /* Just to be sure nobody uses that now */
-  new_config = NULL;
-
   int obs = 0;
   if (old_config)
     obs = --old_config->obstacle_count;