commit 3a3e88dbd4296c6bea8a324a0fa8ccd4628a79f5 Author: Nick Mathewson <nickm@torproject.org> Date: Thu Dec 15 08:12:59 2016 -0500 Fix memory leak when failing to configure hidden services. In 8a0ea3ee43da0063c2546092662fa7ce4900bc2c we added a temp_service_list local variable to rend_config_services, but we didn't add a corresponding "free" for it to all of the exit paths. Fixes bug 20987; bugfix on 0.3.0.1-alpha. --- changes/bug20987 | 3 +++ src/or/rendservice.c | 62 ++++++++++++++++++++++++---------------------------- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/changes/bug20987 b/changes/bug20987 new file mode 100644 index 0000000..9922d73 --- /dev/null +++ b/changes/bug20987 @@ -0,0 +1,3 @@ + o Minor bugfixes (memory leaks): + - Fix a memory leak when configuring hidden services. Fixes bug 20987; + bugfix on 0.3.0.1-alpha. diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 00ede48..e0d163d 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -556,6 +556,7 @@ rend_config_services(const or_options_t *options, int validate_only) smartlist_t *old_service_list = NULL; smartlist_t *temp_service_list = NULL; int ok = 0; + int rv = -1; /* Use a temporary service list, so that we can check the new services' * consistency with each other */ @@ -568,7 +569,8 @@ rend_config_services(const or_options_t *options, int validate_only) * which is registered below the loop */ if (rend_service_check_dir_and_add(temp_service_list, options, service, validate_only) < 0) { - return -1; + service = NULL; + goto free_and_return; } service = tor_malloc_zero(sizeof(rend_service_t)); service->directory = tor_strdup(line->value); @@ -580,8 +582,7 @@ rend_config_services(const or_options_t *options, int validate_only) if (!service) { log_warn(LD_CONFIG, "%s with no preceding HiddenServiceDir directive", line->key); - rend_service_free(service); - return -1; + goto free_and_return; } if (!strcasecmp(line->key, "HiddenServicePort")) { char *err_msg = NULL; @@ -590,8 +591,7 @@ rend_config_services(const or_options_t *options, int validate_only) if (err_msg) log_warn(LD_CONFIG, "%s", err_msg); tor_free(err_msg); - rend_service_free(service); - return -1; + goto free_and_return; } tor_assert(!err_msg); smartlist_add(service->ports, portcfg); @@ -602,8 +602,8 @@ rend_config_services(const or_options_t *options, int validate_only) log_warn(LD_CONFIG, "HiddenServiceAllowUnknownPorts should be 0 or 1, not %s", line->value); - rend_service_free(service); - return -1; + smartlist_free(temp_service_list); + goto free_and_return; } log_info(LD_CONFIG, "HiddenServiceAllowUnknownPorts=%d for %s", @@ -617,8 +617,7 @@ rend_config_services(const or_options_t *options, int validate_only) log_warn(LD_CONFIG, "HiddenServiceDirGroupReadable should be 0 or 1, not %s", line->value); - rend_service_free(service); - return -1; + goto free_and_return; } log_info(LD_CONFIG, "HiddenServiceDirGroupReadable=%d for %s", @@ -631,8 +630,7 @@ rend_config_services(const or_options_t *options, int validate_only) log_warn(LD_CONFIG, "HiddenServiceMaxStreams should be between 0 and %d, not %s", 65535, line->value); - rend_service_free(service); - return -1; + goto free_and_return; } log_info(LD_CONFIG, "HiddenServiceMaxStreams=%d for %s", @@ -646,8 +644,7 @@ rend_config_services(const or_options_t *options, int validate_only) "HiddenServiceMaxStreamsCloseCircuit should be 0 or 1, " "not %s", line->value); - rend_service_free(service); - return -1; + goto free_and_return; } log_info(LD_CONFIG, "HiddenServiceMaxStreamsCloseCircuit=%d for %s", @@ -664,8 +661,7 @@ rend_config_services(const or_options_t *options, int validate_only) "should be between %d and %d, not %s", NUM_INTRO_POINTS_DEFAULT, NUM_INTRO_POINTS_MAX, line->value); - rend_service_free(service); - return -1; + goto free_and_return; } log_info(LD_CONFIG, "HiddenServiceNumIntroductionPoints=%d for %s", service->n_intro_points_wanted, @@ -680,8 +676,7 @@ rend_config_services(const or_options_t *options, int validate_only) if (service->auth_type != REND_NO_AUTH) { log_warn(LD_CONFIG, "Got multiple HiddenServiceAuthorizeClient " "lines for a single service."); - rend_service_free(service); - return -1; + goto free_and_return; } type_names_split = smartlist_new(); smartlist_split_string(type_names_split, line->value, " ", 0, 2); @@ -689,9 +684,7 @@ rend_config_services(const or_options_t *options, int validate_only) log_warn(LD_BUG, "HiddenServiceAuthorizeClient has no value. This " "should have been prevented when parsing the " "configuration."); - smartlist_free(type_names_split); - rend_service_free(service); - return -1; + goto free_and_return; } authname = smartlist_get(type_names_split, 0); if (!strcasecmp(authname, "basic")) { @@ -705,8 +698,7 @@ rend_config_services(const or_options_t *options, int validate_only) (char *) smartlist_get(type_names_split, 0)); SMARTLIST_FOREACH(type_names_split, char *, cp, tor_free(cp)); smartlist_free(type_names_split); - rend_service_free(service); - return -1; + goto free_and_return; } service->clients = smartlist_new(); if (smartlist_len(type_names_split) < 2) { @@ -743,8 +735,7 @@ rend_config_services(const or_options_t *options, int validate_only) client_name, REND_CLIENTNAME_MAX_LEN); SMARTLIST_FOREACH(clients, char *, cp, tor_free(cp)); smartlist_free(clients); - rend_service_free(service); - return -1; + goto free_and_return; } client = tor_malloc_zero(sizeof(rend_authorized_client_t)); client->client_name = tor_strdup(client_name); @@ -766,16 +757,14 @@ rend_config_services(const or_options_t *options, int validate_only) smartlist_len(service->clients), service->auth_type == REND_BASIC_AUTH ? 512 : 16, service->auth_type == REND_BASIC_AUTH ? "basic" : "stealth"); - rend_service_free(service); - return -1; + goto free_and_return; } } else { tor_assert(!strcasecmp(line->key, "HiddenServiceVersion")); if (strcmp(line->value, "2")) { log_warn(LD_CONFIG, "The only supported HiddenServiceVersion is 2."); - rend_service_free(service); - return -1; + goto free_and_return; } } } @@ -784,16 +773,15 @@ rend_config_services(const or_options_t *options, int validate_only) * within the loop. It is ok for this service to be NULL, it is ignored. */ if (rend_service_check_dir_and_add(temp_service_list, options, service, validate_only) < 0) { - return -1; + service = NULL; + goto free_and_return; } + service = NULL; /* Free the newly added services if validating */ if (validate_only) { - SMARTLIST_FOREACH(temp_service_list, rend_service_t *, ptr, - rend_service_free(ptr)); - smartlist_free(temp_service_list); - temp_service_list = NULL; - return 0; + rv = 0; + goto free_and_return; } /* Otherwise, use the newly added services as the new service list @@ -889,6 +877,12 @@ rend_config_services(const or_options_t *options, int validate_only) } return 0; + free_and_return: + rend_service_free(service); + SMARTLIST_FOREACH(temp_service_list, rend_service_t *, ptr, + rend_service_free(ptr)); + smartlist_free(temp_service_list); + return rv; } /** Add the ephemeral service <b>pk</b>/<b>ports</b> if possible, using