From 29a1051e4901bca8a8b839455dcc1e9491486ef8 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
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 <kdudka@redhat.com>
---
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
</datacheck>
</reply>
--
2.25.4
From c3359693e17fccdf2a04f0b908bc8f51cdc38133 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
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 <kdudka@redhat.com>
---
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, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, 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, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, 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 <marc@groundctl.com>
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 <kdudka@redhat.com>
---
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
</datacheck>
</reply>
--
2.25.4
From 01148ee40dd913a169435b0f9ea90e6393821e70 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
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 <kdudka@redhat.com>
---
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