libssh2: Connection is not closed with CURLOPT_TIMEOUT

[ccbenny]

This setup:

• We are using libcurl in our in-house software.
• The code uses SFTP with libssh2 via the curl_easy interface.
• We are setting the transfer timeout using CURLOPT_TIMEOUT.

Problem:
We observe a serious memory leak. I have tracked it down to libssh2_session_free never being called.

• Our code calls curl_easy_cleanup.
• We get to the code in libssh2.c:ssh_statemach_act that handles SSH_SFTP_SHUTDOWN.
• This calls libssh2_sftp_shutdown and that gives EWOULDBLOCK.
• The backtrace here is:

#1  0x00007ffff7f76cc1 in ssh_block_statemach (data=0x5555555a60c8, conn=0x5555555bca08, 
    duringconnect=false) at vssh/libssh2.c:2954
#2  0x00007ffff7f77a4c in sftp_disconnect (data=0x5555555a60c8, conn=0x5555555bca08, 
    dead_connection=false) at vssh/libssh2.c:3473
#3  0x00007ffff7f54724 in Curl_disconnect (data=0x5555555a60c8, conn=0x5555555bca08, 
    dead_connection=false) at url.c:855
#4  0x00007ffff7ede9d5 in Curl_conncache_close_all_connections (connc=0x5555555babf8)
    at conncache.c:556
#5  0x00007ffff7f269e2 in curl_multi_cleanup (multi=0x5555555baaf8) at multi.c:2596
#6  0x00007ffff7f53172 in Curl_close (datap=0x7fffffffde28) at url.c:380
#7  0x00007ffff7ef08d3 in curl_easy_cleanup (data=0x0) at easy.c:742
#8  0x0000555555555ef2 in sftp_upload (url=0x7fffffffe5ba "sftp://benny@localhost:1122/tmp/testing", 
    private_key=0x7fffffffe5ec "/home/benny/Projects/cds_c/src/libs/libmecomcurl/testsrc/sshd/clientkey.rsa", request=0x7ffff79935f0 <__GI__IO_fread>, request_size=0, request_userp=0x55555559ed20)
    at sftpcurl.c:159
#9  0x0000555555555ff1 in test_curl (url=0x7fffffffe5ba "sftp://benny@localhost:1122/tmp/testing", 
    requestFileName=0x7fffffffe5e2 "/dev/null", 
    key=0x7fffffffe5ec "/home/benny/Projects/cds_c/src/libs/libmecomcurl/testsrc/sshd/clientkey.rsa", 
    debug=0) at tSftp.c:58
#10 0x00005555555560a8 in test_curl_memory_leak (count=12, 
    url=0x7fffffffe5ba "sftp://benny@localhost:1122/tmp/testing", 
--Type <RET> for more, q to quit, c to continue without paging--c
    ull", key=0x7fffffffe5ec "/home/benny/Projects/cds_c/src/libs/libmecomcurl/testsrc/sshd/clientkey.rsa") at tSftp.c:83
#11 0x00005555555562db in main (argc=6, argv=0x7fffffffe2a8) at tSftp.c:149

• I.e. ssh_statemach_act is called from ssh_block_statemach.
• In ssh_block_statemach the reaction to this is that connect.c:Curl_timeleft gets called.
• Curl_timeleft fails (returns a negative number), because data->set.timeout is set to a timeout, but data->progress.t_startop is 0.
• After that ssh_block_statemach returns and all the data allocated by libssh2 is abandoned.

It seems that the CURL handle that is used in Curl_conncache_close_all_connections is not the same as the one used during the SFTP data transfer. This code probably does not expect that there is more network traffic during the disconnect handler that would need timeout handling. It is not quite clear where the correct value for data->progress.t_startop would come from. sftp_disconnect could of course just set it to the current time, but that might not be entirely correct.

A couple of additional thoughts:

• Even if the timeout issue were real, the resources allocated by libssh2 should still be freed.
• The debug function that we set with CURLOPT_DEBUGFUNCTION in the top-level handle is not set in the second CURL handle. This means that the call to failf in ssh_block_statemach does not show up anywhere.

[bagder]

libcurl and libssh2 versions ?

[Benjamin-Riefenstahl-mecom]

Both current Git master.

[Benjamin-Riefenstahl-mecom]

Also tested with current version of libssh2 1.9.0. I don't think this is a libssh2 issue.

There were other (comparatively) minor memory leaks in 1.8.0 (~ 2-4KB in my tests) with libcurl 7.64.0, but those are gone since libssh2 1.9.0.

With libcurl 7.74.0 or Git master of libcurl I have a leak of ~ 60KB now.

[bagder]

Please try my first take in #6994. I can still get (other) leaks with libssh2 but I believe those might be libssh2's fault. I'll follow up on those separately.

[ccbenny]

That works for my test case, thank you. I'm a bit worried that now we have no timeout at all for the handshake that seems to happen here. So if the connection breaks down during this last exchange, there is no parachute.

Also, can you look at the issue with CURLOPT_DEBUGFUNCTION? Or should I open another bug report for that?

[bagder]

That works for my test case, thank you. I'm a bit worried that now we have no timeout at all for the handshake that seems to happen here. So if the connection breaks down during this last exchange, there is no parachute.

Yeah not sure that the best action is here really. That's really a flaw in the libssh2 design as it doesn't allow us to just force-cleanup everything without doing anything over the socket (which that risk us getting EAGAIN back). We need to decide to either let libssh2 take its time, or shortcut that and risk a leak!

I would presume that getting eagain should only be a temporary thing and if the connection gets dropped we shouldn't get again but another error that would move us on in the state machine.

[bagder]

Also, can you look at the issue with CURLOPT_DEBUGFUNCTION? Or should I open another bug report for that?

Please file a separate issue.

[ccbenny]

Please file a separate issue.
Will do.

Thank you for the help.