Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -3461,7 +3461,7 @@ MODULE__CTYPES_DEPS=$(srcdir)/Modules/_ctypes/ctypes.h
MODULE__CTYPES_TEST_DEPS=$(srcdir)/Modules/_ctypes/_ctypes_test_generated.c.h
MODULE__CTYPES_MALLOC_CLOSURE=@MODULE__CTYPES_MALLOC_CLOSURE@
MODULE__ELEMENTTREE_DEPS=$(srcdir)/Modules/pyexpat.c @LIBEXPAT_INTERNAL@
MODULE__HASHLIB_DEPS=$(srcdir)/Modules/hashlib.h
MODULE__HASHLIB_DEPS=$(srcdir)/Modules/hashlib.h $(srcdir)/Modules/_openssl_mem.h
MODULE__IO_DEPS=$(srcdir)/Modules/_io/_iomodule.h
MODULE__REMOTE_DEBUGGING_DEPS=$(srcdir)/Modules/_remote_debugging/_remote_debugging.h $(srcdir)/Modules/_remote_debugging/gc_stats.h

Expand All @@ -3480,7 +3480,7 @@ MODULE__HMAC_DEPS=$(srcdir)/Modules/hashlib.h $(LIBHACL_HMAC_HEADERS) $(LIBHACL_
MODULE__HMAC_LDEPS=$(LIBHACL_HMAC_LIB_@LIBHACL_LDEPS_LIBTYPE@)

MODULE__SOCKET_DEPS=$(srcdir)/Modules/socketmodule.h $(srcdir)/Modules/addrinfo.h $(srcdir)/Modules/getaddrinfo.c $(srcdir)/Modules/getnameinfo.c
MODULE__SSL_DEPS=$(srcdir)/Modules/_ssl.h $(srcdir)/Modules/_ssl/cert.c $(srcdir)/Modules/_ssl/debughelpers.c $(srcdir)/Modules/_ssl/misc.c $(srcdir)/Modules/_ssl_data_111.h $(srcdir)/Modules/_ssl_data_300.h $(srcdir)/Modules/socketmodule.h
MODULE__SSL_DEPS=$(srcdir)/Modules/_ssl.h $(srcdir)/Modules/_openssl_mem.h $(srcdir)/Modules/_ssl/cert.c $(srcdir)/Modules/_ssl/debughelpers.c $(srcdir)/Modules/_ssl/misc.c $(srcdir)/Modules/_ssl_data_111.h $(srcdir)/Modules/_ssl_data_300.h $(srcdir)/Modules/socketmodule.h
MODULE__TESTCAPI_DEPS=$(srcdir)/Modules/_testcapi/parts.h $(srcdir)/Modules/_testcapi/util.h
MODULE__TESTLIMITEDCAPI_DEPS=$(srcdir)/Modules/_testlimitedcapi/testcapi_long.h $(srcdir)/Modules/_testlimitedcapi/parts.h $(srcdir)/Modules/_testlimitedcapi/util.h
MODULE__TESTINTERNALCAPI_DEPS=$(srcdir)/Modules/_testinternalcapi/parts.h $(srcdir)/Python/ceval.h $(srcdir)/Modules/_testinternalcapi/test_targets.h $(srcdir)/Modules/_testinternalcapi/test_cases.c.h
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
The :mod:`ssl` and :mod:`hashlib` modules now route OpenSSL memory
allocations through the Python raw memory allocators, making OpenSSL memory
usage visible to :mod:`tracemalloc` and to custom allocators installed with
Comment on lines +2 to +3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we can do this safely. tracemalloc_alloc calls PyGILState_Ensure() within PyMem_Raw allocations, OpenSSL allocates memory with its own internal locks held, and we call into OpenSSL in many circumstances with the GIL released.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is ongoing discussion on tracemalloc regarding this in #155725 but I don't think it causes deadlocks, in worst case it causes contention but that is the cost of using tracemalloc.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we were always consistent about releasing the GIL when calling into openssl APIs across _ssl and _hashlib it should avoid deadlocks... but I don't think we are.

And other extension modules calling libssl APIs might not and we cannot control that. So I don't think we can do this at all.

I let Claude hunt for an example where we don't... its found one and written a deadlock reproducer when using our PyMem_Raw allocators with OpenSSL - deadlocks on regular and free-threaded builds. Reproducer with explanation gist.

:c:func:`PyMem_SetAllocator`.
2 changes: 2 additions & 0 deletions Modules/_hashopenssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "pycore_strhex.h" // _Py_strhex()
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_RELAXED
#include "hashlib.h"
#include "_openssl_mem.h"

/* EVP is the preferred interface to hashing in OpenSSL */
#include <openssl/evp.h>
Expand Down Expand Up @@ -2933,5 +2934,6 @@ static struct PyModuleDef _hashlibmodule = {
PyMODINIT_FUNC
PyInit__hashlib(void)
{
_PyOpenSSL_SetupMemFunctions();
return PyModuleDef_Init(&_hashlibmodule);
}
56 changes: 56 additions & 0 deletions Modules/_openssl_mem.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Route OpenSSL allocations through the raw memory allocators.
// Shared by the _ssl and _hashlib modules.

#ifndef Py_OPENSSL_MEM_H
#define Py_OPENSSL_MEM_H

#include "Python.h"

#include <openssl/crypto.h> // CRYPTO_set_mem_functions()

// LibreSSL stubs out CRYPTO_set_mem_functions() and BoringSSL lacks it.
// AWS-LC has it, but unlike OpenSSL it does not refuse to install hooks
// after the first allocation, so earlier size-prefixed allocations would
// be freed with the wrong allocator.
#if !defined(LIBRESSL_VERSION_NUMBER) && !defined(OPENSSL_IS_BORINGSSL) \
&& !defined(OPENSSL_IS_AWSLC)
# define _Py_OPENSSL_CAN_SET_MEM_FUNCTIONS
#endif

#ifdef _Py_OPENSSL_CAN_SET_MEM_FUNCTIONS

static void *
Comment thread
picnixz marked this conversation as resolved.
_PyOpenSSL_Malloc(size_t size, const char *Py_UNUSED(file),
int Py_UNUSED(line))
{
return PyMem_RawMalloc(size);
}

static void *
_PyOpenSSL_Realloc(void *ptr, size_t size, const char *Py_UNUSED(file),
int Py_UNUSED(line))
{
return PyMem_RawRealloc(ptr, size);
}

static void
_PyOpenSSL_Free(void *ptr, const char *Py_UNUSED(file),
int Py_UNUSED(line))
{
PyMem_RawFree(ptr);
}

#endif // _Py_OPENSSL_CAN_SET_MEM_FUNCTIONS

static void
_PyOpenSSL_SetupMemFunctions(void)
{
#ifdef _Py_OPENSSL_CAN_SET_MEM_FUNCTIONS
// Fails if OpenSSL has already allocated memory (e.g. another
// libcrypto user in the process); it then keeps its current allocator.
(void)CRYPTO_set_mem_functions(_PyOpenSSL_Malloc, _PyOpenSSL_Realloc,
_PyOpenSSL_Free);
#endif
}

#endif // !Py_OPENSSL_MEM_H
2 changes: 2 additions & 0 deletions Modules/_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#endif

#include "_ssl.h"
#include "_openssl_mem.h"

/* Redefined below for Windows debug builds after important #includes */
#define _PySSL_FIX_ERRNO
Expand Down Expand Up @@ -7475,5 +7476,6 @@ static struct PyModuleDef _sslmodule_def = {
PyMODINIT_FUNC
PyInit__ssl(void)
{
_PyOpenSSL_SetupMemFunctions();
return PyModuleDef_Init(&_sslmodule_def);
}
3 changes: 3 additions & 0 deletions PCbuild/_hashlib.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@
<AdditionalDependencies>ws2_32.lib;%(AdditionalDependencies)</AdditionalDependencies>
</Link>
</ItemDefinitionGroup>
<ItemGroup>
<ClInclude Include="..\Modules\_openssl_mem.h" />
</ItemGroup>
<ItemGroup>
<ClCompile Include="..\Modules\_hashopenssl.c" />
</ItemGroup>
Expand Down
8 changes: 8 additions & 0 deletions PCbuild/_hashlib.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
<Filter Include="Source Files">
<UniqueIdentifier>{cc45963d-bd25-4eb8-bdba-a5507090bca4}</UniqueIdentifier>
</Filter>
<Filter Include="Header Files">
<UniqueIdentifier>{5abcdd3e-a8bc-4833-949c-9477609092b9}</UniqueIdentifier>
</Filter>
<Filter Include="Resource Files">
<UniqueIdentifier>{67630fa4-76e4-4035-bced-043a6df1e2e0}</UniqueIdentifier>
</Filter>
Expand All @@ -13,6 +16,11 @@
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="..\Modules\_openssl_mem.h">
<Filter>Header Files</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ResourceCompile Include="..\PC\python_nt.rc">
<Filter>Resource Files</Filter>
Expand Down
3 changes: 3 additions & 0 deletions PCbuild/_ssl.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@
<AdditionalDependencies>ws2_32.lib;crypt32.lib;%(AdditionalDependencies)</AdditionalDependencies>
</Link>
</ItemDefinitionGroup>
<ItemGroup>
<ClInclude Include="..\Modules\_openssl_mem.h" />
</ItemGroup>
<ItemGroup>
<ClCompile Include="..\Modules\_ssl.c" />
</ItemGroup>
Expand Down
8 changes: 8 additions & 0 deletions PCbuild/_ssl.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
<Filter Include="Source Files">
<UniqueIdentifier>{695348f7-e9f6-4fe1-bc03-5f08ffc8095b}</UniqueIdentifier>
</Filter>
<Filter Include="Header Files">
<UniqueIdentifier>{7c1bd5da-8912-4107-b6ac-f3930f2d90c7}</UniqueIdentifier>
</Filter>
<Filter Include="Resource Files">
<UniqueIdentifier>{1b18a2e6-040d-46c7-a9ac-ac2ec64fb5d6}</UniqueIdentifier>
</Filter>
Expand All @@ -13,6 +16,11 @@
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="..\Modules\_openssl_mem.h">
<Filter>Header Files</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ResourceCompile Include="..\PC\python_nt.rc">
<Filter>Resource Files</Filter>
Expand Down
Loading