commit dde091ebc76d8ae5d93c1b3e1febfa2c9d1459fa Author: Nick Mathewson <nickm@torproject.org> Date: Tue Jul 23 09:08:31 2019 -0400 Add a "freeze" function for config_mgr_t objects. It's important to make sure that we don't change a config_mgr_t after we start using it to make objects, or we could get into inconsistent states. This feature is the start of a safety mechanism to prevent this problem. --- src/app/config/config.c | 1 + src/app/config/confparse.c | 20 ++++++++++++++++++++ src/app/config/confparse.h | 1 + src/app/config/statefile.c | 1 + src/feature/dirauth/shared_random_state.c | 1 + src/test/test_confparse.c | 15 +++++++++++++++ 6 files changed, 39 insertions(+) diff --git a/src/app/config/config.c b/src/app/config/config.c index 246f52a64..d8b33aee7 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -904,6 +904,7 @@ get_options_mgr(void) { if (PREDICT_UNLIKELY(options_mgr == NULL)) { options_mgr = config_mgr_new(&options_format); + config_mgr_freeze(options_mgr); } return options_mgr; } diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c index 9d4226307..b086f8c94 100644 --- a/src/app/config/confparse.c +++ b/src/app/config/confparse.c @@ -85,6 +85,10 @@ struct config_mgr_t { smartlist_t *all_abbrevs; /** A smartlist of config_deprecation_t for all configuration formats. */ smartlist_t *all_deprecations; + /** True if this manager has been frozen and cannot have any more formats + * added to it. A manager must be frozen before it can be used to construct + * or manipulate objects. */ + bool frozen; }; #define IDX_TOPLEVEL (-1) @@ -112,6 +116,10 @@ config_mgr_register_fmt(config_mgr_t *mgr, { int i; + tor_assertf(!mgr->frozen, + "Tried to add a format to a configuration manager after " + "it had been frozen."); + /* register variables */ for (i = 0; fmt->vars[i].member.name; ++i) { managed_var_t *mv = tor_malloc_zero(sizeof(managed_var_t)); @@ -161,6 +169,16 @@ config_mgr_get_obj(const config_mgr_t *mgr, const void *toplevel, int idx) return config_mgr_get_obj_mutable(mgr, (void*)toplevel, idx); } +/** + * Mark a configuration manager as "frozen", so that no more formats can be + * added, and so that it can be used for manipulating configuration objects. + **/ +void +config_mgr_freeze(config_mgr_t *mgr) +{ + mgr->frozen = true; +} + /** Release all storage held in <b>mgr</b> */ void config_mgr_free_(config_mgr_t *mgr) @@ -208,6 +226,7 @@ config_mgr_assert_magic_ok(const config_mgr_t *mgr, { tor_assert(mgr); tor_assert(options); + tor_assert(mgr->frozen); struct_check_magic(options, &mgr->toplevel->magic); } @@ -221,6 +240,7 @@ config_mgr_assert_magic_ok(const config_mgr_t *mgr, void * config_new(const config_mgr_t *mgr) { + tor_assert(mgr->frozen); const config_format_t *fmt = mgr->toplevel; void *opts = tor_malloc_zero(fmt->size); struct_set_magic(opts, &fmt->magic); diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h index edc7fa377..83e886696 100644 --- a/src/app/config/confparse.h +++ b/src/app/config/confparse.h @@ -73,6 +73,7 @@ typedef struct config_mgr_t config_mgr_t; config_mgr_t *config_mgr_new(const config_format_t *toplevel_fmt); void config_mgr_free_(config_mgr_t *mgr); +void config_mgr_freeze(config_mgr_t *mgr); #define config_mgr_free(mgr) \ FREE_AND_NULL(config_mgr_t, config_mgr_free_, (mgr)) struct smartlist_t *config_mgr_list_vars(const config_mgr_t *mgr); diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c index a44bcf6fb..93e35659d 100644 --- a/src/app/config/statefile.c +++ b/src/app/config/statefile.c @@ -183,6 +183,7 @@ get_state_mgr(void) { if (PREDICT_UNLIKELY(state_mgr == NULL)) { state_mgr = config_mgr_new(&state_format); + config_mgr_freeze(state_mgr); } return state_mgr; } diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c index a552e621c..12ae79ce3 100644 --- a/src/feature/dirauth/shared_random_state.c +++ b/src/feature/dirauth/shared_random_state.c @@ -112,6 +112,7 @@ get_srs_mgr(void) { if (PREDICT_UNLIKELY(shared_random_state_mgr == NULL)) { shared_random_state_mgr = config_mgr_new(&state_format); + config_mgr_freeze(shared_random_state_mgr); } return shared_random_state_mgr; } diff --git a/src/test/test_confparse.c b/src/test/test_confparse.c index 8a921531f..716384e2f 100644 --- a/src/test/test_confparse.c +++ b/src/test/test_confparse.c @@ -153,6 +153,7 @@ test_confparse_init(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = config_new(mgr); config_init(mgr, tst); @@ -239,6 +240,7 @@ test_confparse_assign_simple(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); tt_str_op(tst->s, OP_EQ, "this is a"); @@ -300,6 +302,7 @@ test_confparse_assign_obsolete(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_line_t *lines = NULL; char *msg = NULL; @@ -330,6 +333,7 @@ test_confparse_assign_deprecated(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_line_t *lines = NULL; char *msg = NULL; @@ -364,6 +368,7 @@ test_confparse_assign_replaced(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_line_t *lines = NULL; char *msg = NULL; @@ -395,6 +400,7 @@ test_confparse_assign_emptystring(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_line_t *lines = NULL; char *msg = NULL; @@ -423,6 +429,7 @@ test_confparse_assign_twice(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_line_t *lines = NULL; char *msg = NULL; @@ -458,6 +465,7 @@ test_confparse_assign_badval(void *arg) { const badval_test_t *bt = arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_line_t *lines = NULL; char *msg = NULL; @@ -512,6 +520,7 @@ test_confparse_dump(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); char *dumped = NULL; @@ -603,6 +612,7 @@ test_confparse_reset(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_reset_line(mgr, tst, "interval", 0); @@ -623,6 +633,7 @@ test_confparse_reassign(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_line_t *lines = NULL; char *msg = NULL, *rs = NULL; @@ -679,6 +690,7 @@ test_confparse_reassign_extend(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_line_t *lines = NULL; char *msg = NULL; @@ -738,6 +750,7 @@ test_confparse_get_assigned(void *arg) (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = get_simple_config(mgr); config_line_t *lines = NULL; @@ -824,6 +837,7 @@ test_confparse_extra_lines(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&etest_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = config_new(mgr); config_line_t *lines = NULL; char *msg = NULL, *dump = NULL; @@ -919,6 +933,7 @@ test_confparse_check_ok_fail(void *arg) { (void)arg; config_mgr_t *mgr = config_mgr_new(&test_fmt); + config_mgr_freeze(mgr); test_struct_t *tst = config_new(mgr); tst->pos = -10; tt_assert(! config_check_ok(mgr, tst, LOG_INFO));