- Andre Guibert de Bruet pointed out a missing return code check for a

strdup() that could lead to segfault if it returned NULL. I extended his
  suggest patch to now have Curl_retry_request() return a regular return code
  and better check that.
This commit is contained in:
Daniel Stenberg 2009-08-21 12:01:36 +00:00
parent 1048043963
commit 8b5102ca83
5 changed files with 40 additions and 15 deletions

View File

@ -7,6 +7,11 @@
Changelog Changelog
Daniel Stenberg (21 Aug 2009) Daniel Stenberg (21 Aug 2009)
- Andre Guibert de Bruet pointed out a missing return code check for a
strdup() that could lead to segfault if it returned NULL. I extended his
suggest patch to now have Curl_retry_request() return a regular return code
and better check that.
- Lots of good work by Krister Johansen, mostly related to pipelining: - Lots of good work by Krister Johansen, mostly related to pipelining:
Fix SIGSEGV on free'd easy_conn when pipe unexpectedly breaks Fix SIGSEGV on free'd easy_conn when pipe unexpectedly breaks

View File

@ -18,6 +18,7 @@ This release includes the following bugfixes:
o SIGSEGV when pipelined pipe unexpectedly breaks o SIGSEGV when pipelined pipe unexpectedly breaks
o data corruption issue with re-connected transfers o data corruption issue with re-connected transfers
o use after free if we're completed but easy_conn not NULL (pipelined) o use after free if we're completed but easy_conn not NULL (pipelined)
o missing strdup() return code check
This release includes the following known bugs: This release includes the following known bugs:
@ -26,6 +27,6 @@ This release includes the following known bugs:
This release would not have looked like this without help, code, reports and This release would not have looked like this without help, code, reports and
advice from friends like these: advice from friends like these:
Karl Moerder, Kamil Dudka, Krister Johansen, Karl Moerder, Kamil Dudka, Krister Johansen, Andre Guibert de Bruet
Thanks! (and sorry if I forgot to mention someone) Thanks! (and sorry if I forgot to mention someone)

View File

@ -1183,7 +1183,16 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
char *newurl; char *newurl;
followtype follow=FOLLOW_NONE; followtype follow=FOLLOW_NONE;
CURLcode drc; CURLcode drc;
bool retry = Curl_retry_request(easy->easy_conn, &newurl); bool retry = FALSE;
drc = Curl_retry_request(easy->easy_conn, &newurl);
if(drc) {
/* a failure here pretty much implies an out of memory */
easy->result = drc;
disconnect_conn = TRUE;
}
else
retry = newurl?TRUE:FALSE;
Curl_posttransfer(easy->easy_handle); Curl_posttransfer(easy->easy_handle);
drc = Curl_done(&easy->easy_conn, easy->result, FALSE); drc = Curl_done(&easy->easy_conn, easy->result, FALSE);
@ -1370,9 +1379,13 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
} }
else if(TRUE == done) { else if(TRUE == done) {
char *newurl; char *newurl;
bool retry = Curl_retry_request(easy->easy_conn, &newurl); bool retry = FALSE;
followtype follow=FOLLOW_NONE; followtype follow=FOLLOW_NONE;
easy->result = Curl_retry_request(easy->easy_conn, &newurl);
if(!easy->result)
retry = newurl?TRUE:FALSE;
/* call this even if the readwrite function returned error */ /* call this even if the readwrite function returned error */
Curl_posttransfer(easy->easy_handle); Curl_posttransfer(easy->easy_handle);
@ -1406,7 +1419,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
multistate(easy, CURLM_STATE_CONNECT); multistate(easy, CURLM_STATE_CONNECT);
result = CURLM_CALL_MULTI_PERFORM; result = CURLM_CALL_MULTI_PERFORM;
} }
else else if(newurl)
/* Since we "took it", we are in charge of freeing this on /* Since we "took it", we are in charge of freeing this on
failure */ failure */
free(newurl); free(newurl);

View File

@ -2550,19 +2550,20 @@ Curl_reconnect_request(struct connectdata **connp)
return result; return result;
} }
/* Returns TRUE and sets '*url' if a request retry is wanted. /* Returns CURLE_OK *and* sets '*url' if a request retry is wanted.
NOTE: that the *url is malloc()ed. */ NOTE: that the *url is malloc()ed. */
bool Curl_retry_request(struct connectdata *conn, CURLcode Curl_retry_request(struct connectdata *conn,
char **url) char **url)
{ {
bool retry = FALSE;
struct SessionHandle *data = conn->data; struct SessionHandle *data = conn->data;
*url = NULL;
/* if we're talking upload, we can't do the checks below, unless the protocol /* if we're talking upload, we can't do the checks below, unless the protocol
is HTTP as when uploading over HTTP we will still get a response */ is HTTP as when uploading over HTTP we will still get a response */
if(data->set.upload && !(conn->protocol&PROT_HTTP)) if(data->set.upload && !(conn->protocol&PROT_HTTP))
return retry; return CURLE_OK;
if((data->req.bytecount + if((data->req.bytecount +
data->req.headerbytecount == 0) && data->req.headerbytecount == 0) &&
@ -2574,6 +2575,8 @@ bool Curl_retry_request(struct connectdata *conn,
it again. Bad luck. Retry the same request on a fresh connect! */ it again. Bad luck. Retry the same request on a fresh connect! */
infof(conn->data, "Connection died, retrying a fresh connect\n"); infof(conn->data, "Connection died, retrying a fresh connect\n");
*url = strdup(conn->data->change.url); *url = strdup(conn->data->change.url);
if(!*url)
return CURLE_OUT_OF_MEMORY;
conn->bits.close = TRUE; /* close this connection */ conn->bits.close = TRUE; /* close this connection */
conn->bits.retry = TRUE; /* mark this as a connection we're about conn->bits.retry = TRUE; /* mark this as a connection we're about
@ -2581,10 +2584,8 @@ bool Curl_retry_request(struct connectdata *conn,
prevent i.e HTTP transfers to return prevent i.e HTTP transfers to return
error just because nothing has been error just because nothing has been
transfered! */ transfered! */
retry = TRUE;
} }
return CURLE_OK;
return retry;
} }
/* /*
@ -2629,7 +2630,12 @@ CURLcode Curl_perform(struct SessionHandle *data)
if(res == CURLE_OK) { if(res == CURLE_OK) {
res = Transfer(conn); /* now fetch that URL please */ res = Transfer(conn); /* now fetch that URL please */
if((res == CURLE_OK) || (res == CURLE_RECV_ERROR)) { if((res == CURLE_OK) || (res == CURLE_RECV_ERROR)) {
bool retry = Curl_retry_request(conn, &newurl); bool retry = FALSE;
CURLcode rc = Curl_retry_request(conn, &newurl);
if(rc)
res = rc;
else
retry = newurl?TRUE:FALSE;
if(retry) { if(retry) {
res = CURLE_OK; res = CURLE_OK;

View File

@ -7,7 +7,7 @@
* | (__| |_| | _ <| |___ * | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____| * \___|\___/|_| \_\_____|
* *
* Copyright (C) 1998 - 2008, Daniel Stenberg, <daniel@haxx.se>, et al. * Copyright (C) 1998 - 2009, Daniel Stenberg, <daniel@haxx.se>, et al.
* *
* This software is licensed as described in the file COPYING, which * This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms * you should have received as part of this distribution. The terms
@ -47,7 +47,7 @@ int Curl_single_getsock(const struct connectdata *conn,
CURLcode Curl_readrewind(struct connectdata *conn); CURLcode Curl_readrewind(struct connectdata *conn);
CURLcode Curl_fillreadbuffer(struct connectdata *conn, int bytes, int *nreadp); CURLcode Curl_fillreadbuffer(struct connectdata *conn, int bytes, int *nreadp);
CURLcode Curl_reconnect_request(struct connectdata **connp); CURLcode Curl_reconnect_request(struct connectdata **connp);
bool Curl_retry_request(struct connectdata *conn, char **url); CURLcode Curl_retry_request(struct connectdata *conn, char **url);
/* This sets up a forthcoming transfer */ /* This sets up a forthcoming transfer */
CURLcode CURLcode