Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Libcurl leaks with mbedtls. #12584

Closed
RevaliQaQ opened this issue Dec 22, 2023 · 4 comments
Closed

Libcurl leaks with mbedtls. #12584

RevaliQaQ opened this issue Dec 22, 2023 · 4 comments
Labels
memory-leak TLS

Comments

@RevaliQaQ
Copy link

RevaliQaQ commented Dec 22, 2023 •
edited by bagder

I did this

I build libcurl whith mbedtls3.5.1 and enable ENABLE_THREADED_RESOLVER. When I called curl_easy_setopt , I found a leak when the program end. The call stack is overflow:

mbedtls-3.5.1\library\md.c (481): !mbedtls_md_setup() + 0xC bytes
mbedtls-3.5.1\library\entropy.c (336): !mbedtls_entropy_func() + 0x16 bytes
curl-8_4_0\lib\vtls\mbedtls.c (133): !entropy_func_mutex() + 0x11 bytes
mbedtls-3.5.1\library\ctr_drbg.c (352): !mbedtls_ctr_drbg_reseed_internal() + 0x25 bytes
mbedtls-3.5.1\library\ctr_drbg.c (457): !mbedtls_ctr_drbg_seed() + 0x15 bytes
curl-8_4_0\lib\vtls\mbedtls.c (341): !mbed_connect_step1() + 0x17 bytes
curl-8_4_0\lib\vtls\mbedtls.c (1093): !mbed_connect_common() + 0xD bytes
curl-8_4_0\lib\vtls\mbedtls.c (1180): !mbedtls_connect_nonblocking() + 0x13 bytes
curl-8_4_0\lib\vtls\vtls.c (375): !ssl_connect_nonblocking() + 0x19 bytes
curl-8_4_0\lib\vtls\vtls.c (1537): !ssl_cf_connect() + 0x11 bytes
curl-8_4_0\lib\cfilters.c (296): !Curl_conn_cf_connect() + 0x1D bytes
curl-8_4_0\lib\connect.c (1201): !cf_setup_connect() + 0x19 bytes
curl-8_4_0\lib\cfilters.c (296): !Curl_conn_cf_connect() + 0x1D bytes
curl-8_4_0\lib\cf-https-connect.c (135): !cf_hc_baller_connect() + 0x16 bytes
curl-8_4_0\lib\cf-https-connect.c (290): !cf_hc_connect() + 0x18 bytes
curl-8_4_0\lib\cfilters.c (351): !Curl_conn_connect() + 0x1D bytes
curl-8_4_0\lib\multi.c (2100): !multi_runsingle() + 0x11 bytes
curl-8_4_0\lib\multi.c (2740): !curl_multi_perform() + 0x11 bytes
curl-8_4_0\lib\easy.c (679): !easy_transfer() + 0xD bytes
curl-8_4_0\lib\easy.c (769): !easy_perform() + 0x22 bytes
curl-8_4_0\lib\easy.c (788): !curl_easy_perform() + 0xB bytes

I cleaned up the environment by using curl_easy_cleanup(curl), and, curl_global_cleanup();.
But leaks always exits.

When I disable ENABLE_THREADED_RESOLVER, leaks doesn't exit.

I expected the following

-Please tell me were I wrong?
-Please check out if the bug is exits.

curl/libcurl version

curl 8.5.0
mbedtls 3.5.1

operating system

Windows 11 with Visual Studio 2019 and CMAKE GUI 3.28.0
@bagder
Copy link
Member

bagder commented Dec 22, 2023

It smells like a potential mbedtls problem to me.

The threaded resolver starts a new thread in which it only resolves a host name in, then it stops the thread again. There is never any TLS or other transfer business going on in that new thread. The new thread has nothing to do with mbedtls and the TLS layer never sees nor knows about that thread. The name resolving is done entirely independent and outside of the TLS layer.

The question is probably if you then also disabled USE_THREADS_WIN32 ? Because the mbedtls code in curl has a different code path setup then.

This looks like triggered by the mbedtls_ctr_drbg_seed call, but I see nothing special mentioned in their docs that would indicate we miss a cleanup or something. Can you?

@RevaliQaQ
Copy link
Author

Yes , the program default enable USE_THREADS_WIN32. The program called mbedtls_ctr_drbg_free() at last but leaks also exists.

@bagder
Copy link
Member

bagder commented Dec 22, 2023

I think it's the entropy thing that is not freed properly. Try this:

diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c
index 4734ce026..159fa6c66 100644
--- a/lib/vtls/mbedtls.c
+++ b/lib/vtls/mbedtls.c
@@ -1204,10 +1204,13 @@ static int mbedtls_init(void)
   return Curl_mbedtlsthreadlock_thread_setup();
 }
 
 static void mbedtls_cleanup(void)
 {
+#ifdef THREADING_SUPPORT
+  mbedtls_entropy_free(&ts_entropy);
+#endif /* THREADING_SUPPORT */
   (void)Curl_mbedtlsthreadlock_thread_cleanup();
 }
 
 static bool mbedtls_data_pending(struct Curl_cfilter *cf,
                                  const struct Curl_easy *data)

bagder added a commit that referenced this issue Dec 22, 2023
@bagder
mbedtls: free the entropy when threaded
7bbfcd3 
The entropy_free was never done for threaded builds, causing a small
(fixed) memory leak.

Reported-by: RevaliQaQ on github
Fixes #12584
@bagder bagder mentioned this issue Dec 22, 2023
Closed
@RevaliQaQ
Copy link
Author

RevaliQaQ commented Dec 22, 2023 •
edited

I change code and recompiled libcurl the leaks issue has been resolved. Thanks SO much.

bagder added a commit that referenced this issue Dec 22, 2023
@bagder
mbedtls: free the entropy when threaded
8b1d229 
The entropy_free was never done for threaded builds, causing a small
(fixed) memory leak.

Reported-by: RevaliQaQ on github
Fixes #12584
Closes #12585
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
memory-leak TLS
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

mbedtls: free the entropy when threaded curl/curl
2 participants
@bagder @RevaliQaQ