diff options
| -rw-r--r-- | CHANGES | 10 | ||||
| -rw-r--r-- | RELEASE-NOTES | 4 | ||||
| -rw-r--r-- | lib/multi.c | 137 |
3 files changed, 83 insertions, 68 deletions
@@ -6,6 +6,16 @@ Changelog +Daniel Stenberg (15 Mar 2010) +- Constantine Sapuntzakis brought a patch: + + The problem mentioned on Dec 10 2009 + (http://curl.haxx.se/bug/view.cgi?id=2905220) was only partially fixed. + Partially because an easy handle can be associated with many connections in + the cache (e.g. if there is a redirect during the lifetime of the easy + handle). The previous patch only cleaned up the first one. The new fix now + removes the easy handle from all connections, not just the first one. + Daniel Stenberg (6 Mar 2010) - Ben Greear brought a patch that fixed the rate limiting logic for TFTP when the easy interface was used. diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 2ecaf6aa0..e03fa2f39 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -31,6 +31,7 @@ This release includes the following bugfixes: o threaded resolver double free when closing curl handle o configure fixes for building with the clang compiler o easy interix rate limiting logic + o curl_multi_remove_handle() caused use after free This release includes the following known bugs: @@ -41,6 +42,7 @@ advice from friends like these: Steven M. Schweda, Yang Tse, Jack Zhang, Tom Donovan, Martin Hager, Daniel Fandrich, Patrick Monnerat, Pat Ray, Wesley Miaw, Ben Greear, - Ryan Chan, Markus Duft, Andrei Benea, Jacob Moshenko, Daniel Johnson + Ryan Chan, Markus Duft, Andrei Benea, Jacob Moshenko, Daniel Johnson, + Constantine Sapuntzakis Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/multi.c b/lib/multi.c index e4e1ce8c9..b24021280 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -181,12 +181,12 @@ struct Curl_multi { previous callback */ }; -static struct connectdata *conn_using(struct Curl_multi *multi, +static void multi_connc_remove_handle(struct Curl_multi *multi, struct SessionHandle *data); static void singlesocket(struct Curl_multi *multi, struct Curl_one_easy *easy); -static void add_closure(struct Curl_multi *multi, - struct SessionHandle *data); +static CURLMcode add_closure(struct Curl_multi *multi, + struct SessionHandle *data); static int update_timer(struct Curl_multi *multi); static CURLcode addHandleToSendOrPendPipeline(struct SessionHandle *handle, @@ -576,7 +576,6 @@ CURLMcode curl_multi_remove_handle(CURLM *multi_handle, { struct Curl_multi *multi=(struct Curl_multi *)multi_handle; struct Curl_one_easy *easy; - struct connectdata *conn; /* First, make some basic checks that the CURLM handle is a good handle */ if(!GOOD_MULTI_HANDLE(multi)) @@ -649,42 +648,9 @@ CURLMcode curl_multi_remove_handle(CURLM *multi_handle, Curl_getoff_all_pipelines(easy->easy_handle, easy->easy_conn); } - /* figure out if the easy handle is used by a connection in the cache */ - conn = conn_using(multi, easy->easy_handle); - - /* If this easy_handle was the last one in charge for one or more - connections in the shared connection cache, we might need to keep this - handle around until either A) the connection is closed and killed - properly, or B) another easy_handle uses the connection. - - The reason why we need to have a easy_handle associated with a live - connection is simply that some connections will need a handle to get - closed down properly. Currently, the only connections that need to keep - a easy_handle handle around are using FTP(S). Such connections have - the PROT_CLOSEACTION bit set. - - Thus, we need to check for all connections in the shared cache that - points to this handle and are using PROT_CLOSEACTION. If there's any, - we need to add this handle to the list of "easy handles kept around for - nice connection closures". - */ - if(conn) { - if(conn->protocol & PROT_CLOSEACTION) { - /* There's at least one CLOSEACTION connection using this handle so we - must keep this handle around. We also keep the connection cache - pointer pointing to the shared one since that will be used on close - as well. */ - easy->easy_handle->state.shared_conn = multi; - - /* this handle is still being used by a shared connection cache and - thus we leave it around for now */ - add_closure(multi, easy->easy_handle); - } - else - /* disconect the easy handle from the connection since the connection - will now remain but this easy handle is going */ - conn->data = NULL; - } + /* figure out if the easy handle is used by one or more connections in the + cache */ + multi_connc_remove_handle(multi, easy->easy_handle); if(easy->easy_handle->state.connc->type == CONNCACHE_MULTI) { /* if this was using the shared connection cache we clear the pointer @@ -2373,48 +2339,71 @@ CURLMcode curl_multi_assign(CURLM *multi_handle, return CURLM_OK; } -static struct connectdata *conn_using(struct Curl_multi *multi, +static void multi_connc_remove_handle(struct Curl_multi *multi, struct SessionHandle *data) { /* a connection in the connection cache pointing to the given 'data' ? */ int i; for(i=0; i< multi->connc->num; i++) { - if(multi->connc->connects[i] && - (multi->connc->connects[i]->data == data)) - return multi->connc->connects[i]; - } + struct connectdata * conn = multi->connc->connects[i]; + + if(conn && conn->data == data) { + /* If this easy_handle was the last one in charge for one or more + connections in the shared connection cache, we might need to keep + this handle around until either A) the connection is closed and + killed properly, or B) another easy_handle uses the connection. + + The reason why we need to have a easy_handle associated with a live + connection is simply that some connections will need a handle to get + closed down properly. Currently, the only connections that need to + keep a easy_handle handle around are using FTP(S). Such connections + have the PROT_CLOSEACTION bit set. + + Thus, we need to check for all connections in the shared cache that + points to this handle and are using PROT_CLOSEACTION. If there's any, + we need to add this handle to the list of "easy handles kept around + for nice connection closures". + */ - return NULL; + if(conn->protocol & PROT_CLOSEACTION) { + /* this handle is still being used by a shared connection and + thus we leave it around for now */ + if(add_closure(multi, data) == CURLM_OK) + data->state.shared_conn = multi; + else { + /* out of memory - so much for graceful shutdown */ + Curl_disconnect(conn); + multi->connc->connects[i] = NULL; + } + } + else + /* disconect the easy handle from the connection since the connection + will now remain but this easy handle is going */ + conn->data = NULL; + } + } } /* Add the given data pointer to the list of 'closure handles' that are kept around only to be able to close some connections nicely - just make sure that this handle isn't already added, like for the cases when an easy handle is removed, added and removed again... */ -static void add_closure(struct Curl_multi *multi, - struct SessionHandle *data) +static CURLMcode add_closure(struct Curl_multi *multi, + struct SessionHandle *data) { - int i; - struct closure *cl = calloc(1, sizeof(struct closure)); - struct closure *p=NULL; - struct closure *n; - if(cl) { - cl->easy_handle = data; - cl->next = multi->closure; - multi->closure = cl; - } - - p = multi->closure; - cl = p->next; /* start immediately on the second since the first is the one - we just added and it is _very_ likely to actually exist - used in the cache since that's the whole purpose of adding - it to this list! */ + struct closure *cl = multi->closure; + struct closure *p = NULL; + bool add = TRUE; - /* When adding, scan through all the other currently kept handles and see if - there are any connections still referring to them and kill them if not. */ + /* Before adding, scan through all the other currently kept handles and see + if there are any connections still referring to them and kill them if + not. */ while(cl) { + struct closure *n; bool inuse = FALSE; + int i; + for(i=0; i< multi->connc->num; i++) { if(multi->connc->connects[i] && (multi->connc->connects[i]->data == cl->easy_handle)) { @@ -2436,13 +2425,27 @@ static void add_closure(struct Curl_multi *multi, else multi->closure = n; free(cl); - } - else + } else { + if(cl->easy_handle == data) + add = FALSE; + p = cl; + } cl = n; } + if (add) { + cl = calloc(1, sizeof(struct closure)); + if(!cl) + return CURLM_OUT_OF_MEMORY; + + cl->easy_handle = data; + cl->next = multi->closure; + multi->closure = cl; + } + + return CURLM_OK; } #ifdef DEBUGBUILD |