commit c4cb969a2a90570120f57f72448241af41e21e97 Author: Nick Mathewson nickm@torproject.org Date: Wed Sep 6 16:50:05 2017 -0400

Taboo the get_options() function while options are validating

When option validation or transition is happening, there are no "current options" -- only "old options" and "maybe new options". Looking at get_options() is likely a mistake, so have a nonfatal assertion let us know if we do that.

Closes 22281. --- changes/ticket22281 | 3 +++ src/or/config.c | 32 +++++++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/changes/ticket22281 b/changes/ticket22281 new file mode 100644 index 000000000..95787580f --- /dev/null +++ b/changes/ticket22281 @@ -0,0 +1,3 @@ + o Minor features (bug detection): + - Log a warning message, with stack trace, for any attempt to call + get_options() during option validation. Closes ticket 22281. diff --git a/src/or/config.c b/src/or/config.c index eb89d6f5e..2c507ab9d 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -766,6 +766,9 @@ static int have_parsed_cmdline = 0; static char *global_dirfrontpagecontents = NULL; /** List of port_cfg_t for all configured ports. */ static smartlist_t *configured_ports = NULL; +/** True iff we're currently validating options, and any calls to + * get_options() are likely to be bugs. */ +static int in_option_validation = 0;

/** Return the contents of our frontpage string, or NULL if not configured. */ MOCK_IMPL(const char*, @@ -779,6 +782,7 @@ MOCK_IMPL(or_options_t *, get_options_mutable, (void)) { tor_assert(global_options); + tor_assert_nonfatal(! in_option_validation); return global_options; }

@@ -2304,24 +2308,35 @@ options_trial_assign(config_line_t *list, unsigned flags, char **msg) return r; }

+ setopt_err_t rv; + + in_option_validation = 1; + if (options_validate(get_options_mutable(), trial_options, global_default_options, 1, msg) < 0) { or_options_free(trial_options); - return SETOPT_ERR_PARSE; /*XXX make this a separate return value. */ + rv = SETOPT_ERR_PARSE; /*XXX make this a separate return value. */ + goto done; }

if (options_transition_allowed(get_options(), trial_options, msg) < 0) { or_options_free(trial_options); - return SETOPT_ERR_TRANSITION; + rv = SETOPT_ERR_TRANSITION; + goto done; } + in_option_validation = 0;

if (set_options(trial_options, msg)<0) { or_options_free(trial_options); - return SETOPT_ERR_SETTING; + rv = SETOPT_ERR_SETTING; + goto done; }

/* we liked it. put it in place. */ - return SETOPT_OK; + rv = SETOPT_OK; + done: + in_option_validation = 0; + return rv; }

/** Print a usage message for tor. */ @@ -2824,8 +2839,11 @@ static int options_validate_cb(void *old_options, void *options, void *default_options, int from_setconf, char **msg) { - return options_validate(old_options, options, default_options, + in_option_validation = 1; + int rv = options_validate(old_options, options, default_options, from_setconf, msg); + in_option_validation = 0; + return rv; }

#define REJECT(arg) \ @@ -5200,6 +5218,7 @@ options_init_from_string(const char *cf_defaults, const char *cf, }

newoptions->IncludeUsed = cf_has_include; + in_option_validation = 1;

/* Validate newoptions */ if (options_validate(oldoptions, newoptions, newdefaultoptions, @@ -5212,17 +5231,20 @@ options_init_from_string(const char *cf_defaults, const char *cf, err = SETOPT_ERR_TRANSITION; goto err; } + in_option_validation = 0;

if (set_options(newoptions, msg)) { err = SETOPT_ERR_SETTING; goto err; /* frees and replaces old options */ } + or_options_free(global_default_options); global_default_options = newdefaultoptions;

return SETOPT_OK;

err: + in_option_validation = 0; or_options_free(newoptions); or_options_free(newdefaultoptions); if (*msg) {