diff --git a/changes/bug5084 b/changes/bug5084 new file mode 100644 index 0000000000..bf175c63ad --- /dev/null +++ b/changes/bug5084 @@ -0,0 +1,4 @@ + o Major bugfixes: + - Avoid a crash when managed proxies are configured and we receive + HUP signals or configuration values too rapidly. Fixes bug 5084; + bugfix on 0.2.3.6-alpha. diff --git a/src/or/transports.c b/src/or/transports.c index 3d8e11dd66..15d96ca99b 100644 --- a/src/or/transports.c +++ b/src/or/transports.c @@ -140,12 +140,35 @@ static INLINE void free_execve_args(char **arg); static smartlist_t *managed_proxy_list = NULL; /** Number of still unconfigured proxies. */ static int unconfigured_proxies_n = 0; +/** Boolean: True iff we might need to restart some proxies. */ +static int check_if_restarts_needed = 0; -/** Return true if there are still unconfigured managed proxies. */ +/** Return true if there are still unconfigured managed proxies, or proxies + * that need restarting. */ int pt_proxies_configuration_pending(void) { - return !! unconfigured_proxies_n; + return unconfigured_proxies_n || check_if_restarts_needed; +} + +/** Assert that the unconfigured_proxies_n value correctly matches the number + * of proxies in a state other than PT_PROTO_COMPLETE. */ +static void +assert_unconfigured_count_ok(void) +{ + int n_completed = 0; + if (!managed_proxy_list) { + tor_assert(unconfigured_proxies_n == 0); + return; + } + + SMARTLIST_FOREACH(managed_proxy_list, managed_proxy_t *, mp, { + if (mp->conf_state == PT_PROTO_COMPLETED) + ++n_completed; + }); + + tor_assert(n_completed + unconfigured_proxies_n == + smartlist_len(managed_proxy_list)); } /** Return true if mp has the same argv as proxy_argv */ @@ -255,6 +278,7 @@ proxy_prepare_for_restart(managed_proxy_t *mp) /* flag it as an infant proxy so that it gets launched on next tick */ mp->conf_state = PT_PROTO_INFANT; + unconfigured_proxies_n++; } /** Launch managed proxy mp. */ @@ -316,26 +340,32 @@ launch_managed_proxy(managed_proxy_t *mp) void pt_configure_remaining_proxies(void) { + smartlist_t *tmp = smartlist_new(); + log_debug(LD_CONFIG, "Configuring remaining managed proxies (%d)!", unconfigured_proxies_n); - SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { + + /* Iterate over tmp, not managed_proxy_list, since configure_proxy can + * remove elements from managed_proxy_list. */ + smartlist_add_all(tmp, managed_proxy_list); + + assert_unconfigured_count_ok(); + + SMARTLIST_FOREACH_BEGIN(tmp, managed_proxy_t *, mp) { tor_assert(mp->conf_state != PT_PROTO_BROKEN || mp->conf_state != PT_PROTO_FAILED_LAUNCH); if (mp->got_hup) { mp->got_hup = 0; - /* This proxy is marked by a SIGHUP. Check whether we need to - restart it. */ + /* This proxy is marked by a SIGHUP. Check whether we need to + restart it. */ if (proxy_needs_restart(mp)) { log_info(LD_GENERAL, "Preparing managed proxy for restart."); proxy_prepare_for_restart(mp); - continue; } else { /* it doesn't need to be restarted. */ log_info(LD_GENERAL, "Nothing changed for managed proxy after HUP: " "not restarting."); - unconfigured_proxies_n--; - tor_assert(unconfigured_proxies_n >= 0); } continue; @@ -347,6 +377,10 @@ pt_configure_remaining_proxies(void) configure_proxy(mp); } SMARTLIST_FOREACH_END(mp); + + smartlist_free(tmp); + check_if_restarts_needed = 0; + assert_unconfigured_count_ok(); } #ifdef _WIN32 @@ -1125,6 +1159,8 @@ managed_proxy_create(const smartlist_t *transport_list, smartlist_add(managed_proxy_list, mp); unconfigured_proxies_n++; + assert_unconfigured_count_ok(); + return mp; } @@ -1154,7 +1190,7 @@ pt_kickstart_proxy(const smartlist_t *transport_list, it. */ if (mp->marked_for_removal) { mp->marked_for_removal = 0; - unconfigured_proxies_n++; + check_if_restarts_needed = 1; } SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport) { @@ -1193,9 +1229,11 @@ pt_prepare_proxy_list_for_config_read(void) if (!managed_proxy_list) return; + assert_unconfigured_count_ok(); SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { /* Destroy unconfigured proxies. */ if (mp->conf_state != PT_PROTO_COMPLETED) { + SMARTLIST_DEL_CURRENT(managed_proxy_list, mp); managed_proxy_destroy(mp, 1); unconfigured_proxies_n--; continue; @@ -1209,6 +1247,8 @@ pt_prepare_proxy_list_for_config_read(void) smartlist_clear(mp->transports_to_launch); } SMARTLIST_FOREACH_END(mp); + assert_unconfigured_count_ok(); + tor_assert(unconfigured_proxies_n == 0); } @@ -1221,13 +1261,14 @@ sweep_proxy_list(void) { if (!managed_proxy_list) return; - + assert_unconfigured_count_ok(); SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) { if (mp->marked_for_removal) { SMARTLIST_DEL_CURRENT(managed_proxy_list, mp); managed_proxy_destroy(mp, 1); } } SMARTLIST_FOREACH_END(mp); + assert_unconfigured_count_ok(); } /** Release all storage held by the pluggable transports subsystem. */ @@ -1239,8 +1280,10 @@ pt_free_all(void) transports and it's the duty of the circuitbuild.c subsystem to free them. Otherwise, it hasn't registered its transports yet and we should free them here. */ - SMARTLIST_FOREACH(managed_proxy_list, managed_proxy_t *, mp, - managed_proxy_destroy(mp, 1)); + SMARTLIST_FOREACH(managed_proxy_list, managed_proxy_t *, mp, { + SMARTLIST_DEL_CURRENT(managed_proxy_list, mp); + managed_proxy_destroy(mp, 1); + }); smartlist_free(managed_proxy_list); managed_proxy_list=NULL;