-
Notifications
You must be signed in to change notification settings - Fork 5.1k
add hooks to debug OpenSSL memory #101626
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.
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
Changes from all commits
a163dbf
9b5b3d3
ea6ac3f
0338b06
8744c2d
401178f
803fc94
cd55d63
638df97
dd9b017
d42f271
01f556f
173827a
a86c713
c0838ea
60e933e
7c03ee3
1fe580f
c0e1a98
2fc9779
9c3da4f
ac41d7f
7f0885b
67d70ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System; | ||
using System.Collections.Concurrent; | ||
using System.Diagnostics; | ||
using System.Runtime.InteropServices; | ||
using System.Security.Cryptography.X509Certificates; | ||
|
@@ -170,5 +171,106 @@ 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; | ||
} | ||
#if DEBUG | ||
[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; | ||
|
||
[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); | ||
|
||
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; | ||
} | ||
} | ||
|
||
public static unsafe void EnableTracking() | ||
{ | ||
_allocations ??= new ConcurrentDictionary<UIntPtr, UIntPtr>(); | ||
_allocations!.Clear(); | ||
SetMemoryTracking(&MemoryTrackinCallback); | ||
} | ||
|
||
public static unsafe void DisableTracking() | ||
{ | ||
SetMemoryTracking(null); | ||
_allocations!.Clear(); | ||
} | ||
|
||
public static unsafe Tuple<UIntPtr, int, string>[] GetIncrementalAllocations() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original PR had |
||
{ | ||
if (_allocations == null || _allocations.IsEmpty) | ||
{ | ||
return Array.Empty<Tuple<UIntPtr, int, string>>(); | ||
} | ||
|
||
Tuple<UIntPtr, int, string>[] allocations = new Tuple<UIntPtr, int, string>[_allocations.Count]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was browsing PRs and accidentally stumbled upon this one. Is there a specific reason this code uses obsolete |
||
int index = 0; | ||
foreach ((UIntPtr ptr, UIntPtr value) in _allocations) | ||
{ | ||
ref MemoryEntry entry = ref *(MemoryEntry*)ptr; | ||
allocations[index] = new Tuple<UIntPtr, int, string>(ptr + Offset, entry.Size, $"{Marshal.PtrToStringAnsi((IntPtr)entry.File)}:{entry.Line}"); | ||
index++; | ||
} | ||
Comment on lines
+263
to
+270
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes. I expect the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My point is that the new size may not necessarily match the size of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, |
||
|
||
return allocations; | ||
} | ||
#endif | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,8 +23,12 @@ internal static partial class OpenSsl | |
{ | ||
private const string TlsCacheSizeCtxName = "System.Net.Security.TlsCacheSize"; | ||
private const string TlsCacheSizeEnvironmentVariable = "DOTNET_SYSTEM_NET_SECURITY_TLSCACHESIZE"; | ||
private const string OpenSslDebugEnvironmentVariable = "DOTNET_SYSTEM_NET_SECURITY_OPENSSL_MEMORY_DEBUG"; | ||
private const SslProtocols FakeAlpnSslProtocol = (SslProtocols)1; // used to distinguish server sessions with ALPN | ||
private static readonly ConcurrentDictionary<SslProtocols, SafeSslContextHandle> s_clientSslContexts = new ConcurrentDictionary<SslProtocols, SafeSslContextHandle>(); | ||
#pragma warning disable CA1823 | ||
private static readonly bool MemoryDebug = GetMemoryDebug(); | ||
#pragma warning restore CA1823 | ||
Comment on lines
+29
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the field is not used anywhere, can we move the call to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also be moved to Interop.Crypto, because following code var ci = typeof(SslStream).Assembly.GetTypes().First(t => t.Name == "Crypto");
ci.InvokeMember("EnableTracking", BindingFlags.InvokeMethod | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static, null, null, null); Will enable tracking, but later when Interop.OpenSsl gets initialized, it turns the tracking of because of Interop.Crypto.EnableTracking();
Interop.Crypto.GetIncrementalAllocations();
Interop.Crypto.DisableTracking(); in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the comment. Based on the feedback I specifically removed the option to enable the detailed tracking via environment to avoid cases when managed code may be invoked on incompatible thread. What remains is ability to subscribe for the detailed reporting later (when all the initialization is done) when caller feels it is safe. I know this part may be tricky to describe. But so far I failed to construct case where it failed e.g. all the cases I was interested in so far just worked and provided the info I was looking for. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following program will not print any memory addresses, although one would expect that it should // var ossl = typeof(SslStream).Assembly.GetTypes().First(t => t.Name == "OpenSsl");
// ossl.InvokeMember("GetMemoryDebug", BindingFlags.InvokeMethod | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static, null, null, null);
var ci = typeof(SslStream).Assembly.GetTypes().First(t => t.Name == "Crypto");
ci.InvokeMember("EnableTracking", BindingFlags.InvokeMethod | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static, null, null, null);
HttpClient client = new HttpClient();
await client.GetAsync("https://www.microsoft.com");
using Process process = Process.GetCurrentProcess();
Console.WriteLine($"Bytes known to GC [{GC.GetTotalMemory(false)}], process working set [{process.WorkingSet64}]");
Console.WriteLine("OpenSSL memory {0}", ci.InvokeMember("GetOpenSslAllocatedMemory", BindingFlags.InvokeMethod | BindingFlags.Public | BindingFlags.Static, null, null, null));
Tuple<UIntPtr, int, string>[] allocations = (Tuple<UIntPtr, int, string>[])ci.InvokeMember("GetIncrementalAllocations", BindingFlags.InvokeMethod | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance, null, null, null);
for (int j = 0; j < allocations.Length; j++)
{
(UIntPtr ptr, int size, string src) = allocations[j];
Console.WriteLine("Allocated {0} bytes at 0x{1:x} from {2}", size, ptr, src);
} It starts to work if you uncomment the first two lines. |
||
|
||
#region internal methods | ||
internal static SafeChannelBindingHandle? QueryChannelBinding(SafeSslHandle context, ChannelBindingKind bindingType) | ||
|
@@ -63,6 +67,23 @@ private static int GetCacheSize() | |
return cacheSize; | ||
} | ||
|
||
private static bool GetMemoryDebug() | ||
{ | ||
string? value = Environment.GetEnvironmentVariable(OpenSslDebugEnvironmentVariable); | ||
if (int.TryParse(value, CultureInfo.InvariantCulture, out int enabled) && enabled == 1) | ||
{ | ||
Interop.Crypto.GetOpenSslAllocationCount(); | ||
Interop.Crypto.GetOpenSslAllocatedMemory(); | ||
#if DEBUG | ||
Interop.Crypto.EnableTracking(); | ||
Interop.Crypto.GetIncrementalAllocations(); | ||
Interop.Crypto.DisableTracking(); | ||
#endif | ||
} | ||
|
||
return enabled == 1; | ||
} | ||
|
||
// This is helper function to adjust requested protocols based on CipherSuitePolicy and system capability. | ||
private static SslProtocols CalculateEffectiveProtocols(SslAuthenticationOptions sslAuthenticationOptions) | ||
{ | ||
|
Uh oh!
There was an error while loading. Please reload this page.