From db5a7f16e664eb5efd76d309f81f0316ff386e19 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Aug 19 2020 13:27:39 +0000 Subject: Resolves: CVE-2020-8231 - libcurl: wrong connect-only connection --- diff --git a/0004-curl-7.66.0-CVE-2020-8231.patch b/0004-curl-7.66.0-CVE-2020-8231.patch new file mode 100644 index 0000000..4c2431b --- /dev/null +++ b/0004-curl-7.66.0-CVE-2020-8231.patch @@ -0,0 +1,1450 @@ +From 29a1051e4901bca8a8b839455dcc1e9491486ef8 Mon Sep 17 00:00:00 2001 +From: Daniel Stenberg +Date: Mon, 9 Dec 2019 11:53:54 +0100 +Subject: [PATCH 1/4] conncache: fix multi-thread use of shared connection + cache + +It could accidentally let the connection get used by more than one +thread, leading to double-free and more. + +Reported-by: Christopher Reid +Fixes #4544 +Closes #4557 + +Upstream-commit: ee263de7a378e701f15e58879f36fdcfe8742006 +Signed-off-by: Kamil Dudka +--- + lib/conncache.c | 30 ++++-------------------------- + lib/conncache.h | 24 +++++++++++++++++++++++- + lib/http.c | 2 +- + lib/http2.c | 5 ++--- + lib/http2.h | 2 +- + lib/multi.c | 20 +++++++++++++------- + lib/url.c | 21 ++++++++------------- + tests/data/test1554 | 6 ++++++ + 8 files changed, 58 insertions(+), 52 deletions(-) + +diff --git a/lib/conncache.c b/lib/conncache.c +index 2f4dd4b..6344b92 100644 +--- a/lib/conncache.c ++++ b/lib/conncache.c +@@ -40,27 +40,6 @@ + #include "curl_memory.h" + #include "memdebug.h" + +-#ifdef CURLDEBUG +-/* the debug versions of these macros make extra certain that the lock is +- never doubly locked or unlocked */ +-#define CONN_LOCK(x) if((x)->share) { \ +- Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE); \ +- DEBUGASSERT(!(x)->state.conncache_lock); \ +- (x)->state.conncache_lock = TRUE; \ +- } +- +-#define CONN_UNLOCK(x) if((x)->share) { \ +- DEBUGASSERT((x)->state.conncache_lock); \ +- (x)->state.conncache_lock = FALSE; \ +- Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT); \ +- } +-#else +-#define CONN_LOCK(x) if((x)->share) \ +- Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE) +-#define CONN_UNLOCK(x) if((x)->share) \ +- Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT) +-#endif +- + #define HASHKEY_SIZE 128 + + static void conn_llist_dtor(void *user, void *element) +@@ -122,6 +101,7 @@ static int bundle_remove_conn(struct connectbundle *cb_ptr, + } + curr = curr->next; + } ++ DEBUGASSERT(0); + return 0; + } + +@@ -430,17 +410,15 @@ conncache_find_first_connection(struct conncache *connc) + * + * Return TRUE if stored, FALSE if closed. + */ +-bool Curl_conncache_return_conn(struct connectdata *conn) ++bool Curl_conncache_return_conn(struct Curl_easy *data, ++ struct connectdata *conn) + { +- struct Curl_easy *data = conn->data; +- + /* data->multi->maxconnects can be negative, deal with it. */ + size_t maxconnects = + (data->multi->maxconnects < 0) ? data->multi->num_easy * 4: + data->multi->maxconnects; + struct connectdata *conn_candidate = NULL; + +- conn->data = NULL; /* no owner anymore */ + conn->lastused = Curl_now(); /* it was used up until now */ + if(maxconnects > 0 && + Curl_conncache_size(data) > maxconnects) { +@@ -543,7 +521,7 @@ Curl_conncache_extract_oldest(struct Curl_easy *data) + while(curr) { + conn = curr->ptr; + +- if(!CONN_INUSE(conn) && !conn->data) { ++ if(!CONN_INUSE(conn) && !conn->data && !conn->bits.close) { + /* Set higher score for the age passed since the connection was used */ + score = Curl_timediff(now, conn->lastused); + +diff --git a/lib/conncache.h b/lib/conncache.h +index 58f9024..5fe80b4 100644 +--- a/lib/conncache.h ++++ b/lib/conncache.h +@@ -42,6 +42,27 @@ struct conncache { + #define BUNDLE_UNKNOWN 0 /* initial value */ + #define BUNDLE_MULTIPLEX 2 + ++#ifdef CURLDEBUG ++/* the debug versions of these macros make extra certain that the lock is ++ never doubly locked or unlocked */ ++#define CONN_LOCK(x) if((x)->share) { \ ++ Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE); \ ++ DEBUGASSERT(!(x)->state.conncache_lock); \ ++ (x)->state.conncache_lock = TRUE; \ ++ } ++ ++#define CONN_UNLOCK(x) if((x)->share) { \ ++ DEBUGASSERT((x)->state.conncache_lock); \ ++ (x)->state.conncache_lock = FALSE; \ ++ Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT); \ ++ } ++#else ++#define CONN_LOCK(x) if((x)->share) \ ++ Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE) ++#define CONN_UNLOCK(x) if((x)->share) \ ++ Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT) ++#endif ++ + struct connectbundle { + int multiuse; /* supports multi-use */ + size_t num_connections; /* Number of connections in the bundle */ +@@ -61,7 +82,8 @@ void Curl_conncache_unlock(struct Curl_easy *data); + size_t Curl_conncache_size(struct Curl_easy *data); + size_t Curl_conncache_bundle_size(struct connectdata *conn); + +-bool Curl_conncache_return_conn(struct connectdata *conn); ++bool Curl_conncache_return_conn(struct Curl_easy *data, ++ struct connectdata *conn); + CURLcode Curl_conncache_add_conn(struct conncache *connc, + struct connectdata *conn) WARN_UNUSED_RESULT; + void Curl_conncache_remove_conn(struct Curl_easy *data, +diff --git a/lib/http.c b/lib/http.c +index 28d1fa6..4820e4f 100644 +--- a/lib/http.c ++++ b/lib/http.c +@@ -1620,7 +1620,7 @@ CURLcode Curl_http_done(struct connectdata *conn, + Curl_add_buffer_free(&http->send_buffer); + } + +- Curl_http2_done(conn, premature); ++ Curl_http2_done(data, premature); + + Curl_mime_cleanpart(&http->form); + +diff --git a/lib/http2.c b/lib/http2.c +index 31d2d69..3f3d7d6 100644 +--- a/lib/http2.c ++++ b/lib/http2.c +@@ -1168,11 +1168,10 @@ static void populate_settings(struct connectdata *conn, + httpc->local_settings_num = 3; + } + +-void Curl_http2_done(struct connectdata *conn, bool premature) ++void Curl_http2_done(struct Curl_easy *data, bool premature) + { +- struct Curl_easy *data = conn->data; + struct HTTP *http = data->req.protop; +- struct http_conn *httpc = &conn->proto.httpc; ++ struct http_conn *httpc = &data->conn->proto.httpc; + + /* there might be allocated resources done before this got the 'h2' pointer + setup */ +diff --git a/lib/http2.h b/lib/http2.h +index 93058cc..12d36ee 100644 +--- a/lib/http2.h ++++ b/lib/http2.h +@@ -50,7 +50,7 @@ CURLcode Curl_http2_switched(struct connectdata *conn, + /* called from http_setup_conn */ + void Curl_http2_setup_conn(struct connectdata *conn); + void Curl_http2_setup_req(struct Curl_easy *data); +-void Curl_http2_done(struct connectdata *conn, bool premature); ++void Curl_http2_done(struct Curl_easy *data, bool premature); + CURLcode Curl_http2_done_sending(struct connectdata *conn); + CURLcode Curl_http2_add_child(struct Curl_easy *parent, + struct Curl_easy *child, +diff --git a/lib/multi.c b/lib/multi.c +index 2e91e4f..4dd064b 100755 +--- a/lib/multi.c ++++ b/lib/multi.c +@@ -531,6 +531,8 @@ static CURLcode multi_done(struct Curl_easy *data, + /* Stop if multi_done() has already been called */ + return CURLE_OK; + ++ conn->data = data; /* ensure the connection uses this transfer now */ ++ + /* Stop the resolver and free its own resources (but not dns_entry yet). */ + Curl_resolver_kill(conn); + +@@ -567,15 +569,17 @@ static CURLcode multi_done(struct Curl_easy *data, + + process_pending_handles(data->multi); /* connection / multiplex */ + ++ CONN_LOCK(data); + detach_connnection(data); + if(CONN_INUSE(conn)) { + /* Stop if still used. */ ++ CONN_UNLOCK(data); + DEBUGF(infof(data, "Connection still in use %zu, " + "no more multi_done now!\n", + conn->easyq.size)); + return CURLE_OK; + } +- ++ conn->data = NULL; /* the connection now has no owner */ + data->state.done = TRUE; /* called just now! */ + + if(conn->dns_entry) { +@@ -618,7 +622,10 @@ static CURLcode multi_done(struct Curl_easy *data, + #endif + ) || conn->bits.close + || (premature && !(conn->handler->flags & PROTOPT_STREAM))) { +- CURLcode res2 = Curl_disconnect(data, conn, premature); ++ CURLcode res2; ++ conn->bits.close = TRUE; /* forcibly prevents reuse */ ++ CONN_UNLOCK(data); ++ res2 = Curl_disconnect(data, conn, premature); + + /* If we had an error already, make sure we return that one. But + if we got a new error, return that. */ +@@ -635,9 +642,9 @@ static CURLcode multi_done(struct Curl_easy *data, + conn->bits.httpproxy ? conn->http_proxy.host.dispname : + conn->bits.conn_to_host ? conn->conn_to_host.dispname : + conn->host.dispname); +- + /* the connection is no longer in use by this transfer */ +- if(Curl_conncache_return_conn(conn)) { ++ CONN_UNLOCK(data); ++ if(Curl_conncache_return_conn(data, conn)) { + /* remember the most recently used connection */ + data->state.lastconnect = conn; + infof(data, "%s\n", buffer); +@@ -744,10 +751,8 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi, + vanish with this handle */ + + /* Remove the association between the connection and the handle */ +- if(data->conn) { +- data->conn->data = NULL; ++ if(data->conn) + detach_connnection(data); +- } + + #ifdef USE_LIBPSL + /* Remove the PSL association. */ +@@ -1242,6 +1247,7 @@ static CURLcode multi_do(struct Curl_easy *data, bool *done) + + DEBUGASSERT(conn); + DEBUGASSERT(conn->handler); ++ DEBUGASSERT(conn->data == data); + + if(conn->handler->do_it) { + /* generic protocol-specific function pointer set in curl_connect() */ +diff --git a/lib/url.c b/lib/url.c +index 767ddec..91729b9 100644 +--- a/lib/url.c ++++ b/lib/url.c +@@ -1074,16 +1074,15 @@ ConnectionExists(struct Curl_easy *data, + check = curr->ptr; + curr = curr->next; + +- if(check->bits.connect_only) +- /* connect-only connections will not be reused */ ++ if(check->bits.connect_only || check->bits.close) ++ /* connect-only or to-be-closed connections will not be reused */ + continue; + + multiplexed = CONN_INUSE(check) && + (bundle->multiuse == BUNDLE_MULTIPLEX); + + if(canmultiplex) { +- if(check->bits.protoconnstart && check->bits.close) +- continue; ++ ; + } + else { + if(multiplexed) { +@@ -1103,12 +1102,9 @@ ConnectionExists(struct Curl_easy *data, + } + } + +- if((check->sock[FIRSTSOCKET] == CURL_SOCKET_BAD) || +- check->bits.close) { +- if(!check->bits.close) +- foundPendingCandidate = TRUE; +- /* Don't pick a connection that hasn't connected yet or that is going +- to get closed. */ ++ if(check->sock[FIRSTSOCKET] == CURL_SOCKET_BAD) { ++ foundPendingCandidate = TRUE; ++ /* Don't pick a connection that hasn't connected yet */ + infof(data, "Connection #%ld isn't open enough, can't reuse\n", + check->connection_id); + continue; +@@ -1186,8 +1182,7 @@ ConnectionExists(struct Curl_easy *data, + already in use so we skip it */ + continue; + +- if(CONN_INUSE(check) && check->data && +- (check->data->multi != needle->data->multi)) ++ if(check->data && (check->data->multi != needle->data->multi)) + /* this could be subject for multiplex use, but only if they belong to + * the same multi handle */ + continue; +@@ -1629,6 +1624,7 @@ static struct connectdata *allocate_conn(struct Curl_easy *data) + it may live on without (this specific) Curl_easy */ + conn->fclosesocket = data->set.fclosesocket; + conn->closesocket_client = data->set.closesocket_client; ++ conn->lastused = Curl_now(); /* used now */ + + return conn; + error: +@@ -3592,7 +3588,6 @@ static CURLcode create_conn(struct Curl_easy *data, + reuse = FALSE; + + infof(data, "We can reuse, but we want a new connection anyway\n"); +- Curl_conncache_return_conn(conn_temp); + } + } + } +diff --git a/tests/data/test1554 b/tests/data/test1554 +index be48e02..06f1897 100644 +--- a/tests/data/test1554 ++++ b/tests/data/test1554 +@@ -38,6 +38,8 @@ run 1: foobar and so on fun! + <- Mutex unlock + -> Mutex lock + <- Mutex unlock ++-> Mutex lock ++<- Mutex unlock + run 1: foobar and so on fun! + -> Mutex lock + <- Mutex unlock +@@ -47,6 +49,8 @@ run 1: foobar and so on fun! + <- Mutex unlock + -> Mutex lock + <- Mutex unlock ++-> Mutex lock ++<- Mutex unlock + run 1: foobar and so on fun! + -> Mutex lock + <- Mutex unlock +@@ -54,6 +58,8 @@ run 1: foobar and so on fun! + <- Mutex unlock + -> Mutex lock + <- Mutex unlock ++-> Mutex lock ++<- Mutex unlock + + + +-- +2.25.4 + + +From c3359693e17fccdf2a04f0b908bc8f51cdc38133 Mon Sep 17 00:00:00 2001 +From: Daniel Stenberg +Date: Mon, 27 Apr 2020 00:33:21 +0200 +Subject: [PATCH 2/4] conncache: various concept cleanups + +More connection cache accesses are protected by locks. + +CONNCACHE_* is a beter prefix for the connection cache lock macros. + +Curl_attach_connnection: now called as soon as there's a connection +struct available and before the connection is added to the connection +cache. + +Curl_disconnect: now assumes that the connection is already removed from +the connection cache. + +Ref: #4915 +Closes #5009 + +Upstream-commit: c06902713998d68202c5a764de910ba8d0e8f54d +Signed-off-by: Kamil Dudka +--- + lib/conncache.c | 91 ++++++++++++++++++++----------------------- + lib/conncache.h | 9 ++--- + lib/hostip.c | 12 +++--- + lib/http_negotiate.h | 6 ++- + lib/http_ntlm.h | 6 ++- + lib/multi.c | 56 +++++++++++++------------- + lib/multiif.h | 1 + + lib/url.c | 69 +++++++++++++++++--------------- + tests/data/test1554 | 18 +++++++++ + tests/unit/unit1620.c | 4 -- + 10 files changed, 144 insertions(+), 128 deletions(-) + +diff --git a/lib/conncache.c b/lib/conncache.c +index cbd3bb1..95fcea6 100644 +--- a/lib/conncache.c ++++ b/lib/conncache.c +@@ -49,53 +49,51 @@ static void conn_llist_dtor(void *user, void *element) + conn->bundle = NULL; + } + +-static CURLcode bundle_create(struct Curl_easy *data, +- struct connectbundle **cb_ptr) ++static CURLcode bundle_create(struct connectbundle **bundlep) + { +- (void)data; +- DEBUGASSERT(*cb_ptr == NULL); +- *cb_ptr = malloc(sizeof(struct connectbundle)); +- if(!*cb_ptr) ++ DEBUGASSERT(*bundlep == NULL); ++ *bundlep = malloc(sizeof(struct connectbundle)); ++ if(!*bundlep) + return CURLE_OUT_OF_MEMORY; + +- (*cb_ptr)->num_connections = 0; +- (*cb_ptr)->multiuse = BUNDLE_UNKNOWN; ++ (*bundlep)->num_connections = 0; ++ (*bundlep)->multiuse = BUNDLE_UNKNOWN; + +- Curl_llist_init(&(*cb_ptr)->conn_list, (curl_llist_dtor) conn_llist_dtor); ++ Curl_llist_init(&(*bundlep)->conn_list, (curl_llist_dtor) conn_llist_dtor); + return CURLE_OK; + } + +-static void bundle_destroy(struct connectbundle *cb_ptr) ++static void bundle_destroy(struct connectbundle *bundle) + { +- if(!cb_ptr) ++ if(!bundle) + return; + +- Curl_llist_destroy(&cb_ptr->conn_list, NULL); ++ Curl_llist_destroy(&bundle->conn_list, NULL); + +- free(cb_ptr); ++ free(bundle); + } + + /* Add a connection to a bundle */ +-static void bundle_add_conn(struct connectbundle *cb_ptr, ++static void bundle_add_conn(struct connectbundle *bundle, + struct connectdata *conn) + { +- Curl_llist_insert_next(&cb_ptr->conn_list, cb_ptr->conn_list.tail, conn, ++ Curl_llist_insert_next(&bundle->conn_list, bundle->conn_list.tail, conn, + &conn->bundle_node); +- conn->bundle = cb_ptr; +- cb_ptr->num_connections++; ++ conn->bundle = bundle; ++ bundle->num_connections++; + } + + /* Remove a connection from a bundle */ +-static int bundle_remove_conn(struct connectbundle *cb_ptr, ++static int bundle_remove_conn(struct connectbundle *bundle, + struct connectdata *conn) + { + struct curl_llist_element *curr; + +- curr = cb_ptr->conn_list.head; ++ curr = bundle->conn_list.head; + while(curr) { + if(curr->ptr == conn) { +- Curl_llist_remove(&cb_ptr->conn_list, curr, NULL); +- cb_ptr->num_connections--; ++ Curl_llist_remove(&bundle->conn_list, curr, NULL); ++ bundle->num_connections--; + conn->bundle = NULL; + return 1; /* we removed a handle */ + } +@@ -164,20 +162,15 @@ static void hashkey(struct connectdata *conn, char *buf, + msnprintf(buf, len, "%ld%s", port, hostname); + } + +-void Curl_conncache_unlock(struct Curl_easy *data) +-{ +- CONN_UNLOCK(data); +-} +- + /* Returns number of connections currently held in the connection cache. + Locks/unlocks the cache itself! + */ + size_t Curl_conncache_size(struct Curl_easy *data) + { + size_t num; +- CONN_LOCK(data); ++ CONNCACHE_LOCK(data); + num = data->state.conn_cache->num_conn; +- CONN_UNLOCK(data); ++ CONNCACHE_UNLOCK(data); + return num; + } + +@@ -187,9 +180,9 @@ size_t Curl_conncache_size(struct Curl_easy *data) + size_t Curl_conncache_bundle_size(struct connectdata *conn) + { + size_t num; +- CONN_LOCK(conn->data); ++ CONNCACHE_LOCK(conn->data); + num = conn->bundle->num_connections; +- CONN_UNLOCK(conn->data); ++ CONNCACHE_UNLOCK(conn->data); + return num; + } + +@@ -202,7 +195,7 @@ struct connectbundle *Curl_conncache_find_bundle(struct connectdata *conn, + const char **hostp) + { + struct connectbundle *bundle = NULL; +- CONN_LOCK(conn->data); ++ CONNCACHE_LOCK(conn->data); + if(connc) { + char key[HASHKEY_SIZE]; + hashkey(conn, key, sizeof(key), hostp); +@@ -249,8 +242,7 @@ CURLcode Curl_conncache_add_conn(struct conncache *connc, + struct connectdata *conn) + { + CURLcode result = CURLE_OK; +- struct connectbundle *bundle; +- struct connectbundle *new_bundle = NULL; ++ struct connectbundle *bundle = NULL; + struct Curl_easy *data = conn->data; + + /* *find_bundle() locks the connection cache */ +@@ -259,20 +251,19 @@ CURLcode Curl_conncache_add_conn(struct conncache *connc, + int rc; + char key[HASHKEY_SIZE]; + +- result = bundle_create(data, &new_bundle); ++ result = bundle_create(&bundle); + if(result) { + goto unlock; + } + + hashkey(conn, key, sizeof(key), NULL); +- rc = conncache_add_bundle(data->state.conn_cache, key, new_bundle); ++ rc = conncache_add_bundle(data->state.conn_cache, key, bundle); + + if(!rc) { +- bundle_destroy(new_bundle); ++ bundle_destroy(bundle); + result = CURLE_OUT_OF_MEMORY; + goto unlock; + } +- bundle = new_bundle; + } + + bundle_add_conn(bundle, conn); +@@ -284,15 +275,17 @@ CURLcode Curl_conncache_add_conn(struct conncache *connc, + conn->connection_id, connc->num_conn)); + + unlock: +- CONN_UNLOCK(data); ++ CONNCACHE_UNLOCK(data); + + return result; + } + + /* +- * Removes the connectdata object from the connection cache *and* clears the +- * ->data pointer association. Pass TRUE/FALSE in the 'lock' argument +- * depending on if the parent function already holds the lock or not. ++ * Removes the connectdata object from the connection cache, but does *not* ++ * clear the conn->data association. The transfer still owns this connection. ++ * ++ * Pass TRUE/FALSE in the 'lock' argument depending on if the parent function ++ * already holds the lock or not. + */ + void Curl_conncache_remove_conn(struct Curl_easy *data, + struct connectdata *conn, bool lock) +@@ -304,7 +297,7 @@ void Curl_conncache_remove_conn(struct Curl_easy *data, + due to a failed connection attempt, before being added to a bundle */ + if(bundle) { + if(lock) { +- CONN_LOCK(data); ++ CONNCACHE_LOCK(data); + } + bundle_remove_conn(bundle, conn); + if(bundle->num_connections == 0) +@@ -315,9 +308,8 @@ void Curl_conncache_remove_conn(struct Curl_easy *data, + DEBUGF(infof(data, "The cache now contains %zu members\n", + connc->num_conn)); + } +- conn->data = NULL; /* clear the association */ + if(lock) { +- CONN_UNLOCK(data); ++ CONNCACHE_UNLOCK(data); + } + } + } +@@ -346,7 +338,7 @@ bool Curl_conncache_foreach(struct Curl_easy *data, + if(!connc) + return FALSE; + +- CONN_LOCK(data); ++ CONNCACHE_LOCK(data); + Curl_hash_start_iterate(&connc->hash, &iter); + + he = Curl_hash_next_element(&iter); +@@ -364,12 +356,12 @@ bool Curl_conncache_foreach(struct Curl_easy *data, + curr = curr->next; + + if(1 == func(conn, param)) { +- CONN_UNLOCK(data); ++ CONNCACHE_UNLOCK(data); + return TRUE; + } + } + } +- CONN_UNLOCK(data); ++ CONNCACHE_UNLOCK(data); + return FALSE; + } + +@@ -508,7 +500,7 @@ Curl_conncache_extract_oldest(struct Curl_easy *data) + + now = Curl_now(); + +- CONN_LOCK(data); ++ CONNCACHE_LOCK(data); + Curl_hash_start_iterate(&connc->hash, &iter); + + he = Curl_hash_next_element(&iter); +@@ -544,7 +536,7 @@ Curl_conncache_extract_oldest(struct Curl_easy *data) + connc->num_conn)); + conn_candidate->data = data; /* associate! */ + } +- CONN_UNLOCK(data); ++ CONNCACHE_UNLOCK(data); + + return conn_candidate; + } +@@ -561,6 +553,7 @@ void Curl_conncache_close_all_connections(struct conncache *connc) + sigpipe_ignore(conn->data, &pipe_st); + /* This will remove the connection from the cache */ + connclose(conn, "kill all"); ++ Curl_conncache_remove_conn(conn->data, conn, TRUE); + (void)Curl_disconnect(connc->closure_handle, conn, FALSE); + sigpipe_restore(&pipe_st); + +diff --git a/lib/conncache.h b/lib/conncache.h +index e3e4c9c..3dda21c 100644 +--- a/lib/conncache.h ++++ b/lib/conncache.h +@@ -45,21 +45,21 @@ struct conncache { + #ifdef CURLDEBUG + /* the debug versions of these macros make extra certain that the lock is + never doubly locked or unlocked */ +-#define CONN_LOCK(x) if((x)->share) { \ ++#define CONNCACHE_LOCK(x) if((x)->share) { \ + Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE); \ + DEBUGASSERT(!(x)->state.conncache_lock); \ + (x)->state.conncache_lock = TRUE; \ + } + +-#define CONN_UNLOCK(x) if((x)->share) { \ ++#define CONNCACHE_UNLOCK(x) if((x)->share) { \ + DEBUGASSERT((x)->state.conncache_lock); \ + (x)->state.conncache_lock = FALSE; \ + Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT); \ + } + #else +-#define CONN_LOCK(x) if((x)->share) \ ++#define CONNCACHE_LOCK(x) if((x)->share) \ + Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE) +-#define CONN_UNLOCK(x) if((x)->share) \ ++#define CONNCACHE_UNLOCK(x) if((x)->share) \ + Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT) + #endif + +@@ -77,7 +77,6 @@ void Curl_conncache_destroy(struct conncache *connc); + struct connectbundle *Curl_conncache_find_bundle(struct connectdata *conn, + struct conncache *connc, + const char **hostp); +-void Curl_conncache_unlock(struct Curl_easy *data); + /* returns number of connections currently held in the connection cache */ + size_t Curl_conncache_size(struct Curl_easy *data); + size_t Curl_conncache_bundle_size(struct connectdata *conn); +diff --git a/lib/hostip.c b/lib/hostip.c +index c0feb79..f5bb634 100644 +--- a/lib/hostip.c ++++ b/lib/hostip.c +@@ -1059,10 +1059,12 @@ CURLcode Curl_once_resolved(struct connectdata *conn, + + result = Curl_setup_conn(conn, protocol_done); + +- if(result) +- /* We're not allowed to return failure with memory left allocated +- in the connectdata struct, free those here */ +- Curl_disconnect(conn->data, conn, TRUE); /* close the connection */ +- ++ if(result) { ++ struct Curl_easy *data = conn->data; ++ DEBUGASSERT(data); ++ Curl_detach_connnection(data); ++ Curl_conncache_remove_conn(data, conn, TRUE); ++ Curl_disconnect(data, conn, TRUE); ++ } + return result; + } +diff --git a/lib/http_negotiate.h b/lib/http_negotiate.h +index 4f0ac16..a737f6f 100644 +--- a/lib/http_negotiate.h ++++ b/lib/http_negotiate.h +@@ -7,7 +7,7 @@ + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * +- * Copyright (C) 1998 - 2019, Daniel Stenberg, , et al. ++ * Copyright (C) 1998 - 2020, 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 +@@ -33,6 +33,8 @@ CURLcode Curl_output_negotiate(struct connectdata *conn, bool proxy); + + void Curl_http_auth_cleanup_negotiate(struct connectdata *conn); + +-#endif /* !CURL_DISABLE_HTTP && USE_SPNEGO */ ++#else /* !CURL_DISABLE_HTTP && USE_SPNEGO */ ++#define Curl_http_auth_cleanup_negotiate(x) ++#endif + + #endif /* HEADER_CURL_HTTP_NEGOTIATE_H */ +diff --git a/lib/http_ntlm.h b/lib/http_ntlm.h +index 003714d..3ebdf97 100644 +--- a/lib/http_ntlm.h ++++ b/lib/http_ntlm.h +@@ -7,7 +7,7 @@ + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * +- * Copyright (C) 1998 - 2019, Daniel Stenberg, , et al. ++ * Copyright (C) 1998 - 2020, 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 +@@ -35,6 +35,8 @@ CURLcode Curl_output_ntlm(struct connectdata *conn, bool proxy); + + void Curl_http_auth_cleanup_ntlm(struct connectdata *conn); + +-#endif /* !CURL_DISABLE_HTTP && USE_NTLM */ ++#else /* !CURL_DISABLE_HTTP && USE_NTLM */ ++#define Curl_http_auth_cleanup_ntlm(x) ++#endif + + #endif /* HEADER_CURL_HTTP_NTLM_H */ +diff --git a/lib/multi.c b/lib/multi.c +index e10e752..273653d 100644 +--- a/lib/multi.c ++++ b/lib/multi.c +@@ -77,7 +77,6 @@ static CURLMcode add_next_timeout(struct curltime now, + static CURLMcode multi_timeout(struct Curl_multi *multi, + long *timeout_ms); + static void process_pending_handles(struct Curl_multi *multi); +-static void detach_connnection(struct Curl_easy *data); + + #ifdef DEBUGBUILD + static const char * const statename[]={ +@@ -110,7 +109,7 @@ static void Curl_init_completed(struct Curl_easy *data) + + /* Important: reset the conn pointer so that we don't point to memory + that could be freed anytime */ +- detach_connnection(data); ++ Curl_detach_connnection(data); + Curl_expire_clear(data); /* stop all timers */ + } + +@@ -486,6 +485,7 @@ CURLMcode curl_multi_add_handle(struct Curl_multi *multi, + easy handle is added */ + memset(&multi->timer_lastcall, 0, sizeof(multi->timer_lastcall)); + ++ CONNCACHE_LOCK(data); + /* The closure handle only ever has default timeouts set. To improve the + state somewhat we clone the timeouts from each added handle so that the + closure handle always has the same timeouts as the most recently added +@@ -495,6 +495,7 @@ CURLMcode curl_multi_add_handle(struct Curl_multi *multi, + data->set.server_response_timeout; + data->state.conn_cache->closure_handle->set.no_signal = + data->set.no_signal; ++ CONNCACHE_UNLOCK(data); + + Curl_update_timer(multi); + return CURLM_OK; +@@ -569,11 +570,11 @@ static CURLcode multi_done(struct Curl_easy *data, + + process_pending_handles(data->multi); /* connection / multiplex */ + +- CONN_LOCK(data); +- detach_connnection(data); ++ CONNCACHE_LOCK(data); ++ Curl_detach_connnection(data); + if(CONN_INUSE(conn)) { + /* Stop if still used. */ +- CONN_UNLOCK(data); ++ CONNCACHE_UNLOCK(data); + DEBUGF(infof(data, "Connection still in use %zu, " + "no more multi_done now!\n", + conn->easyq.size)); +@@ -624,7 +625,8 @@ static CURLcode multi_done(struct Curl_easy *data, + || (premature && !(conn->handler->flags & PROTOPT_STREAM))) { + CURLcode res2; + conn->bits.close = TRUE; /* forcibly prevents reuse */ +- CONN_UNLOCK(data); ++ Curl_conncache_remove_conn(data, conn, FALSE); ++ CONNCACHE_UNLOCK(data); + res2 = Curl_disconnect(data, conn, premature); + + /* If we had an error already, make sure we return that one. But +@@ -643,7 +645,7 @@ static CURLcode multi_done(struct Curl_easy *data, + conn->bits.conn_to_host ? conn->conn_to_host.dispname : + conn->host.dispname); + /* the connection is no longer in use by this transfer */ +- CONN_UNLOCK(data); ++ CONNCACHE_UNLOCK(data); + if(Curl_conncache_return_conn(data, conn)) { + /* remember the most recently used connection */ + data->state.lastconnect = conn; +@@ -751,8 +753,7 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi, + vanish with this handle */ + + /* Remove the association between the connection and the handle */ +- if(data->conn) +- detach_connnection(data); ++ Curl_detach_connnection(data); + + #ifdef USE_LIBPSL + /* Remove the PSL association. */ +@@ -801,9 +802,13 @@ bool Curl_multiplex_wanted(const struct Curl_multi *multi) + return (multi && (multi->multiplexing)); + } + +-/* This is the only function that should clear data->conn. This will +- occasionally be called with the pointer already cleared. */ +-static void detach_connnection(struct Curl_easy *data) ++/* ++ * Curl_detach_connnection() removes the given transfer from the connection. ++ * ++ * This is the only function that should clear data->conn. This will ++ * occasionally be called with the data->conn pointer already cleared. ++ */ ++void Curl_detach_connnection(struct Curl_easy *data) + { + struct connectdata *conn = data->conn; + if(conn) +@@ -811,7 +816,11 @@ static void detach_connnection(struct Curl_easy *data) + data->conn = NULL; + } + +-/* This is the only function that should assign data->conn */ ++/* ++ * Curl_attach_connnection() attaches this transfer to this connection. ++ * ++ * This is the only function that should assign data->conn ++ */ + void Curl_attach_connnection(struct Curl_easy *data, + struct connectdata *conn) + { +@@ -1414,19 +1423,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, + bool stream_error = FALSE; + rc = CURLM_OK; + +- DEBUGASSERT((data->mstate <= CURLM_STATE_CONNECT) || +- (data->mstate >= CURLM_STATE_DONE) || +- data->conn); +- if(!data->conn && +- data->mstate > CURLM_STATE_CONNECT && +- data->mstate < CURLM_STATE_DONE) { +- /* In all these states, the code will blindly access 'data->conn' +- so this is precaution that it isn't NULL. And it silences static +- analyzers. */ +- failf(data, "In state %d with no conn, bail out!\n", data->mstate); +- return CURLM_INTERNAL_ERROR; +- } +- + if(multi_ischanged(multi, TRUE)) { + DEBUGF(infof(data, "multi changed, check CONNECT_PEND queue!\n")); + process_pending_handles(multi); /* multiplexed */ +@@ -2104,8 +2100,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, + * access free'd data, if the connection is free'd and the handle + * removed before we perform the processing in CURLM_STATE_COMPLETED + */ +- if(data->conn) +- detach_connnection(data); ++ Curl_detach_connnection(data); + } + + #ifndef CURL_DISABLE_FTP +@@ -2157,7 +2152,10 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, + /* This is where we make sure that the conn pointer is reset. + We don't have to do this in every case block above where a + failure is detected */ +- detach_connnection(data); ++ Curl_detach_connnection(data); ++ ++ /* remove connection from cache */ ++ Curl_conncache_remove_conn(data, conn, TRUE); + + /* disconnect properly */ + Curl_disconnect(data, conn, dead_connection); +diff --git a/lib/multiif.h b/lib/multiif.h +index bde755e..c07587b 100644 +--- a/lib/multiif.h ++++ b/lib/multiif.h +@@ -33,6 +33,7 @@ void Curl_expire_done(struct Curl_easy *data, expire_id id); + void Curl_update_timer(struct Curl_multi *multi); + void Curl_attach_connnection(struct Curl_easy *data, + struct connectdata *conn); ++void Curl_detach_connnection(struct Curl_easy *data); + bool Curl_multiplex_wanted(const struct Curl_multi *multi); + void Curl_set_in_callback(struct Curl_easy *data, bool value); + bool Curl_is_in_callback(struct Curl_easy *easy); +diff --git a/lib/url.c b/lib/url.c +index a826f8a..4ed0623 100644 +--- a/lib/url.c ++++ b/lib/url.c +@@ -672,9 +672,7 @@ static void conn_reset_all_postponed_data(struct connectdata *conn) + + static void conn_shutdown(struct connectdata *conn) + { +- if(!conn) +- return; +- ++ DEBUGASSERT(conn); + infof(conn->data, "Closing connection %ld\n", conn->connection_id); + DEBUGASSERT(conn->data); + +@@ -695,16 +693,11 @@ static void conn_shutdown(struct connectdata *conn) + Curl_closesocket(conn, conn->tempsock[0]); + if(CURL_SOCKET_BAD != conn->tempsock[1]) + Curl_closesocket(conn, conn->tempsock[1]); +- +- /* unlink ourselves. this should be called last since other shutdown +- procedures need a valid conn->data and this may clear it. */ +- Curl_conncache_remove_conn(conn->data, conn, TRUE); + } + + static void conn_free(struct connectdata *conn) + { +- if(!conn) +- return; ++ DEBUGASSERT(conn); + + free_idnconverted_hostname(&conn->host); + free_idnconverted_hostname(&conn->conn_to_host); +@@ -772,13 +765,17 @@ static void conn_free(struct connectdata *conn) + CURLcode Curl_disconnect(struct Curl_easy *data, + struct connectdata *conn, bool dead_connection) + { +- if(!conn) +- return CURLE_OK; /* this is closed and fine already */ ++ /* there must be a connection to close */ ++ DEBUGASSERT(conn); + +- if(!data) { +- DEBUGF(infof(data, "DISCONNECT without easy handle, ignoring\n")); +- return CURLE_OK; +- } ++ /* it must be removed from the connection cache */ ++ DEBUGASSERT(!conn->bundle); ++ ++ /* there must be an associated transfer */ ++ DEBUGASSERT(data); ++ ++ /* the transfer must be detached from the connection */ ++ DEBUGASSERT(!data->conn); + + /* + * If this connection isn't marked to force-close, leave it open if there +@@ -794,16 +791,11 @@ CURLcode Curl_disconnect(struct Curl_easy *data, + conn->dns_entry = NULL; + } + +- Curl_hostcache_prune(data); /* kill old DNS cache entries */ +- +-#if !defined(CURL_DISABLE_HTTP) && defined(USE_NTLM) + /* Cleanup NTLM connection-related data */ + Curl_http_auth_cleanup_ntlm(conn); +-#endif +-#if !defined(CURL_DISABLE_HTTP) && defined(USE_SPNEGO) ++ + /* Cleanup NEGOTIATE connection-related data */ + Curl_http_auth_cleanup_negotiate(conn); +-#endif + + /* the protocol specific disconnect handler and conn_shutdown need a transfer + for the connection! */ +@@ -972,8 +964,12 @@ static int call_extract_if_dead(struct connectdata *conn, void *param) + static void prune_dead_connections(struct Curl_easy *data) + { + struct curltime now = Curl_now(); +- timediff_t elapsed = ++ timediff_t elapsed; ++ ++ CONNCACHE_LOCK(data); ++ elapsed = + Curl_timediff(now, data->state.conn_cache->last_cleanup); ++ CONNCACHE_UNLOCK(data); + + if(elapsed >= 1000L) { + struct prunedead prune; +@@ -981,10 +977,17 @@ static void prune_dead_connections(struct Curl_easy *data) + prune.extracted = NULL; + while(Curl_conncache_foreach(data, data->state.conn_cache, &prune, + call_extract_if_dead)) { ++ /* unlocked */ ++ ++ /* remove connection from cache */ ++ Curl_conncache_remove_conn(data, prune.extracted, TRUE); ++ + /* disconnect it */ + (void)Curl_disconnect(data, prune.extracted, /* dead_connection */TRUE); + } ++ CONNCACHE_LOCK(data); + data->state.conn_cache->last_cleanup = now; ++ CONNCACHE_UNLOCK(data); + } + } + +@@ -1044,7 +1047,7 @@ ConnectionExists(struct Curl_easy *data, + if((bundle->multiuse == BUNDLE_UNKNOWN) && data->set.pipewait) { + infof(data, "Server doesn't support multiplex yet, wait\n"); + *waitpipe = TRUE; +- Curl_conncache_unlock(data); ++ CONNCACHE_UNLOCK(data); + return FALSE; /* no re-use */ + } + +@@ -1352,11 +1355,12 @@ ConnectionExists(struct Curl_easy *data, + if(chosen) { + /* mark it as used before releasing the lock */ + chosen->data = data; /* own it! */ +- Curl_conncache_unlock(data); ++ Curl_attach_connnection(data, chosen); ++ CONNCACHE_UNLOCK(data); + *usethis = chosen; + return TRUE; /* yes, we found one to use! */ + } +- Curl_conncache_unlock(data); ++ CONNCACHE_UNLOCK(data); + + if(foundPendingCandidate && data->set.pipewait) { + infof(data, +@@ -3466,6 +3470,7 @@ static CURLcode create_conn(struct Curl_easy *data, + if(!result) { + conn->bits.tcpconnect[FIRSTSOCKET] = TRUE; /* we are "connected */ + ++ Curl_attach_connnection(data, conn); + result = Curl_conncache_add_conn(data->state.conn_cache, conn); + if(result) + goto out; +@@ -3480,7 +3485,6 @@ static CURLcode create_conn(struct Curl_easy *data, + (void)conn->handler->done(conn, result, FALSE); + goto out; + } +- Curl_attach_connnection(data, conn); + Curl_setup_transfer(data, -1, -1, FALSE, -1); + } + +@@ -3644,7 +3648,7 @@ static CURLcode create_conn(struct Curl_easy *data, + + /* The bundle is full. Extract the oldest connection. */ + conn_candidate = Curl_conncache_extract_bundle(data, bundle); +- Curl_conncache_unlock(data); ++ CONNCACHE_UNLOCK(data); + + if(conn_candidate) + (void)Curl_disconnect(data, conn_candidate, +@@ -3656,7 +3660,7 @@ static CURLcode create_conn(struct Curl_easy *data, + } + } + else +- Curl_conncache_unlock(data); ++ CONNCACHE_UNLOCK(data); + + } + +@@ -3690,6 +3694,8 @@ static CURLcode create_conn(struct Curl_easy *data, + * This is a brand new connection, so let's store it in the connection + * cache of ours! + */ ++ Curl_attach_connnection(data, conn); ++ + result = Curl_conncache_add_conn(data->state.conn_cache, conn); + if(result) + goto out; +@@ -3842,7 +3848,7 @@ CURLcode Curl_connect(struct Curl_easy *data, + result = create_conn(data, &conn, asyncp); + + if(!result) { +- if(CONN_INUSE(conn)) ++ if(CONN_INUSE(conn) > 1) + /* multiplexed */ + *protocol_done = TRUE; + else if(!*asyncp) { +@@ -3859,11 +3865,10 @@ CURLcode Curl_connect(struct Curl_easy *data, + else if(result && conn) { + /* We're not allowed to return failure with memory left allocated in the + connectdata struct, free those here */ ++ Curl_detach_connnection(data); ++ Curl_conncache_remove_conn(data, conn, TRUE); + Curl_disconnect(data, conn, TRUE); + } +- else if(!result && !data->conn) +- /* FILE: transfers already have the connection attached */ +- Curl_attach_connnection(data, conn); + + return result; + } +diff --git a/tests/data/test1554 b/tests/data/test1554 +index 06f1897..d3926d9 100644 +--- a/tests/data/test1554 ++++ b/tests/data/test1554 +@@ -29,6 +29,12 @@ run 1: foobar and so on fun! + <- Mutex unlock + -> Mutex lock + <- Mutex unlock ++-> Mutex lock ++<- Mutex unlock ++-> Mutex lock ++<- Mutex unlock ++-> Mutex lock ++<- Mutex unlock + run 1: foobar and so on fun! + -> Mutex lock + <- Mutex unlock +@@ -40,6 +46,12 @@ run 1: foobar and so on fun! + <- Mutex unlock + -> Mutex lock + <- Mutex unlock ++-> Mutex lock ++<- Mutex unlock ++-> Mutex lock ++<- Mutex unlock ++-> Mutex lock ++<- Mutex unlock + run 1: foobar and so on fun! + -> Mutex lock + <- Mutex unlock +@@ -51,6 +63,12 @@ run 1: foobar and so on fun! + <- Mutex unlock + -> Mutex lock + <- Mutex unlock ++-> Mutex lock ++<- Mutex unlock ++-> Mutex lock ++<- Mutex unlock ++-> Mutex lock ++<- Mutex unlock + run 1: foobar and so on fun! + -> Mutex lock + <- Mutex unlock +diff --git a/tests/unit/unit1620.c b/tests/unit/unit1620.c +index 6e572c6..b23e5b9 100644 +--- a/tests/unit/unit1620.c ++++ b/tests/unit/unit1620.c +@@ -71,10 +71,6 @@ UNITTEST_START + fail_unless(rc == CURLE_OK, + "Curl_parse_login_details() failed"); + +- rc = Curl_disconnect(empty, empty->conn, FALSE); +- fail_unless(rc == CURLE_OK, +- "Curl_disconnect() with dead_connection set FALSE failed"); +- + Curl_freeset(empty); + for(i = (enum dupstring)0; i < STRING_LAST; i++) { + fail_unless(empty->set.str[i] == NULL, +-- +2.25.4 + + +From 6830828c9eecd9ab14404f2f49f19b56dec62130 Mon Sep 17 00:00:00 2001 +From: Marc Aldorasi +Date: Thu, 30 Jul 2020 14:16:17 -0400 +Subject: [PATCH 3/4] multi_remove_handle: close unused connect-only + connections + +Previously any connect-only connections in a multi handle would be kept +alive until the multi handle was closed. Since these connections cannot +be re-used, they can be marked for closure when the associated easy +handle is removed from the multi handle. + +Closes #5749 + +Upstream-commit: d5bb459ccf1fc5980ae4b95c05b4ecf6454a7599 +Signed-off-by: Kamil Dudka +--- + lib/multi.c | 34 ++++++++++++++++++++++++++++++---- + tests/data/test1554 | 6 ++++++ + 2 files changed, 36 insertions(+), 4 deletions(-) + +diff --git a/lib/multi.c b/lib/multi.c +index 249e360..f1371bd 100644 +--- a/lib/multi.c ++++ b/lib/multi.c +@@ -659,6 +659,26 @@ static CURLcode multi_done(struct Curl_easy *data, + return result; + } + ++static int close_connect_only(struct connectdata *conn, void *param) ++{ ++ struct Curl_easy *data = param; ++ ++ if(data->state.lastconnect != conn) ++ return 0; ++ ++ if(conn->data != data) ++ return 1; ++ conn->data = NULL; ++ ++ if(!conn->bits.connect_only) ++ return 1; ++ ++ connclose(conn, "Removing connect-only easy handle"); ++ conn->bits.connect_only = FALSE; ++ ++ return 1; ++} ++ + CURLMcode curl_multi_remove_handle(struct Curl_multi *multi, + struct Curl_easy *data) + { +@@ -742,10 +762,6 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi, + multi_done() as that may actually call Curl_expire that uses this */ + Curl_llist_destroy(&data->state.timeoutlist, NULL); + +- /* as this was using a shared connection cache we clear the pointer to that +- since we're not part of that multi handle anymore */ +- data->state.conn_cache = NULL; +- + /* change state without using multistate(), only to make singlesocket() do + what we want */ + data->mstate = CURLM_STATE_COMPLETED; +@@ -755,12 +771,22 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi, + /* Remove the association between the connection and the handle */ + Curl_detach_connnection(data); + ++ if(data->state.lastconnect) { ++ /* Mark any connect-only connection for closure */ ++ Curl_conncache_foreach(data, data->state.conn_cache, ++ data, &close_connect_only); ++ } ++ + #ifdef USE_LIBPSL + /* Remove the PSL association. */ + if(data->psl == &multi->psl) + data->psl = NULL; + #endif + ++ /* as this was using a shared connection cache we clear the pointer to that ++ since we're not part of that multi handle anymore */ ++ data->state.conn_cache = NULL; ++ + data->multi = NULL; /* clear the association to this multi handle */ + + /* make sure there's no pending message in the queue sent from this easy +diff --git a/tests/data/test1554 b/tests/data/test1554 +index d3926d9..fffa6ad 100644 +--- a/tests/data/test1554 ++++ b/tests/data/test1554 +@@ -52,6 +52,8 @@ run 1: foobar and so on fun! + <- Mutex unlock + -> Mutex lock + <- Mutex unlock ++-> Mutex lock ++<- Mutex unlock + run 1: foobar and so on fun! + -> Mutex lock + <- Mutex unlock +@@ -69,6 +71,8 @@ run 1: foobar and so on fun! + <- Mutex unlock + -> Mutex lock + <- Mutex unlock ++-> Mutex lock ++<- Mutex unlock + run 1: foobar and so on fun! + -> Mutex lock + <- Mutex unlock +@@ -78,6 +82,8 @@ run 1: foobar and so on fun! + <- Mutex unlock + -> Mutex lock + <- Mutex unlock ++-> Mutex lock ++<- Mutex unlock + + + +-- +2.25.4 + + +From 01148ee40dd913a169435b0f9ea90e6393821e70 Mon Sep 17 00:00:00 2001 +From: Daniel Stenberg +Date: Sun, 16 Aug 2020 11:34:35 +0200 +Subject: [PATCH 4/4] Curl_easy: remember last connection by id, not by pointer + +CVE-2020-8231 + +Bug: https://curl.haxx.se/docs/CVE-2020-8231.html + +Reported-by: Marc Aldorasi +Closes #5824 + +Upstream-commit: 3c9e021f86872baae412a427e807fbfa2f3e8a22 +Signed-off-by: Kamil Dudka +--- + lib/connect.c | 19 ++++++++++--------- + lib/easy.c | 3 +-- + lib/multi.c | 9 +++++---- + lib/url.c | 2 +- + lib/urldata.h | 2 +- + 5 files changed, 18 insertions(+), 17 deletions(-) + +diff --git a/lib/connect.c b/lib/connect.c +index 29293f0..e1c5662 100644 +--- a/lib/connect.c ++++ b/lib/connect.c +@@ -1328,15 +1328,15 @@ CURLcode Curl_connecthost(struct connectdata *conn, /* context */ + } + + struct connfind { +- struct connectdata *tofind; +- bool found; ++ long id_tofind; ++ struct connectdata *found; + }; + + static int conn_is_conn(struct connectdata *conn, void *param) + { + struct connfind *f = (struct connfind *)param; +- if(conn == f->tofind) { +- f->found = TRUE; ++ if(conn->connection_id == f->id_tofind) { ++ f->found = conn; + return 1; + } + return 0; +@@ -1358,21 +1358,22 @@ curl_socket_t Curl_getconnectinfo(struct Curl_easy *data, + * - that is associated with a multi handle, and whose connection + * was detached with CURLOPT_CONNECT_ONLY + */ +- if(data->state.lastconnect && (data->multi_easy || data->multi)) { +- struct connectdata *c = data->state.lastconnect; ++ if((data->state.lastconnect_id != -1) && (data->multi_easy || data->multi)) { ++ struct connectdata *c; + struct connfind find; +- find.tofind = data->state.lastconnect; +- find.found = FALSE; ++ find.id_tofind = data->state.lastconnect_id; ++ find.found = NULL; + + Curl_conncache_foreach(data, data->multi_easy? + &data->multi_easy->conn_cache: + &data->multi->conn_cache, &find, conn_is_conn); + + if(!find.found) { +- data->state.lastconnect = NULL; ++ data->state.lastconnect_id = -1; + return CURL_SOCKET_BAD; + } + ++ c = find.found; + if(connp) { + /* only store this if the caller cares for it */ + *connp = c; +diff --git a/lib/easy.c b/lib/easy.c +index 292cca7..a69eb9e 100644 +--- a/lib/easy.c ++++ b/lib/easy.c +@@ -828,8 +828,7 @@ struct Curl_easy *curl_easy_duphandle(struct Curl_easy *data) + + /* the connection cache is setup on demand */ + outcurl->state.conn_cache = NULL; +- +- outcurl->state.lastconnect = NULL; ++ outcurl->state.lastconnect_id = -1; + + outcurl->progress.flags = data->progress.flags; + outcurl->progress.callback = data->progress.callback; +diff --git a/lib/multi.c b/lib/multi.c +index f1371bd..778c537 100644 +--- a/lib/multi.c ++++ b/lib/multi.c +@@ -433,6 +433,7 @@ CURLMcode curl_multi_add_handle(struct Curl_multi *multi, + data->state.conn_cache = &data->share->conn_cache; + else + data->state.conn_cache = &multi->conn_cache; ++ data->state.lastconnect_id = -1; + + #ifdef USE_LIBPSL + /* Do the same for PSL. */ +@@ -648,11 +649,11 @@ static CURLcode multi_done(struct Curl_easy *data, + CONNCACHE_UNLOCK(data); + if(Curl_conncache_return_conn(data, conn)) { + /* remember the most recently used connection */ +- data->state.lastconnect = conn; ++ data->state.lastconnect_id = conn->connection_id; + infof(data, "%s\n", buffer); + } + else +- data->state.lastconnect = NULL; ++ data->state.lastconnect_id = -1; + } + + Curl_free_request_state(data); +@@ -663,7 +664,7 @@ static int close_connect_only(struct connectdata *conn, void *param) + { + struct Curl_easy *data = param; + +- if(data->state.lastconnect != conn) ++ if(data->state.lastconnect_id != conn->connection_id) + return 0; + + if(conn->data != data) +@@ -771,7 +772,7 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi, + /* Remove the association between the connection and the handle */ + Curl_detach_connnection(data); + +- if(data->state.lastconnect) { ++ if(data->state.lastconnect_id != -1) { + /* Mark any connect-only connection for closure */ + Curl_conncache_foreach(data, data->state.conn_cache, + data, &close_connect_only); +diff --git a/lib/url.c b/lib/url.c +index a1a6b69..2919a3d 100644 +--- a/lib/url.c ++++ b/lib/url.c +@@ -608,7 +608,7 @@ CURLcode Curl_open(struct Curl_easy **curl) + Curl_initinfo(data); + + /* most recent connection is not yet defined */ +- data->state.lastconnect = NULL; ++ data->state.lastconnect_id = -1; + + data->progress.flags |= PGRS_HIDE; + data->state.current_speed = -1; /* init to negative == impossible */ +diff --git a/lib/urldata.h b/lib/urldata.h +index f80a02d..6d8eb69 100644 +--- a/lib/urldata.h ++++ b/lib/urldata.h +@@ -1274,7 +1274,7 @@ struct UrlState { + /* buffers to store authentication data in, as parsed from input options */ + struct curltime keeps_speed; /* for the progress meter really */ + +- struct connectdata *lastconnect; /* The last connection, NULL if undefined */ ++ long lastconnect_id; /* The last connection, -1 if undefined */ + + char *headerbuff; /* allocated buffer to store headers in */ + size_t headersize; /* size of the allocation */ +-- +2.25.4 + diff --git a/curl.spec b/curl.spec index 174c9de..a64e63a 100644 --- a/curl.spec +++ b/curl.spec @@ -1,7 +1,7 @@ Summary: A utility for getting files from remote servers (FTP, HTTP, and others) Name: curl Version: 7.66.0 -Release: 2%{?dist} +Release: 3%{?dist} License: MIT Source: https://curl.haxx.se/download/%{name}-%{version}.tar.xz @@ -14,6 +14,9 @@ Patch2: 0002-curl-7.69.1-CVE-2020-8169.patch # avoid overwriting a local file with -J (CVE-2020-8177) Patch3: 0003-curl-7.69.1-CVE-2020-8177.patch +# libcurl: wrong connect-only connection (CVE-2020-8231) +Patch4: 0004-curl-7.66.0-CVE-2020-8231.patch + # patch making libcurl multilib ready Patch101: 0101-curl-7.32.0-multilib.patch @@ -183,6 +186,7 @@ be installed. %patch1 -p1 %patch2 -p1 %patch3 -p1 +%patch4 -p1 # Fedora patches %patch101 -p1 @@ -358,6 +362,9 @@ rm -f ${RPM_BUILD_ROOT}%{_libdir}/libcurl.la %{_libdir}/libcurl.so.4.[0-9].[0-9].minimal %changelog +* Wed Aug 19 2020 Kamil Dudka - 7.66.0-3 +- libcurl: wrong connect-only connection (CVE-2020-8231) + * Wed Jun 24 2020 Kamil Dudka - 7.66.0-2 - avoid overwriting a local file with -J (CVE-2020-8177) - fix partial password leak over DNS on HTTP redirect (CVE-2020-8169)