commit 006ce47ffa0dbc60dc0e3fc5d58e24943e291ef9 Author: Nick Mathewson <nickm@torproject.org> Date: Thu Nov 7 12:47:20 2019 -0500 Extract a function for one-time-only pre-reversible options. These changes _only_ happen at startup, and happen before _any_ reversible option change is set. --- src/app/config/config.c | 109 ++++++++++++++++++++++++++++-------------------- 1 file changed, 64 insertions(+), 45 deletions(-) diff --git a/src/app/config/config.c b/src/app/config/config.c index ff9bf833f..57835f95d 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -904,8 +904,8 @@ 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; -/* True iff we've initialized libevent */ -static int libevent_initialized = 0; +/** True iff we have run options_act_once_on_startup() */ +static bool have_set_startup_options = false; /* A global configuration manager to handle all configuration objects. */ static config_mgr_t *options_mgr = NULL; @@ -1085,7 +1085,7 @@ config_free_all(void) cleanup_protocol_warning_severity_level(); - libevent_initialized = 0; + have_set_startup_options = false; config_mgr_free(options_mgr); } @@ -1422,27 +1422,24 @@ create_keys_directory(const or_options_t *options) /* Helps determine flags to pass to switch_id. */ static int have_low_ports = -1; -/** Fetch the active option list, and take actions based on it. All of the - * things we do should survive being done repeatedly. If present, - * <b>old_options</b> contains the previous value of the options. - * - * Return 0 if all goes well, return -1 if things went badly. - */ -MOCK_IMPL(STATIC int, -options_act_reversible,(const or_options_t *old_options, char **msg)) +/** Take case of initial startup tasks that must occur before any of the + * transactional option-related changes are allowed. */ +static int +options_act_once_on_startup(char **msg_out) { - smartlist_t *new_listeners = smartlist_new(); - or_options_t *options = get_options_mutable(); + if (have_set_startup_options) + return 0; + + const or_options_t *options = get_options(); int running_tor = options->command == CMD_RUN_TOR; - int set_conn_limit = 0; - int r = -1; - int logs_marked = 0, logs_initialized = 0; - int old_min_log_level = get_min_log_level(); + + if (!running_tor) + return 0; /* Daemonize _first_, since we only want to open most of this stuff in * the subprocess. Libevent bases can't be reliably inherited across * processes. */ - if (running_tor && options->RunAsDaemon) { + if (options->RunAsDaemon) { if (! start_daemon_has_been_called()) subsystems_prefork(); /* No need to roll back, since you can't change the value. */ @@ -1455,6 +1452,55 @@ options_act_reversible,(const or_options_t *old_options, char **msg)) sd_notifyf(0, "MAINPID=%ld\n", (long int)getpid()); #endif + /* Set up libevent. (We need to do this before we can register the + * listeners as listeners.) */ + init_libevent(options); + + /* This has to come up after libevent is initialized. */ + control_initialize_event_queue(); + + /* + * Initialize the scheduler - this has to come after + * options_init_from_torrc() sets up libevent - why yes, that seems + * completely sensible to hide the libevent setup in the option parsing + * code! It also needs to happen before init_keys(), so it needs to + * happen here too. How yucky. */ + scheduler_init(); + + /* Attempt to lock all current and future memory with mlockall() only once. + * This must happen before setuid. */ + if (options->DisableAllSwap) { + if (tor_mlockall() == -1) { + *msg_out = tor_strdup("DisableAllSwap failure. Do you have proper " + "permissions?"); + return -1; + } + } + + have_set_startup_options = true; + return 0; +} + +/** Fetch the active option list, and take actions based on it. All of the + * things we do should survive being done repeatedly. If present, + * <b>old_options</b> contains the previous value of the options. + * + * Return 0 if all goes well, return -1 if things went badly. + */ +MOCK_IMPL(STATIC int, +options_act_reversible,(const or_options_t *old_options, char **msg)) +{ + smartlist_t *new_listeners = smartlist_new(); + or_options_t *options = get_options_mutable(); + int running_tor = options->command == CMD_RUN_TOR; + int set_conn_limit = 0; + int r = -1; + int logs_marked = 0, logs_initialized = 0; + int old_min_log_level = get_min_log_level(); + + if (options_act_once_on_startup(msg) < 0) + goto rollback; + if (running_tor) { int n_ports=0; /* We need to set the connection limit before we can open the listeners. */ @@ -1471,24 +1517,6 @@ options_act_reversible,(const or_options_t *old_options, char **msg)) options->ConnLimit_ = old_options->ConnLimit_; } - /* Set up libevent. (We need to do this before we can register the - * listeners as listeners.) */ - if (running_tor && !libevent_initialized) { - init_libevent(options); - libevent_initialized = 1; - - /* This has to come up after libevent is initialized. */ - control_initialize_event_queue(); - - /* - * Initialize the scheduler - this has to come after - * options_init_from_torrc() sets up libevent - why yes, that seems - * completely sensible to hide the libevent setup in the option parsing - * code! It also needs to happen before init_keys(), so it needs to - * happen here too. How yucky. */ - scheduler_init(); - } - /* Adjust the port configuration so we can launch listeners. */ /* 31851: some ports are relay-only */ if (parse_ports(options, 0, msg, &n_ports, NULL)) { @@ -1533,15 +1561,6 @@ options_act_reversible,(const or_options_t *old_options, char **msg)) } #endif /* defined(HAVE_NET_IF_H) && defined(HAVE_NET_PFVAR_H) */ - /* Attempt to lock all current and future memory with mlockall() only once */ - if (options->DisableAllSwap) { - if (tor_mlockall() == -1) { - *msg = tor_strdup("DisableAllSwap failure. Do you have proper " - "permissions?"); - goto done; - } - } - /* Setuid/setgid as appropriate */ if (options->User) { tor_assert(have_low_ports != -1);