From 7f1140c8bfc47dd9b7a8010818e97e87a289bfaf Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Fri, 6 Oct 2017 01:11:17 +0200 Subject: multi_cleanup: call DONE on handles that never got that ... fixes a memory leak with at least IMAP when remove_handle is never called and the transfer is abruptly just abandoned early. Test 1552 added to verify Detected by OSS-fuzz Assisted-by: Max Dymond Closes #1954 --- lib/multi.c | 39 ++++++++++--------- tests/data/Makefile.inc | 2 +- tests/data/test1552 | 52 ++++++++++++++++++++++++++ tests/libtest/Makefile.inc | 5 ++- tests/libtest/lib1552.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 171 insertions(+), 20 deletions(-) create mode 100644 tests/data/test1552 create mode 100644 tests/libtest/lib1552.c diff --git a/lib/multi.c b/lib/multi.c index 70aa6bcf9..faf42f1fa 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -2224,6 +2224,27 @@ CURLMcode curl_multi_cleanup(struct Curl_multi *multi) multi->type = 0; /* not good anymore */ + /* Firsrt remove all remaining easy handles */ + data = multi->easyp; + while(data) { + nextdata = data->next; + if(!data->state.done && data->easy_conn) + /* if DONE was never called for this handle */ + (void)multi_done(&data->easy_conn, CURLE_OK, TRUE); + if(data->dns.hostcachetype == HCACHE_MULTI) { + /* clear out the usage of the shared DNS cache */ + Curl_hostcache_clean(data, data->dns.hostcache); + data->dns.hostcache = NULL; + data->dns.hostcachetype = HCACHE_NONE; + } + + /* Clear the pointer to the connection cache */ + data->state.conn_cache = NULL; + data->multi = NULL; /* clear the association */ + + data = nextdata; + } + /* Close all the connections in the connection cache */ close_all_connections(multi); @@ -2243,24 +2264,6 @@ CURLMcode curl_multi_cleanup(struct Curl_multi *multi) Curl_llist_destroy(&multi->msglist, NULL); Curl_llist_destroy(&multi->pending, NULL); - /* remove all easy handles */ - data = multi->easyp; - while(data) { - nextdata = data->next; - if(data->dns.hostcachetype == HCACHE_MULTI) { - /* clear out the usage of the shared DNS cache */ - Curl_hostcache_clean(data, data->dns.hostcache); - data->dns.hostcache = NULL; - data->dns.hostcachetype = HCACHE_NONE; - } - - /* Clear the pointer to the connection cache */ - data->state.conn_cache = NULL; - data->multi = NULL; /* clear the association */ - - data = nextdata; - } - Curl_hash_destroy(&multi->hostcache); /* Free the blacklists by setting them to NULL */ diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index 4243c2183..e431d46c1 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -169,7 +169,7 @@ test1520 test1521 \ test1525 test1526 test1527 test1528 test1529 test1530 test1531 test1532 \ test1533 test1534 test1535 test1536 test1537 test1538 \ test1540 \ -test1550 test1551 \ +test1550 test1551 test1552 \ test1600 test1601 test1602 test1603 test1604 test1605 test1606 \ \ test1700 test1701 test1702 \ diff --git a/tests/data/test1552 b/tests/data/test1552 new file mode 100644 index 000000000..c5b1b5728 --- /dev/null +++ b/tests/data/test1552 @@ -0,0 +1,52 @@ + + + +IMAP +Clear Text +FETCH + + + +# +# Server-side + + +From: me@somewhere +To: fake@nowhere + +body + +-- + yours sincerely + + + + + + + +# +# Client-side + + +imap + + +IMAP multi transfer error without curl_multi_remove_handle + +# tool is what to use instead of 'curl' + +lib1552 + + +'imap://%HOSTIP:%IMAPPORT/1552/;UID=1' + + + +# +# Verify data after the test has been "shot" + + + + + diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc index 065899276..59e3b281d 100644 --- a/tests/libtest/Makefile.inc +++ b/tests/libtest/Makefile.inc @@ -27,7 +27,7 @@ noinst_PROGRAMS = chkhostname libauthretry libntlmconnect \ lib1525 lib1526 lib1527 lib1528 lib1529 lib1530 lib1531 lib1532 lib1533 \ lib1534 lib1535 lib1536 lib1537 lib1538 \ lib1540 \ - lib1550 lib1551 \ + lib1550 lib1551 lib1552 \ lib1900 \ lib2033 @@ -454,6 +454,9 @@ lib1550_CPPFLAGS = $(AM_CPPFLAGS) -DLIB1517 lib1551_SOURCES = lib1551.c $(SUPPORTFILES) lib1551_CPPFLAGS = $(AM_CPPFLAGS) +lib1552_SOURCES = lib1552.c $(SUPPORTFILES) $(TESTUTIL) +lib1552_CPPFLAGS = $(AM_CPPFLAGS) + lib1900_SOURCES = lib1900.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) lib1900_LDADD = $(TESTUTIL_LIBS) lib1900_CPPFLAGS = $(AM_CPPFLAGS) diff --git a/tests/libtest/lib1552.c b/tests/libtest/lib1552.c new file mode 100644 index 000000000..02c4ea860 --- /dev/null +++ b/tests/libtest/lib1552.c @@ -0,0 +1,93 @@ +/*************************************************************************** + * _ _ ____ _ + * Project ___| | | | _ \| | + * / __| | | | |_) | | + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * + * Copyright (C) 1998 - 2017, 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 + * are also available at https://curl.haxx.se/docs/copyright.html. + * + * You may opt to use, copy, modify, merge, publish, distribute and/or sell + * copies of the Software, and permit persons to whom the Software is + * furnished to do so, under the terms of the COPYING file. + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY + * KIND, either express or implied. + * + ***************************************************************************/ +#include "test.h" + +#include "testutil.h" +#include "warnless.h" +#include "memdebug.h" + +#define TEST_HANG_TIMEOUT 60 * 1000 + +int test(char *URL) +{ + CURL *curls = NULL; + CURLM *multi = NULL; + int still_running; + int i = 0; + int res = 0; + CURLMsg *msg; + int counter = 3; + + start_test_timing(); + + global_init(CURL_GLOBAL_ALL); + + multi_init(multi); + + easy_init(curls); + + easy_setopt(curls, CURLOPT_URL, URL); + easy_setopt(curls, CURLOPT_HEADER, 1L); + easy_setopt(curls, CURLOPT_VERBOSE, 1L); + easy_setopt(curls, CURLOPT_USERPWD, "u:s"); + + multi_add_handle(multi, curls); + + multi_perform(multi, &still_running); + + abort_on_test_timeout(); + + while(still_running && counter--) { + int num; + res = curl_multi_wait(multi, NULL, 0, TEST_HANG_TIMEOUT, &num); + if(res != CURLM_OK) { + printf("curl_multi_wait() returned %d\n", res); + res = TEST_ERR_MAJOR_BAD; + goto test_cleanup; + } + + abort_on_test_timeout(); + + multi_perform(multi, &still_running); + + abort_on_test_timeout(); + } + + msg = curl_multi_info_read(multi, &still_running); + if(msg) + /* this should now contain a result code from the easy handle, + get it */ + i = msg->data.result; + +test_cleanup: + + /* undocumented cleanup sequence - type UA */ + + curl_multi_cleanup(multi); + curl_easy_cleanup(curls); + curl_global_cleanup(); + + if(res) + i = res; + + return i; /* return the final return code */ +} -- cgit v1.2.3