On 24/08/2023 08:38, Martin Bonner via openssl-users wrote:
I have fixed the leak by moving Openssl_thread_stop() to dll_thread_detach.
Beware! Here be dragons!
dll_thread_detach is called from DllMain when that is called with
dwReason==DLL_THREAD_DETACH.
The significance of this is that there are quite severe limitations on what you
can do inside DllMain. See:
https://learn.microsoft.com/en-us/windows/win32/dlls/dllmain and
https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-best-practices
I have two particular concerns:
* Openssl_thread_stop is not documented as being safe to call from DllMain
(it would be awesome if it was) - and although it way work today, somebody may
make a change which means it stops working tomorrow.
* I rather expect Openssl_thread_stop calls into all loaded providers - and if
your customer has a third-party provider loaded which you haven't tested with,
things could go wrong.
The only suggestion I have as to how to fix this is "submit a PR to document
that Openssl_thread_stop/Openssl_thread_stop_ex are safe to call from DllMain"
- preferably with the additions of loads of comments to the relevant code.
Apart from that, do lots of testing, and be prepared for issues.
OPENSSL_thread_stop() is used for freeing resources and I wouldn't
generally expect more complex processing to occur in it.
If running on Windows and linking to OpenSSL dynamically then
OPENSSL_thread_stop() is automatically called (the application does not
need to do anything special). OpenSSL itself does this from DllMain and
using DLL_THREAD_DETACH:
https://github.com/openssl/openssl/blob/84a149254f977f502dd2314169812fc6eae8c309/crypto/dllmain.c#L28-L44
The documentation for OPENSSL_thread_stop() does explicitly mention this:
"Resources local to a thread are deallocated automatically when the
thread exits (e.g. in a pthreads environment, when pthread_exit() is
called). On Windows platforms this is done in response to a
DLL_THREAD_DETACH message being sent to the libcrypto32.dll entry point.
Some windows functions may cause threads to exit without sending this
message (for example ExitProcess()). If the application uses such
functions, then the application must free up OpenSSL resources directly
via a call to OPENSSL_thread_stop() on each thread. Similarly this
message will also not be sent if OpenSSL is linked statically, and
therefore applications using static linking should also call
OPENSSL_thread_stop() on each thread. Additionally if OpenSSL is loaded
dynamically via LoadLibrary() and the threads are not destroyed until
after FreeLibrary() is called then each thread should call
OPENSSL_thread_stop() prior to the FreeLibrary() call."
https://www.openssl.org/docs/man3.1/man3/OPENSSL_thread_stop_ex.html
Matt
