From ac9a179fe980f1a78438a791671e22671ebb6451 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 14 Jun 2018 16:28:59 +0200 Subject: multi: fix memory leak when stopped during name resolve When the application just started the transfer and then stops it while the name resolve in the background thread hasn't completed, we need to wait for the resolve to complete and then cleanup data accordingly. Enabled test 1553 again and added test 1590 to also check when the host name resolves successfully. Detected by OSS-fuzz. Closes #1968 --- lib/asyn-ares.c | 19 +++++++++++-------- lib/asyn-thread.c | 6 ++++-- lib/multi.c | 34 +++++++++++++++++++++++----------- 3 files changed, 38 insertions(+), 21 deletions(-) (limited to 'lib') diff --git a/lib/asyn-ares.c b/lib/asyn-ares.c index aa581a49a..00fe1adb5 100644 --- a/lib/asyn-ares.c +++ b/lib/asyn-ares.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2017, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2018, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -312,22 +312,25 @@ CURLcode Curl_resolver_is_resolved(struct connectdata *conn, conn->async.os_specific; CURLcode result = CURLE_OK; - *dns = NULL; + if(dns) + *dns = NULL; waitperform(conn, 0); if(res && !res->num_pending) { - (void)Curl_addrinfo_callback(conn, res->last_status, res->temp_ai); - /* temp_ai ownership is moved to the connection, so we need not free-up - them */ - res->temp_ai = NULL; + if(dns) { + (void)Curl_addrinfo_callback(conn, res->last_status, res->temp_ai); + /* temp_ai ownership is moved to the connection, so we need not free-up + them */ + res->temp_ai = NULL; + } if(!conn->async.dns) { failf(data, "Could not resolve: %s (%s)", conn->async.hostname, ares_strerror(conn->async.status)); result = conn->bits.proxy?CURLE_COULDNT_RESOLVE_PROXY: CURLE_COULDNT_RESOLVE_HOST; } - else + else if(dns) *dns = conn->async.dns; destroy_async_data(&conn->async); @@ -390,7 +393,7 @@ CURLcode Curl_resolver_wait_resolv(struct connectdata *conn, timeout_ms = 1000; waitperform(conn, timeout_ms); - result = Curl_resolver_is_resolved(conn, &temp_entry); + result = Curl_resolver_is_resolved(conn, entry?&temp_entry:NULL); if(result || conn->async.done) break; diff --git a/lib/asyn-thread.c b/lib/asyn-thread.c index b11fab246..894ca459b 100644 --- a/lib/asyn-thread.c +++ b/lib/asyn-thread.c @@ -481,8 +481,10 @@ CURLcode Curl_resolver_wait_resolv(struct connectdata *conn, DEBUGASSERT(conn && td); /* wait for the thread to resolve the name */ - if(Curl_thread_join(&td->thread_hnd)) - result = getaddrinfo_complete(conn); + if(Curl_thread_join(&td->thread_hnd)) { + if(entry) + result = getaddrinfo_complete(conn); + } else DEBUGASSERT(0); diff --git a/lib/multi.c b/lib/multi.c index 9cad600e2..e30737dd2 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -69,8 +69,8 @@ #define GOOD_MULTI_HANDLE(x) \ ((x) && (x)->type == CURL_MULTI_HANDLE) -static void singlesocket(struct Curl_multi *multi, - struct Curl_easy *data); +static CURLMcode singlesocket(struct Curl_multi *multi, + struct Curl_easy *data); static int update_timer(struct Curl_multi *multi); static CURLMcode add_next_timeout(struct curltime now, @@ -515,6 +515,11 @@ static CURLcode multi_done(struct connectdata **connp, /* Stop if multi_done() has already been called */ return CURLE_OK; + if(data->mstate == CURLM_STATE_WAITRESOLVE) { + /* still waiting for the resolve to complete */ + (void)Curl_resolver_wait_resolv(conn, NULL); + } + Curl_getoff_all_pipelines(data, conn); /* Cleanup possible redirect junk */ @@ -2304,8 +2309,8 @@ CURLMsg *curl_multi_info_read(struct Curl_multi *multi, int *msgs_in_queue) * and if we have a different state in any of those sockets from last time we * call the callback accordingly. */ -static void singlesocket(struct Curl_multi *multi, - struct Curl_easy *data) +static CURLMcode singlesocket(struct Curl_multi *multi, + struct Curl_easy *data) { curl_socket_t socks[MAX_SOCKSPEREASYHANDLE]; int i; @@ -2352,7 +2357,7 @@ static void singlesocket(struct Curl_multi *multi, entry = sh_addentry(&multi->sockhash, s, data); if(!entry) /* fatal */ - return; + return CURLM_OUT_OF_MEMORY; } /* we know (entry != NULL) at this point, see the logic above */ @@ -2440,6 +2445,7 @@ static void singlesocket(struct Curl_multi *multi, memcpy(data->sockets, socks, num*sizeof(curl_socket_t)); data->numsocks = num; + return CURLM_OK; } void Curl_updatesocket(struct Curl_easy *data) @@ -2553,8 +2559,8 @@ static CURLMcode multi_socket(struct Curl_multi *multi, and callbacks */ if(result != CURLM_BAD_HANDLE) { data = multi->easyp; - while(data) { - singlesocket(multi, data); + while(data && !result) { + result = singlesocket(multi, data); data = data->next; } } @@ -2608,10 +2614,13 @@ static CURLMcode multi_socket(struct Curl_multi *multi, /* clear the bitmask only if not locked */ data->easy_conn->cselect_bits = 0; - if(CURLM_OK >= result) + if(CURLM_OK >= result) { /* get the socket(s) and check if the state has been changed since last */ - singlesocket(multi, data); + result = singlesocket(multi, data); + if(result) + return result; + } /* Now we fall-through and do the timer-based stuff, since we don't want to force the user to have to deal with timeouts as long as at least @@ -2645,10 +2654,13 @@ static CURLMcode multi_socket(struct Curl_multi *multi, result = multi_runsingle(multi, now, data); sigpipe_restore(&pipe_st); - if(CURLM_OK >= result) + if(CURLM_OK >= result) { /* get the socket(s) and check if the state has been changed since last */ - singlesocket(multi, data); + result = singlesocket(multi, data); + if(result) + return result; + } } /* Check if there's one (more) expired timer to deal with! This function -- cgit v1.2.3