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/multi.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 102 insertions(+), 25 deletions(-) (limited to 'lib/multi.c') diff --git a/lib/multi.c b/lib/multi.c index 0f75c2d72..1099b525d 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -192,7 +192,9 @@ static int update_timer(struct Curl_multi *multi); static CURLcode addHandleToSendOrPendPipeline(struct SessionHandle *handle, struct connectdata *conn); static int checkPendPipeline(struct connectdata *conn); -static void moveHandleFromSendToRecvPipeline(struct SessionHandle *habdle, +static void moveHandleFromSendToRecvPipeline(struct SessionHandle *handle, + struct connectdata *conn); +static void moveHandleFromRecvToDonePipeline(struct SessionHandle *handle, struct connectdata *conn); static bool isHandleAtHead(struct SessionHandle *handle, struct curl_llist *pipeline); @@ -233,14 +235,16 @@ static void multistate(struct Curl_one_easy *easy, CURLMstate state) easy->state = state; #ifdef DEBUGBUILD - if(easy->state > CURLM_STATE_CONNECT && - easy->state < CURLM_STATE_COMPLETED) - connectindex = easy->easy_conn->connectindex; + if(easy->easy_conn) { + if(easy->state > CURLM_STATE_CONNECT && + easy->state < CURLM_STATE_COMPLETED) + connectindex = easy->easy_conn->connectindex; - infof(easy->easy_handle, - "STATE: %s => %s handle %p; (connection #%ld) \n", - statename[oldstate], statename[easy->state], - (char *)easy, connectindex); + infof(easy->easy_handle, + "STATE: %s => %s handle %p; (connection #%ld) \n", + statename[oldstate], statename[easy->state], + (char *)easy, connectindex); + } #endif if(state == CURLM_STATE_COMPLETED) /* changing to COMPLETED means there's one less easy handle 'alive' */ @@ -925,7 +929,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, break; } - if(easy->state > CURLM_STATE_CONNECT && + if(easy->easy_conn && easy->state > CURLM_STATE_CONNECT && easy->state < CURLM_STATE_COMPLETED) /* Make sure we set the connection's current owner */ easy->easy_conn->data = easy->easy_handle; @@ -1149,7 +1153,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, &dophase_done); if(CURLE_OK == easy->result) { - if(!dophase_done) { /* DO was not completed in one function call, we must continue DOING... */ @@ -1170,6 +1173,49 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, result = CURLM_CALL_MULTI_PERFORM; } } + else if ((CURLE_SEND_ERROR == easy->result) && + easy->easy_conn->bits.reuse) { + /* + * In this situation, a connection that we were trying to use + * may have unexpectedly died. If possible, send the connection + * back to the CONNECT phase so we can try again. + */ + char *newurl; + followtype follow=FOLLOW_NONE; + CURLcode drc; + bool retry = Curl_retry_request(easy->easy_conn, &newurl); + + Curl_posttransfer(easy->easy_handle); + drc = Curl_done(&easy->easy_conn, easy->result, FALSE); + + /* When set to retry the connection, we must to go back to + * the CONNECT state */ + if(retry) { + if ((drc == CURLE_OK) || (drc == CURLE_SEND_ERROR)) { + follow = FOLLOW_RETRY; + drc = Curl_follow(easy->easy_handle, newurl, follow); + if(drc == CURLE_OK) { + multistate(easy, CURLM_STATE_CONNECT); + result = CURLM_CALL_MULTI_PERFORM; + easy->result = CURLE_OK; + } + else { + /* Follow failed */ + easy->result = drc; + free(newurl); + } + } + else { + /* done didn't return OK or SEND_ERROR */ + easy->result = drc; + free(newurl); + } + } + else { + /* Have error handler disconnect conn if we can't retry */ + disconnect_conn = TRUE; + } + } else { /* failure detected */ Curl_posttransfer(easy->easy_handle); @@ -1331,8 +1377,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, Curl_posttransfer(easy->easy_handle); /* we're no longer receving */ - Curl_removeHandleFromPipeline(easy->easy_handle, - easy->easy_conn->recv_pipe); + moveHandleFromRecvToDonePipeline(easy->easy_handle, + easy->easy_conn); /* expire the new receiving pipeline head */ if(easy->easy_conn->recv_pipe->head) @@ -1386,21 +1432,35 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, break; case CURLM_STATE_DONE: - /* Remove ourselves from the receive pipeline */ - Curl_removeHandleFromPipeline(easy->easy_handle, - easy->easy_conn->recv_pipe); - /* Check if we can move pending requests to send pipe */ - checkPendPipeline(easy->easy_conn); - if(easy->easy_conn->bits.stream_was_rewound) { - /* This request read past its response boundary so we quickly let the - other requests consume those bytes since there is no guarantee that - the socket will become active again */ - result = CURLM_CALL_MULTI_PERFORM; - } + if(easy->easy_conn) { + /* Remove ourselves from the receive and done pipelines. Handle + should be on one of these lists, depending upon how we got here. */ + Curl_removeHandleFromPipeline(easy->easy_handle, + easy->easy_conn->recv_pipe); + Curl_removeHandleFromPipeline(easy->easy_handle, + easy->easy_conn->done_pipe); + /* Check if we can move pending requests to send pipe */ + checkPendPipeline(easy->easy_conn); + + if(easy->easy_conn->bits.stream_was_rewound) { + /* This request read past its response boundary so we quickly let + the other requests consume those bytes since there is no + guarantee that the socket will become active again */ + result = CURLM_CALL_MULTI_PERFORM; + } - /* post-transfer command */ - easy->result = Curl_done(&easy->easy_conn, CURLE_OK, FALSE); + /* post-transfer command */ + easy->result = Curl_done(&easy->easy_conn, CURLE_OK, FALSE); + /* + * If there are other handles on the pipeline, Curl_done won't set + * easy_conn to NULL. In such a case, curl_multi_remove_handle() can + * access free'd data, if the connection is free'd and the handle + * removed before we perform the processing in CURLM_STATE_COMPLETED + */ + if (easy->easy_conn) + easy->easy_conn = NULL; + } /* after we have DONE what we're supposed to do, go COMPLETED, and it doesn't matter what the Curl_done() returned! */ @@ -1443,6 +1503,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, easy->easy_conn->send_pipe); Curl_removeHandleFromPipeline(easy->easy_handle, easy->easy_conn->recv_pipe); + Curl_removeHandleFromPipeline(easy->easy_handle, + easy->easy_conn->done_pipe); /* Check if we can move pending requests to send pipe */ checkPendPipeline(easy->easy_conn); } @@ -2173,6 +2235,21 @@ static void moveHandleFromSendToRecvPipeline(struct SessionHandle *handle, } } +static void moveHandleFromRecvToDonePipeline(struct SessionHandle *handle, + struct connectdata *conn) +{ + struct curl_llist_element *curr; + + curr = conn->recv_pipe->head; + while(curr) { + if(curr->ptr == handle) { + Curl_llist_move(conn->recv_pipe, curr, + conn->done_pipe, conn->done_pipe->tail); + break; + } + curr = curr->next; + } +} static bool isHandleAtHead(struct SessionHandle *handle, struct curl_llist *pipeline) { -- cgit v1.2.3