Changelog
+Daniel Stenberg (21 Aug 2009)
+- 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
+
Kamil Dudka (13 Aug 2009)
- Changed NSS code to not ignore the value of ssl.verifyhost and produce more
verbose error messages. Originally reported at:
This release includes the following bugfixes:
o The windows makefiles work again
+ o libcurl-NSS acknowledges verifyhost
+ o SIGSEGV when pipelined pipe unexpectedly breaks
+ o data corruption issue with re-connected transfers
+ o use after free if we're completed but easy_conn not NULL (pipelined)
This release includes the following known bugs:
This release would not have looked like this without help, code, reports and
advice from friends like these:
- Karl Moerder
+ Karl Moerder, Kamil Dudka, Krister Johansen,
Thanks! (and sorry if I forgot to mention someone)
254 - Problem re-using easy handle after call to curl_multi_remove_handle
http://curl.haxx.se/mail/lib-2009-07/0249.html
-255 - debugging a crash in Curl_pgrsTime/checkPendPipeline?
- http://curl.haxx.se/mail/lib-2009-08/0066.html
-
256 - "More questions about ares behavior"
http://curl.haxx.se/mail/lib-2009-08/0012.html
70. Problem re-using easy handle after call to curl_multi_remove_handle
http://curl.haxx.se/mail/lib-2009-07/0249.html
-69. debugging a crash in Curl_pgrsTime/checkPendPipeline?
- http://curl.haxx.se/mail/lib-2009-08/0066.html
-
68. "More questions about ares behavior".
http://curl.haxx.se/mail/lib-2009-08/0012.html
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);
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' */
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;
&dophase_done);
if(CURLE_OK == easy->result) {
-
if(!dophase_done) {
/* DO was not completed in one function call, we must continue
DOING... */
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);
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)
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! */
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);
}
}
}
+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)
{
}
if(!data->req.forbidchunk && data->req.upload_chunky) {
- /* if chunked Transfer-Encoding
+ /* if chunked Transfer-Encoding
* build chunk:
*
* <HEX SIZE> CRLF
/* copy the prefix to the buffer, leaving out the NUL */
memcpy(data->req.upload_fromhere, hexbuffer, hexlen);
- /* always append ASCII CRLF to the data */
- memcpy(data->req.upload_fromhere + nread,
- endofline_network,
+ /* always append ASCII CRLF to the data */
+ memcpy(data->req.upload_fromhere + nread,
+ endofline_network,
strlen(endofline_network));
#ifdef CURL_DOES_CONVERSIONS
return res;
}
+CURLcode
+Curl_reconnect_request(struct connectdata **connp)
+{
+ CURLcode result = CURLE_OK;
+ struct connectdata *conn = *connp;
+ struct SessionHandle *data = conn->data;
+
+ /* 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 */
+
+ /*
+ * 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;
+ }
+ }
+ }
+
+ return result;
+}
+
/* Returns TRUE and sets '*url' if a request retry is wanted.
NOTE: that the *url is malloc()ed. */
int numsocks);
CURLcode Curl_readrewind(struct connectdata *conn);
CURLcode Curl_fillreadbuffer(struct connectdata *conn, int bytes, int *nreadp);
+CURLcode Curl_reconnect_request(struct connectdata **connp);
bool Curl_retry_request(struct connectdata *conn, char **url);
/* This sets up a forthcoming transfer */
/* 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)
}
}
}
+ 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) {
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)
/* 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);
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;
}
#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;
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);
}
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
/* 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)
their responses on this pipeline */
struct curl_llist *pend_pipe; /* List of pending handles on
this pipeline */
+ struct curl_llist *done_pipe; /* Handles that are finished, but
+ still reference this connectdata */
#define MAX_PIPELINE_LENGTH 5
char* master_buffer; /* The master buffer allocated on-demand;
projects / curl / commitdiff