Daniel:

On Mon, May 26, 2008 at 11:39:26PM +0200, Daniel Stenberg wrote:
> On Mon, 26 May 2008, Daniel Stenberg wrote:
>
>> Argh, it killed test case 612. I'll have to do some more work and get
>> back.
>
> Okay, it was test 621 it broke (me and my lousy typing!), but here's my
> updated attempt and this one doesn't break anything for me and yet it fixes
> the two memory leak cases. See the attachement. I'll commit this within a
> day or two unless someone finds problems with it.

I got this memory leak from libssh2-0.18 + libcurl 7.18.1 + ssh.c patch below,
when uploading via sftp and authenticating via user, password. I do not get a
leak when authenticating via public key.

This command line shows the leak:
# curl --netrc-optional -v -T "{/etc/termcap,/etc/passwd}" sftp://myurl/

Valgrind output:
==00:00:00:23.447 914== 170 bytes in 5 blocks are definitely lost in loss
record
 1 of 3
 ==00:00:00:23.447 914== at 0x401B504: malloc (vg_replace_malloc.c:149)
 ==00:00:00:23.447 914== by 0x404E2A3: curl_domalloc (memdebug.c:136)
 ==00:00:00:23.448 914== by 0x405B1C8: libssh2_malloc (ssh.c:281)
 ==00:00:00:23.448 914== by 0x41F607E: libssh2_packet_read (transport.c:426)
 ==00:00:00:23.448 914== by 0x41E79D8: libssh2_packet_requirev_ex
 (packet.c:12
 15)
 ==00:00:00:23.448 914== by 0x41F4E30:
 libssh2_userauth_keyboard_interactive_e
 x (userauth.c:1234)
 ==00:00:00:23.448 914== by 0x405BD15: ssh_statemach_act (ssh.c:694)
 ==00:00:00:23.448 914== by 0x405EDD3: ssh_easy_statemach (ssh.c:1987)
 ==00:00:00:23.448 914== by 0x405EF68: ssh_connect (ssh.c:2073)
 ==00:00:00:23.448 914== by 0x404077D: Curl_protocol_connect (url.c:2795)
 ==00:00:00:23.448 914== by 0x4042AE8: SetupConnection (url.c:4281)
 ==00:00:00:23.448 914== by 0x4042C2A: Curl_connect (url.c:4355)
 ==00:00:00:23.448 914== by 0x404CD0A: connect_host (transfer.c:2275)
 ==00:00:00:23.449 914== by 0x404CEAD: Curl_perform (transfer.c:2354)
 ==00:00:00:23.449 914== by 0x404D884: curl_easy_perform (easy.c:509)
 ==00:00:00:23.449 914== by 0x805156D: operate (main.c:4684)
 ==00:00:00:23.449 914== by 0x8051DA8: main (main.c:4992)

curl debug output:
* About to connect() to myhost port 22 (#0)
* Trying xxx.xxx.xxx.xxx... connected
* Connected to myhost (xxx.xxx.xxx.xxx) port 22 (#0)
* SFTP 0x4368f88 state change from SSH_STOP to SSH_S_STARTUP
* SFTP 0x4368f88 state change from SSH_S_STARTUP to SSH_AUTHLIST
* SSH authentication methods available: publickey,keyboard-interactive
* SFTP 0x4368f88 state change from SSH_AUTHLIST to SSH_AUTH_PKEY_INIT
* Using ssh public key file /root/.ssh/id_dsa.pub
* Using ssh private key file /root/.ssh/id_dsa
* SFTP 0x4368f88 state change from SSH_AUTH_PKEY_INIT to SSH_AUTH_PKEY
* SSH public key authentication failed: Unable to open public key file
* SFTP 0x4368f88 state change from SSH_AUTH_PKEY to SSH_AUTH_PASS_INIT
* SFTP 0x4368f88 state change from SSH_AUTH_PASS_INIT to SSH_AUTH_HOST_INIT
* SFTP 0x4368f88 state change from SSH_AUTH_HOST_INIT to SSH_AUTH_KEY_INIT
* SFTP 0x4368f88 state change from SSH_AUTH_KEY_INIT to SSH_AUTH_KEY
* Initialized keyboard interactive authentication
* SFTP 0x4368f88 state change from SSH_AUTH_KEY to SSH_AUTH_DONE
* Authentication complete
* SFTP 0x4368f88 state change from SSH_AUTH_DONE to SSH_SFTP_INIT
* SFTP 0x4368f88 state change from SSH_SFTP_INIT to SSH_SFTP_REALPATH
* SSH CONNECT phase done
* SFTP 0x4368f88 state change from SSH_SFTP_REALPATH to SSH_STOP
* DO phase starts
* SFTP 0x4368f88 state change from SSH_STOP to SSH_SFTP_QUOTE_INIT
* SFTP 0x4368f88 state change from SSH_SFTP_QUOTE_INIT to SSH_SFTP_TRANS_INIT
* SFTP 0x4368f88 state change from SSH_SFTP_TRANS_INIT to SSH_SFTP_UPLOAD_INIT
* SFTP 0x4368f88 state change from SSH_SFTP_UPLOAD_INIT to SSH_STOP
* DO phase is complete
} [data not shown]
  % Total % Received % Xferd Average Speed Time Time Time Current
                                 Dload Upload Total Spent Left Speed
  0 8969 0 0 0 0 0 0 --:--:-- 0:00:13 --:--:-- 0*
SFTP 0x4368f88 state change from SSH_STOP to SSH_SFTP_CLOSE
* SFTP DONE done
* SFTP 0x4368f88 state change from SSH_SFTP_CLOSE to SSH_STOP
100 8969 0 0 100 8969 0 617 0:00:14 0:00:14 --:--:-- 617*
Connection #0 to host myhost left intact

* Re-using existing connection! (#0) with host myhost
* Connected to myhost (xxx.xxx.xxx.xxx) port 22 (#0)
* DO phase starts
* SFTP 0x4368f88 state change from SSH_STOP to SSH_SFTP_QUOTE_INIT
* SFTP 0x4368f88 state change from SSH_SFTP_QUOTE_INIT to SSH_SFTP_TRANS_INIT
* SFTP 0x4368f88 state change from SSH_SFTP_TRANS_INIT to SSH_SFTP_UPLOAD_INIT
* SFTP 0x4368f88 state change from SSH_SFTP_UPLOAD_INIT to SSH_STOP
* DO phase is complete
} [data not shown]
  0 696 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0*
SFTP 0x4368f88 state change from SSH_STOP to SSH_SFTP_CLOSE
* SFTP DONE done
* SFTP 0x4368f88 state change from SSH_SFTP_CLOSE to SSH_STOP
100 696 0 0 100 696 0 1482 --:--:-- --:--:-- --:--:-- 1482*
Connection #0 to host myhost left intact

* SSH DISCONNECT starts now
* SFTP 0x4368f88 state change from SSH_STOP to SSH_SFTP_SHUTDOWN
* SFTP 0x4368f88 state change from SSH_SFTP_SHUTDOWN to SSH_SESSION_DISCONNECT
* SFTP 0x4368f88 state change from SSH_SESSION_DISCONNECT to SSH_SESSION_FREE
* SFTP 0x4368f88 state change from SSH_SESSION_FREE to SSH_STOP
* SSH DISCONNECT is done
* Closing connection #0

    Jeff
>
> --
>
> / daniel.haxx.se

> Index: lib/ssh.c
> ===================================================================
> RCS file: /cvsroot/curl/curl/lib/ssh.c,v
> retrieving revision 1.101
> diff -u -r1.101 ssh.c
> --- lib/ssh.c 21 May 2008 21:08:31 -0000 1.101
> +++ lib/ssh.c 26 May 2008 21:37:11 -0000
> @@ -1698,6 +1698,20 @@
> 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 @@
> 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 @@
> 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! */
> +
> + state(conn, SSH_SESSION_DISCONNECT);
>
> - result = ssh_easy_statemach(conn);
> + 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 @@
>
> 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 @@
> 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_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 @@
> }
> 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 */

“This e-mail message and any attachments are confidential and may be privileged.
If you are not the intended recipient please notify American Superconductor Corporation
immediately by replying to this message or by sending a message to postmaster_at_amsc.com
and destroy all copies of this message and any attachments.
Thank you.”
Received on 2008-06-09