Skip to content
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
61d9b38
add hooks to debug OpenSSL memory
wfurt Apr 26, 2024
ae0b47b
opensslshim
wfurt Apr 27, 2024
3de18d1
1.x
wfurt Apr 27, 2024
32b5b22
1.0.1
wfurt Apr 29, 2024
a5ceb86
android
wfurt Apr 29, 2024
04819ad
Collections
wfurt Apr 29, 2024
a810556
unsafe
wfurt Apr 30, 2024
83795f2
build
wfurt Apr 30, 2024
04f5bde
feedback
wfurt May 11, 2024
d9bc610
update
wfurt May 11, 2024
de0ef7d
feedback
wfurt May 21, 2024
c0d8f2d
feedback
wfurt May 23, 2024
250822d
feedback
wfurt May 29, 2024
607b4d7
fix build
wfurt May 29, 2024
e29aeb1
gcc
wfurt May 29, 2024
518446c
gcc
wfurt May 29, 2024
860490a
Update src/native/libs/System.Security.Cryptography.Native/openssl.c
wfurt May 30, 2024
ebcb3d8
Move init to Interop.Crypto
rzikm Jan 16, 2025
918d337
Fix data race on GetIncrementalAllocations
rzikm Jan 17, 2025
23af46f
Use modern tuple type
rzikm Jan 17, 2025
36cd612
Fix typo
rzikm Jan 17, 2025
b5e2208
Merge remote-tracking branch 'upstream/main' into openssl-mallloc-deb…
rzikm Jan 21, 2025
d310a31
Move functionality to separate file
rzikm Jan 21, 2025
03e0b9c
Revert unnecessary changes
rzikm Jan 21, 2025
051f01c
WIP tracking in native code
rzikm Jan 23, 2025
d972e43
Improvements
rzikm Jan 27, 2025
8af851e
Reintroduce rw lock
rzikm Jan 28, 2025
ea5cce4
Add readme
rzikm Jan 28, 2025
3924367
Fix build
rzikm Jan 28, 2025
12e2a39
Code review feedback
rzikm Jan 29, 2025
efeee1f
code review changes - cont.
rzikm Jan 29, 2025
a997064
More code review feedback
rzikm Jan 29, 2025
d8c6c71
Expose the "API" via system.security.cryptography
rzikm Jan 31, 2025
2ee91f8
Fix infinite link list traversal
rzikm Jan 31, 2025
938ada9
Fix memory accounting for realloc
rzikm Jan 31, 2025
e659676
Refactoring
rzikm Jan 31, 2025
f619b76
Improve comments
rzikm Jan 31, 2025
4ef5e7d
Improve Readme
rzikm Jan 31, 2025
e64df06
Simplify implementation
rzikm Jan 31, 2025
e9cb00d
Don't use CRYPTO_atomic_add
rzikm Jan 31, 2025
1add544
Readme fixes
rzikm Jan 31, 2025
61f7b66
Fix build
rzikm Feb 3, 2025
7b650ee
Fix compilation, attempt 2
rzikm Feb 3, 2025
7cf2283
Fix build - attemt no 3
rzikm Feb 3, 2025
201f945
Apply suggestions from code review
rzikm Feb 4, 2025
86f543b
Feedback
rzikm Feb 4, 2025
7dbede7
Merge branch 'main' into openssl-mallloc-debug-hooks
rzikm Feb 10, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Runtime.InteropServices;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using Microsoft.Win32.SafeHandles;

internal static partial class Interop
Expand Down Expand Up @@ -164,5 +168,154 @@ internal static byte[] GetDynamicBuffer<THandle>(NegativeSizeReadMethod<THandle>

return bytes;
}

[LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_GetMemoryUse")]
internal static partial int GetMemoryUse(ref int memoryUse, ref int allocationCount);

public static int GetOpenSslAllocatedMemory()
{
int used = 0;
int count = 0;
GetMemoryUse(ref used, ref count);
return used;
}

public static int GetOpenSslAllocationCount()
{
int used = 0;
int count = 0;
GetMemoryUse(ref used, ref count);
return count;
}

#pragma warning disable CA1823
private static readonly bool MemoryDebug = GetMemoryDebug();
#pragma warning restore CA1823

private static bool GetMemoryDebug()
{
string? value = Environment.GetEnvironmentVariable(Interop.OpenSsl.OpenSslDebugEnvironmentVariable);
if (int.TryParse(value, CultureInfo.InvariantCulture, out int enabled) && enabled == 1)
{
Interop.Crypto.GetOpenSslAllocationCount();
Interop.Crypto.GetOpenSslAllocatedMemory();
Interop.Crypto.EnableTracking();
Interop.Crypto.GetIncrementalAllocations();
Interop.Crypto.DisableTracking();
}

return enabled == 1;
}

[LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetMemoryTracking")]
private static unsafe partial int SetMemoryTracking(delegate* unmanaged<MemoryOperation, UIntPtr, UIntPtr, int, char*, int, void> trackingCallback);

[StructLayout(LayoutKind.Sequential)]
private unsafe struct MemoryEntry
{
public int Size;
public int Line;
public char* File;
}

private enum MemoryOperation
{
Malloc = 1,
Realloc = 2,
Free = 3,
}

private static readonly unsafe nuint Offset = (nuint)sizeof(MemoryEntry);
// We only need to store the keys but we use ConcurrentDictionary to avoid locking
private static ConcurrentDictionary<UIntPtr, UIntPtr>? _allocations;

// Even though ConcurrentDictionary is thread safe, it is not guaranteed that the
// enumeration will return a point-in-time snapshot of the dictionary. It is possible
// that a single element can be concurrently:
// - removed from the dictionary (and the pointed to-memory subsequently deallocated)
// - accessed via GetIncrementalAllocations (and the pointer getting dereferenced)
//
// To avoid this, we use the *readers* role of the RW-lock to secure insertion/deletion,
// and the *writer* role to secure enumeration. This allows concurrent modifications to
// the dictionary (which is safely handled internally) while preventing the above
// mentioned race and potential crash from access violation.
private static ReaderWriterLockSlim? _allocationsLock;

[UnmanagedCallersOnly]
private static unsafe void MemoryTrackinCallback(MemoryOperation operation, UIntPtr ptr, UIntPtr oldPtr, int size, char* file, int line)
{
ref MemoryEntry entry = ref *(MemoryEntry*)ptr;

Debug.Assert(entry.File != null);
Debug.Assert(ptr != UIntPtr.Zero);

try
{
// see comment at _allocationsLock for why Readl lock is used here
_allocationsLock!.EnterReadLock();
switch (operation)
{
case MemoryOperation.Malloc:
Debug.Assert(size == entry.Size);
_allocations!.TryAdd(ptr, ptr);
break;
case MemoryOperation.Realloc:
if ((IntPtr)oldPtr != IntPtr.Zero)
{
_allocations!.TryRemove(oldPtr, out _);
}
_allocations!.TryAdd(ptr, ptr);
break;
case MemoryOperation.Free:
_allocations!.TryRemove(ptr, out _);
break;
}
}
finally
{
_allocationsLock!.ExitReadLock();
}
}

public static unsafe void EnableTracking()
{
_allocationsLock ??= new ReaderWriterLockSlim();
_allocations ??= new ConcurrentDictionary<UIntPtr, UIntPtr>();
_allocations!.Clear();
SetMemoryTracking(&MemoryTrackinCallback);
}

public static unsafe void DisableTracking()
{
SetMemoryTracking(null);
_allocations!.Clear();
}

public static unsafe (UIntPtr, int, string)[] GetIncrementalAllocations()
{
ConcurrentDictionary<UIntPtr, UIntPtr>? allocations = _allocations;

if (allocations == null || allocations.IsEmpty)
{
return Array.Empty<(UIntPtr, int, string)>();
}

try
{
// see comment at _allocationsLock for why Write lock is used here
_allocationsLock!.EnterWriteLock();

return allocations.Select(kvp =>
{
(UIntPtr ptr, _) = kvp;
ref MemoryEntry entry = ref *(MemoryEntry*)ptr;
return (ptr + Offset, entry.Size, $"{Marshal.PtrToStringAnsi((IntPtr)entry.File)}:{entry.Line}");
}).ToArray();
}
finally
{
_allocationsLock!.ExitWriteLock();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ internal static partial class OpenSsl
{
private const string TlsCacheSizeCtxName = "System.Net.Security.TlsCacheSize";
private const string TlsCacheSizeEnvironmentVariable = "DOTNET_SYSTEM_NET_SECURITY_TLSCACHESIZE";
internal const string OpenSslDebugEnvironmentVariable = "DOTNET_SYSTEM_NET_SECURITY_OPENSSL_MEMORY_DEBUG";
private const SslProtocols FakeAlpnSslProtocol = (SslProtocols)1; // used to distinguish server sessions with ALPN

private sealed class SafeSslContextCache : SafeHandleCache<SslContextCacheKey, SafeSslContextHandle> { }
Expand Down
13 changes: 13 additions & 0 deletions src/native/libs/System.Security.Cryptography.Native/apibridge_30.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@
#pragma once
#include "pal_types.h"

typedef void *(*CRYPTO_malloc_fn)(size_t num, const char *file, int line);
typedef void *(*CRYPTO_realloc_fn)(void *addr, size_t num, const char *file, int line);
typedef void (*CRYPTO_free_fn)(void *addr, const char *file, int line);

#ifndef CRYPTO_RWLOCK
typedef void CRYPTO_RWLOCK;
#endif

CRYPTO_RWLOCK *CRYPTO_THREAD_lock_new(void);
int CRYPTO_atomic_add(int *val, int amount, int *ret, CRYPTO_RWLOCK *lock);

typedef struct evp_kdf_st EVP_KDF;
typedef struct evp_kdf_ctx_st EVP_KDF_CTX;
typedef struct evp_mac_st EVP_MAC;
Expand All @@ -16,3 +27,5 @@ int local_EVP_PKEY_CTX_set_rsa_oaep_md(EVP_PKEY_CTX* ctx, const EVP_MD* md);
int local_EVP_PKEY_CTX_set_rsa_padding(EVP_PKEY_CTX* ctx, int pad_mode);
int local_EVP_PKEY_CTX_set_rsa_pss_saltlen(EVP_PKEY_CTX* ctx, int saltlen);
int local_EVP_PKEY_CTX_set_signature_md(EVP_PKEY_CTX* ctx, const EVP_MD* md);

int CRYPTO_set_mem_functions11(CRYPTO_malloc_fn malloc_fn, CRYPTO_realloc_fn realloc_fn, CRYPTO_free_fn free_fn);
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ static const Entry s_cryptoNative[] =
DllImportEntry(CryptoNative_GetECKeyParameters)
DllImportEntry(CryptoNative_GetMaxMdSize)
DllImportEntry(CryptoNative_GetMemoryBioSize)
DllImportEntry(CryptoNative_GetMemoryUse)
DllImportEntry(CryptoNative_SetMemoryTracking)
DllImportEntry(CryptoNative_GetObjectDefinitionByName)
DllImportEntry(CryptoNative_GetOcspRequestDerSize)
DllImportEntry(CryptoNative_GetPkcs7Certificates)
Expand Down
144 changes: 144 additions & 0 deletions src/native/libs/System.Security.Cryptography.Native/openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1495,6 +1495,118 @@ int32_t CryptoNative_OpenSslAvailable(void)
#endif
}

static CRYPTO_RWLOCK* g_allocLock = NULL;
static int g_allocatedMemory;
static int g_allocationCount;

static CRYPTO_allocation_cb g_memoryCallback;

struct memoryEntry
{
int size;
int line;
const char* file;
} __attribute__((aligned(8)));

static void* mallocFunction(size_t size, const char *file, int line)
{
void* ptr = malloc(size + sizeof(struct memoryEntry));
if (ptr != NULL)
{
int newCount;
CRYPTO_atomic_add(&g_allocatedMemory, (int)size, &newCount, g_allocLock);
CRYPTO_atomic_add(&g_allocationCount, 1, &newCount, g_allocLock);
struct memoryEntry* entry = (struct memoryEntry*)ptr;
entry->size = (int)size;
entry->line = line;
entry->file = file;

if (g_memoryCallback != NULL)
{
g_memoryCallback(MallocOperation, ptr, NULL, entry->size, file, line);
}
}

return (void*)((char*)ptr + sizeof(struct memoryEntry));
}

static void* reallocFunction (void *ptr, size_t size, const char *file, int line)
{
struct memoryEntry* entry;
int newCount;

if (ptr != NULL)
{
ptr = (void*)((char*)ptr - sizeof(struct memoryEntry));
entry = (struct memoryEntry*)ptr;
CRYPTO_atomic_add(&g_allocatedMemory, (int)(-entry->size), &newCount, g_allocLock);
}

void* newPtr = realloc(ptr, size + sizeof(struct memoryEntry));
if (newPtr != NULL)
{
CRYPTO_atomic_add(&g_allocatedMemory, (int)size, &newCount, g_allocLock);
CRYPTO_atomic_add(&g_allocationCount, 1, &newCount, g_allocLock);

entry = (struct memoryEntry*)newPtr;
entry->size = (int)size;
entry->line = line;
entry->file = file;

if (g_memoryCallback != NULL)
{
#if defined(__GNUC__) && __GNUC__ > 11
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wuse-after-free"
#endif
// Now try just the _majorVer added
g_memoryCallback(ReallocOperation, newPtr, ptr, entry->size, file, line);

#if defined(__GNUC__) && __GNUC__ > 11
#pragma GCC diagnostic pop
#endif
}

return (void*)((char*)newPtr + sizeof(struct memoryEntry));
}

return NULL;
}

static void freeFunction(void *ptr, const char *file, int line)
{
if (ptr != NULL)
{
int newCount;
struct memoryEntry* entry = (struct memoryEntry*)((char*)ptr - sizeof(struct memoryEntry));
CRYPTO_atomic_add(&g_allocatedMemory, (int)-entry->size, &newCount, g_allocLock);
if (g_memoryCallback != NULL)
{
g_memoryCallback(FreeOperation, entry, NULL, entry->size, file, line);
}

free(entry);
}
}

int32_t CryptoNative_GetMemoryUse(int* totalUsed, int* allocationCount)
{
if (totalUsed == NULL || allocationCount == NULL)
{
return 0;
}
*totalUsed = g_allocatedMemory;
*allocationCount = g_allocationCount;

return 1;
}

PALEXPORT int32_t CryptoNative_SetMemoryTracking(CRYPTO_allocation_cb callback)
{
g_memoryCallback = callback;
return 1;
}

static int32_t g_initStatus = 1;
int g_x509_ocsp_index = -1;
int g_ssl_sess_cert_index = -1;
Expand All @@ -1508,7 +1620,39 @@ static int32_t EnsureOpenSslInitializedCore(void)
// Otherwise call the 1.1 one.
#ifdef FEATURE_DISTRO_AGNOSTIC_SSL
InitializeOpenSSLShim();
#endif

const char* debug = getenv("DOTNET_SYSTEM_NET_SECURITY_OPENSSL_MEMORY_DEBUG");
if (debug != NULL && strcmp(debug, "1") == 0)
{
// This needs to be done before any allocation is done e.g. EnsureOpenSsl* is called.
// And it also needs to be after the pointers are loaded for DISTRO_AGNOSTIC_SSL
#ifdef FEATURE_DISTRO_AGNOSTIC_SSL
if (API_EXISTS(CRYPTO_THREAD_lock_new))
{
// This should cover 1.1.1+

CRYPTO_set_mem_functions11(mallocFunction, reallocFunction, freeFunction);
g_allocLock = CRYPTO_THREAD_lock_new();

if (!API_EXISTS(SSL_state))
{
// CRYPTO_set_mem_functions exists in OpenSSL 1.0.1 as well but it has different prototype
// and that makes it difficult to use with managed callbacks.
// Since 1.0 is long time out of support we use it only on 1.1.1+
CRYPTO_set_mem_functions11(mallocFunction, reallocFunction, freeFunction);
g_allocLock = CRYPTO_THREAD_lock_new();
}
}
#elif OPENSSL_VERSION_NUMBER >= OPENSSL_VERSION_1_1_0_RTM
// OpenSSL 1.0 has different prototypes and it is out of support so we enable this only
// on 1.1.1+
CRYPTO_set_mem_functions(mallocFunction, reallocFunction, freeFunction);
g_allocLock = CRYPTO_THREAD_lock_new();
#endif
}

#ifdef FEATURE_DISTRO_AGNOSTIC_SSL
if (API_EXISTS(SSL_state))
{
ret = EnsureOpenSsl10Initialized();
Expand Down
13 changes: 13 additions & 0 deletions src/native/libs/System.Security.Cryptography.Native/openssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,16 @@ PALEXPORT int64_t CryptoNative_OpenSslVersionNumber(void);
PALEXPORT void CryptoNative_RegisterLegacyAlgorithms(void);

PALEXPORT int32_t CryptoNative_OpenSslAvailable(void);

PALEXPORT int32_t CryptoNative_GetMemoryUse(int* totalUsed, int* allocationCount);

typedef enum
{
MallocOperation = 1,
ReallocOperation = 2,
FreeOperation = 3,
} MemoryOperation;

typedef void (*CRYPTO_allocation_cb)(MemoryOperation operation, void* ptr, void* oldPtr, int size, const char *file, int line);

PALEXPORT int32_t CryptoNative_SetMemoryTracking(CRYPTO_allocation_cb callback);
Loading
Loading