Calling OpenSSL_thread_stop() multiple times

[Matt Caswell]

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