From 9354822e09ce11fce78a45a897fe2a184565a35e Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 9 Nov 2006 21:36:18 +0000 Subject: [PATCH] Ciprian Badescu found a SIGSEGV when doing multiple TFTP transfers using the multi interface, but I could also repeat it doing multiple sequential ones with the easy interface. Using Ciprian's test case, I could fix it. --- CHANGES | 5 ++++ RELEASE-NOTES | 5 +++- lib/tftp.c | 64 ++++++++++++++++++++++++++++++++++----------------- 3 files changed, 52 insertions(+), 22 deletions(-) diff --git a/CHANGES b/CHANGES index a2313ff277..154de4b122 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,11 @@ Changelog +Daniel (9 November 2006) +- Ciprian Badescu found a SIGSEGV when doing multiple TFTP transfers using the + multi interface, but I could also repeat it doing multiple sequential ones + with the easy interface. Using Ciprian's test case, I could fix it. + Daniel (8 November 2006) - Bradford Bruce reported that when setting CURLOPT_DEBUGFUNCTION without CURLOPT_VERBOSE set to non-zero, you still got a few debug messages from the diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 64c9fee08a..68c4e99532 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -18,6 +18,8 @@ This release includes the following bugfixes: o proxy close during CONNECT authentication is now dealt with nicely o the CURLOPT_DEBUGFUNCTION was sometimes called even when CURLOPT_VERBOSE was not enabled + o multiple TFTP transfers on the same (easy or multi) handle could cause a + crash Other curl-related news: @@ -30,6 +32,7 @@ New curl mirrors: This release would not have looked like this without help, code, reports and advice from friends like these: - James Housley, Olaf Stueben, Yang Tse, Gisle Vanem, Bradford Bruce + James Housley, Olaf Stueben, Yang Tse, Gisle Vanem, Bradford Bruce, + Ciprian Badescu Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/tftp.c b/lib/tftp.c index 09998fa70c..76b248fdb6 100644 --- a/lib/tftp.c +++ b/lib/tftp.c @@ -569,10 +569,13 @@ CURLcode Curl_tftp_connect(struct connectdata *conn, bool *done) tftp_state_data_t *state; int rc; - state = conn->data->reqdata.proto.tftp = calloc(sizeof(tftp_state_data_t), 1); + state = conn->data->reqdata.proto.tftp = calloc(sizeof(tftp_state_data_t), + 1); if(!state) return CURLE_OUT_OF_MEMORY; + conn->bits.close = FALSE; /* keep it open if possible */ + state->conn = conn; state->sockfd = state->conn->sock[FIRSTSOCKET]; state->state = TFTP_STATE_START; @@ -582,24 +585,27 @@ CURLcode Curl_tftp_connect(struct connectdata *conn, bool *done) tftp_set_timeouts(state); - /* Bind to any interface, random UDP port. - * - * We once used the size of the local_addr struct as the third argument for - * bind() to better work with IPv6 or whatever size the struct could have, - * but we learned that at least Tru64, AIX and IRIX *requires* the size of - * that argument to match the exact size of a 'sockaddr_in' struct when - * running IPv4-only. - * - * Therefore we use the size from the address we connected to, which we - * assume uses the same IP version and thus hopefully this works for both - * IPv4 and IPv6... - */ - rc = bind(state->sockfd, (struct sockaddr *)&state->local_addr, - conn->ip_addr->ai_addrlen); - if(rc) { - failf(conn->data, "bind() failed; %s\n", - Curl_strerror(conn, Curl_sockerrno())); - return CURLE_COULDNT_CONNECT; + if(!conn->bits.reuse) { + /* If not reused, bind to any interface, random UDP port. If it is reused, + * this has already been done! + * + * We once used the size of the local_addr struct as the third argument for + * bind() to better work with IPv6 or whatever size the struct could have, + * but we learned that at least Tru64, AIX and IRIX *requires* the size of + * that argument to match the exact size of a 'sockaddr_in' struct when + * running IPv4-only. + * + * Therefore we use the size from the address we connected to, which we + * assume uses the same IP version and thus hopefully this works for both + * IPv4 and IPv6... + */ + rc = bind(state->sockfd, (struct sockaddr *)&state->local_addr, + conn->ip_addr->ai_addrlen); + if(rc) { + failf(conn->data, "bind() failed; %s\n", + Curl_strerror(conn, Curl_sockerrno())); + return CURLE_COULDNT_CONNECT; + } } Curl_pgrsStartNow(conn->data); @@ -620,8 +626,10 @@ CURLcode Curl_tftp_done(struct connectdata *conn, CURLcode status) { (void)status; /* unused */ +#if 0 free(conn->data->reqdata.proto.tftp); conn->data->reqdata.proto.tftp = NULL; +#endif Curl_pgrsDone(conn); return CURLE_OK; @@ -641,7 +649,8 @@ CURLcode Curl_tftp_done(struct connectdata *conn, CURLcode status) CURLcode Curl_tftp(struct connectdata *conn, bool *done) { struct SessionHandle *data = conn->data; - tftp_state_data_t *state = (tftp_state_data_t *)(conn->data->reqdata.proto.tftp); + tftp_state_data_t *state = + (tftp_state_data_t *) conn->data->reqdata.proto.tftp; tftp_event_t event; CURLcode code; int rc; @@ -649,7 +658,20 @@ CURLcode Curl_tftp(struct connectdata *conn, bool *done) socklen_t fromlen; int check_time = 0; - (void)done; /* prevent compiler warning */ + *done = TRUE; + + /* + Since connections can be re-used between SessionHandles, this might be a + connection already existing but on a fresh SessionHandle struct so we must + make sure we have a good 'struct TFTP' to play with. For new connections, + the struct TFTP is allocated and setup in the Curl_tftp_connect() function. + */ + if(!state) { + code = Curl_tftp_connect(conn, done); + if(code) + return code; + state = (tftp_state_data_t *)conn->data->reqdata.proto.tftp; + } /* Run the TFTP State Machine */ for(tftp_state_machine(state, TFTP_EVENT_INIT);