diff --git a/CHANGES b/CHANGES index be599457c..7e7f99727 100644 --- a/CHANGES +++ b/CHANGES @@ -7,6 +7,12 @@ Changelog +Daniel Stenberg (28 May 2008) +- Jeff Weber reported memory leaks with aborted SCP and SFTP transfers and + provided excellent repeat recipes. I fixed the cases I managed to reproduce + but Jeff still got some (SCP) problems even after these fixes: + http://curl.haxx.se/mail/lib-2008-05/0342.html + Daniel Stenberg (26 May 2008) - Bug report #1973352 (http://curl.haxx.se/bug/view.cgi?id=1973352) identified how the HTTP redirect following code didn't properly follow to a new URL if diff --git a/RELEASE-NOTES b/RELEASE-NOTES index c88ca2d69..7beb1d48e 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -38,6 +38,7 @@ This release includes the following bugfixes: o several curl_multi_socket() fixes o builds fine for Haiku OS o follow redirect with only a new query string + o SCP and SFTP memory leaks on aborted transfers This release includes the following known bugs: @@ -60,6 +61,6 @@ advice from friends like these: Rafa Muyo, Andre Guibert de Bruet, Brock Noland, Sandor Feldi, Stefan Krause, David Shaw, Norbert Frese, Bart Whiteley, Jean-Francois Bertrand, Ben Van Hof, Yuriy Sosov, Christopher Palow, Yang Tse, Liam Healy, Nikolai Kondrashov, - David Rosenstrauch, Andreas Faerber, Scott McCreary + David Rosenstrauch, Andreas Faerber, Scott McCreary, Jeff Weber Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/ssh.c b/lib/ssh.c index 1723171c6..2e1633a00 100644 --- a/lib/ssh.c +++ b/lib/ssh.c @@ -1698,6 +1698,20 @@ static CURLcode ssh_statemach_act(struct connectdata *conn) break; case SSH_SFTP_SHUTDOWN: + /* during times we get here due to a broken transfer and then the + sftp_handle might not have been taken down so make sure that is done + before we proceed */ + + if(sshc->sftp_handle) { + rc = libssh2_sftp_close(sshc->sftp_handle); + if(rc == LIBSSH2_ERROR_EAGAIN) { + break; + } + else if(rc < 0) { + infof(data, "Failed to close libssh2 file\n"); + } + sshc->sftp_handle = NULL; + } if(sshc->sftp_session) { rc = libssh2_sftp_shutdown(sshc->sftp_session); if(rc == LIBSSH2_ERROR_EAGAIN) { @@ -1893,6 +1907,20 @@ static CURLcode ssh_statemach_act(struct connectdata *conn) break; case SSH_SESSION_DISCONNECT: + /* during weird times when we've been prematurely aborted, the channel + is still alive when we reach this state and we MUST kill the channel + properly first */ + if(sshc->ssh_channel) { + rc = libssh2_channel_free(sshc->ssh_channel); + if(rc == LIBSSH2_ERROR_EAGAIN) { + break; + } + else if(rc < 0) { + infof(data, "Failed to free libssh2 scp subsystem\n"); + } + sshc->ssh_channel = NULL; + } + if(sshc->ssh_session) { rc = libssh2_session_disconnect(sshc->ssh_session, "Shutdown"); if(rc == LIBSSH2_ERROR_EAGAIN) { @@ -2152,23 +2180,25 @@ static CURLcode scp_disconnect(struct connectdata *conn) Curl_safefree(conn->data->state.proto.ssh); conn->data->state.proto.ssh = NULL; - state(conn, SSH_SESSION_DISCONNECT); + if(conn->proto.sshc.ssh_session) { + /* only if there's a session still around to use! */ - result = ssh_easy_statemach(conn); + state(conn, SSH_SESSION_DISCONNECT); + + result = ssh_easy_statemach(conn); + } return result; } -static CURLcode scp_done(struct connectdata *conn, CURLcode status, - bool premature) +/* generic done function for both SCP and SFTP called from their specific + done functions */ +static CURLcode ssh_done(struct connectdata *conn, CURLcode status) { CURLcode result = CURLE_OK; bool done = FALSE; - (void)premature; /* not used */ - (void)status; /* unused */ if(status == CURLE_OK) { - state(conn, SSH_SCP_DONE); /* run the state-machine */ if(conn->data->state.used_interface == Curl_if_multi) { result = ssh_multi_statemach(conn, &done); @@ -2185,12 +2215,25 @@ static CURLcode scp_done(struct connectdata *conn, CURLcode status, if(done) { struct SSHPROTO *sftp_scp = conn->data->state.proto.ssh; + Curl_safefree(sftp_scp->path); sftp_scp->path = NULL; Curl_pgrsDone(conn); } return result; +} + + +static CURLcode scp_done(struct connectdata *conn, CURLcode status, + bool premature) +{ + (void)premature; /* not used */ + + if(status == CURLE_OK) + state(conn, SSH_SCP_DONE); + + return ssh_done(conn, status); } @@ -2295,8 +2338,11 @@ static CURLcode sftp_disconnect(struct connectdata *conn) Curl_safefree(conn->data->state.proto.ssh); conn->data->state.proto.ssh = NULL; - state(conn, SSH_SFTP_SHUTDOWN); - result = ssh_easy_statemach(conn); + if(conn->proto.sshc.ssh_session) { + /* only if there's a session still around to use! */ + state(conn, SSH_SFTP_SHUTDOWN); + result = ssh_easy_statemach(conn); + } DEBUGF(infof(conn->data, "SSH DISCONNECT is done\n")); @@ -2307,12 +2353,8 @@ static CURLcode sftp_disconnect(struct connectdata *conn) static CURLcode sftp_done(struct connectdata *conn, CURLcode status, bool premature) { - CURLcode result = CURLE_OK; - bool done = FALSE; struct ssh_conn *sshc = &conn->proto.sshc; - (void)status; /* unused */ - if(status == CURLE_OK) { /* Before we shut down, see if there are any post-quote commands to send: */ if(!status && !premature && conn->data->set.postquote) { @@ -2321,25 +2363,8 @@ static CURLcode sftp_done(struct connectdata *conn, CURLcode status, } else state(conn, SSH_SFTP_CLOSE); - - /* run the state-machine */ - if(conn->data->state.used_interface == Curl_if_multi) - result = ssh_multi_statemach(conn, &done); - else { - result = ssh_easy_statemach(conn); - done = TRUE; - } } - else { - result = status; - done = TRUE; - } - - if(done) { - Curl_pgrsDone(conn); - } - - return result; + return ssh_done(conn, status); } /* return number of sent bytes */