New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbedtls: fix memory leak when destroying SSL connection data #626

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@raporrass
Contributor

raporrass commented Feb 1, 2016

Using cURL 7.46 with mbedTLS 2.2.1 I've come across some memory leak according to Valgrind:

==93== 8,940 (1,080 direct, 7,860 indirect) bytes in 10 blocks are definitely lost in loss record 6 of 7
==93==    at 0x402B1E5: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
[...]
==93==    by 0x80C7856: rsa_alloc_wrap (pk_wrap.c:136)
==93==    by 0x80C7233: mbedtls_pk_setup (pk.c:106)
==93==    by 0x80C8CA6: mbedtls_pk_parse_key (pkparse.c:1087)
==93==    by 0x80C8F8D: mbedtls_pk_parse_keyfile (pkparse.c:134)
==93==    by 0x808AF31: mbedtls_connect_common
==93==    by 0x8089D0E: Curl_ssl_connect_nonblocking
==93==    by 0x808E16E: Curl_http_connect

To fix this, I've added some missing mbedtls_xxx_free to Curl_mbedtls_close. I've also substituted some memcpy with the proper mbedtls_xxx_init function when available.

@mention-bot

This comment has been minimized.

mention-bot commented Feb 1, 2016

By analyzing the blame information on this pull request, we identified @sasq64, @jay and @bagder to be potential reviewers

@bagder bagder added the SSL/TLS label Feb 1, 2016

@bagder

This comment has been minimized.

Member

bagder commented Feb 9, 2016

Does curl even work with mbedtls atm without this patch? I wanted to try this patch but I get a segmentation fault using git master and mbedtls 2.1.2! Seems to be related to alpn...

$ ./src/curl https://curl.haxx.se/
Segmentation fault
$ gdb --args ./src/curl https://curl.haxx.se/
[snip]
(gdb) run
Program received signal SIGSEGV, Segmentation fault.
strlen () at ../sysdeps/x86_64/strlen.S:106
106     ../sysdeps/x86_64/strlen.S: No such file or directory.
(gdb) bt
#0  strlen () at ../sysdeps/x86_64/strlen.S:106
#1  0x000000000049c144 in ssl_write_alpn_ext ()
#2  0x000000000049ce1a in ssl_write_client_hello ()
#3  0x00000000004a1848 in mbedtls_ssl_handshake_client_step ()
#4  0x000000000049837e in mbedtls_ssl_handshake_step ()
#5  0x000000000049840b in mbedtls_ssl_handshake ()
#6  0x0000000000438ae8 in mbedtls_connect_step2 (conn=0x7c6848, sockindex=0)
    at vtls/mbedtls.c:432
#7  0x00000000004396c2 in mbedtls_connect_common (conn=0x7c6848, sockindex=0, 
    nonblocking=true, done=0x7fffffffd81c) at vtls/mbedtls.c:776
@jay

This comment has been minimized.

Member

jay commented Feb 9, 2016

Does curl even work with mbedtls atm without this patch?

It works with mbedTLS, at least it does in Windows. I don't know about that version though. I'm using 2.2.1 because versions before that had some problems like ARMmbed/mbedtls#380. I've been using mbedTLS as my curl SSL backend for about 3 weeks now on my personal computers just to see how it fared. I can connect to https://curl.haxx.se/ without a problem.

curl 7.47.0-DEV (i686-w64-mingw32) libcurl/7.47.0-DEV mbedTLS/2.2.1 zlib/1.2.8 libidn/1.32 libssh2/1.6.0 nghttp2/1.6.0
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smtp smtps telnet tftp
Features: AsynchDNS Debug TrackMemory IDN IPv6 Largefile SSPI Kerberos SPNEGO NTLM SSL libz HTTP2

Do you get a crash on other sites?

@bagder

This comment has been minimized.

Member

bagder commented Feb 9, 2016

Do you get a crash on other sites?

I do. But I should get an updated version and work out this problem separately since it isn't really related to this PR. I'll file a separate issue if it remains.

@bagder bagder closed this in c62d794 Feb 9, 2016

@bagder

This comment has been minimized.

Member

bagder commented Feb 9, 2016

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment