From 1048043963d5487ed4d70f426a85aff07c2ee5f1 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Fri, 21 Aug 2009 07:11:20 +0000 Subject: - Lots of good work by Krister Johansen, mostly related to pipelining: Fix SIGSEGV on free'd easy_conn when pipe unexpectedly breaks Fix data corruption issue with re-connected transfers Fix use after free if we're completed but easy_conn not NULL --- lib/url.c | 88 +++++++++++++++++++++++++++------------------------------------ 1 file changed, 37 insertions(+), 51 deletions(-) (limited to 'lib/url.c') diff --git a/lib/url.c b/lib/url.c index ae2917548..011f6c4d7 100644 --- a/lib/url.c +++ b/lib/url.c @@ -146,7 +146,7 @@ void idn_free (void *ptr); /* prototype from idn-free.h, not provided by /* Local static prototypes */ static long ConnectionKillOne(struct SessionHandle *data); static void conn_free(struct connectdata *conn); -static void signalPipeClose(struct curl_llist *pipeline); +static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke); #ifdef CURL_DISABLE_VERBOSE_STRINGS #define verboseconnect(x) do { } while (0) @@ -420,6 +420,16 @@ CURLcode Curl_close(struct SessionHandle *data) } } } + pipeline = connptr->done_pipe; + if(pipeline) { + for (curr = pipeline->head; curr; curr=curr->next) { + if(data == (struct SessionHandle *) curr->ptr) { + fprintf(stderr, + "MAJOR problem we %p are still in done pipe for %p done %d\n", + data, connptr, (int)connptr->bits.done); + } + } + } pipeline = connptr->pend_pipe; if(pipeline) { for (curr = pipeline->head; curr; curr=curr->next) { @@ -2316,6 +2326,7 @@ static void conn_free(struct connectdata *conn) Curl_llist_destroy(conn->send_pipe, NULL); Curl_llist_destroy(conn->recv_pipe, NULL); Curl_llist_destroy(conn->pend_pipe, NULL); + Curl_llist_destroy(conn->done_pipe, NULL); /* possible left-overs from the async name resolvers */ #if defined(USE_ARES) @@ -2415,9 +2426,10 @@ CURLcode Curl_disconnect(struct connectdata *conn) /* Indicate to all handles on the pipe that we're dead */ if(Curl_isPipeliningEnabled(data)) { - signalPipeClose(conn->send_pipe); - signalPipeClose(conn->recv_pipe); - signalPipeClose(conn->pend_pipe); + signalPipeClose(conn->send_pipe, TRUE); + signalPipeClose(conn->recv_pipe, TRUE); + signalPipeClose(conn->pend_pipe, TRUE); + signalPipeClose(conn->done_pipe, FALSE); } conn_free(conn); @@ -2535,9 +2547,10 @@ void Curl_getoff_all_pipelines(struct SessionHandle *data, if(Curl_removeHandleFromPipeline(data, conn->send_pipe) && send_head) conn->writechannel_inuse = FALSE; Curl_removeHandleFromPipeline(data, conn->pend_pipe); + Curl_removeHandleFromPipeline(data, conn->done_pipe); } -static void signalPipeClose(struct curl_llist *pipeline) +static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke) { struct curl_llist_element *curr; @@ -2556,7 +2569,8 @@ static void signalPipeClose(struct curl_llist *pipeline) } #endif - data->state.pipe_broke = TRUE; + if (pipe_broke) + data->state.pipe_broke = TRUE; Curl_multi_handlePipeBreak(data); Curl_llist_remove(pipeline, curr, NULL); curr = next; @@ -4217,6 +4231,7 @@ static void reuse_conn(struct connectdata *old_conn, Curl_llist_destroy(old_conn->send_pipe, NULL); Curl_llist_destroy(old_conn->recv_pipe, NULL); Curl_llist_destroy(old_conn->pend_pipe, NULL); + Curl_llist_destroy(old_conn->done_pipe, NULL); Curl_safefree(old_conn->master_buffer); } @@ -4319,7 +4334,9 @@ static CURLcode create_conn(struct SessionHandle *data, conn->send_pipe = Curl_llist_alloc((curl_llist_dtor) llist_dtor); conn->recv_pipe = Curl_llist_alloc((curl_llist_dtor) llist_dtor); conn->pend_pipe = Curl_llist_alloc((curl_llist_dtor) llist_dtor); - if(!conn->send_pipe || !conn->recv_pipe || !conn->pend_pipe) + conn->done_pipe = Curl_llist_alloc((curl_llist_dtor) llist_dtor); + if(!conn->send_pipe || !conn->recv_pipe || !conn->pend_pipe || + !conn->done_pipe) return CURLE_OUT_OF_MEMORY; /* This initing continues below, see the comment "Continue connectdata @@ -5001,53 +5018,22 @@ CURLcode Curl_do(struct connectdata **connp, bool *done) /* This was formerly done in transfer.c, but we better do it here */ if((CURLE_SEND_ERROR == result) && conn->bits.reuse) { - /* This was a re-use of a connection and we got a write error in the - * DO-phase. Then we DISCONNECT this connection and have another attempt - * to CONNECT and then DO again! The retry cannot possibly find another - * connection to re-use, since we only keep one possible connection for - * each. */ - - infof(data, "Re-used connection seems dead, get a new one\n"); - - conn->bits.close = TRUE; /* enforce close of this connection */ - result = Curl_done(&conn, result, FALSE); /* we are so done with this */ - - /* conn may no longer be a good pointer */ + /* + * If the connection is using an easy handle, call reconnect + * to re-establish the connection. Otherwise, let the multi logic + * figure out how to re-establish the connection. + */ + if(!data->multi) { + result = Curl_reconnect_request(connp); - /* - * According to bug report #1330310. We need to check for - * CURLE_SEND_ERROR here as well. I figure this could happen when the - * request failed on a FTP connection and thus Curl_done() itself tried - * to use the connection (again). Slight Lack of feedback in the report, - * but I don't think this extra check can do much harm. - */ - if((CURLE_OK == result) || (CURLE_SEND_ERROR == result)) { - bool async; - bool protocol_done = TRUE; - - /* Now, redo the connect and get a new connection */ - result = Curl_connect(data, connp, &async, &protocol_done); - if(CURLE_OK == result) { - /* We have connected or sent away a name resolve query fine */ - - conn = *connp; /* setup conn to again point to something nice */ - if(async) { - /* Now, if async is TRUE here, we need to wait for the name - to resolve */ - result = Curl_wait_for_resolv(conn, NULL); - if(result) - return result; - - /* Resolved, continue with the connection */ - result = Curl_async_resolved(conn, &protocol_done); - if(result) - return result; + if(result == CURLE_OK) { + /* ... finally back to actually retry the DO phase */ + result = conn->handler->do_it(conn, done); } - - /* ... finally back to actually retry the DO phase */ - result = conn->handler->do_it(conn, done); } - } + else { + return result; + } } if((result == CURLE_OK) && *done) -- cgit v1.2.3