From ce07065236293f92f3179f7b1381a046a4a5424a Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 1 Feb 2023 19:01:24 -0800 Subject: [PATCH 1/9] Add guards to zlib memory allocator --- src/native/external/zlib-intel.cmake | 9 + src/native/external/zlib.cmake | 9 + .../external/zlib/dotnet_allocator_unix.c | 162 +++++++++++++++ .../external/zlib/dotnet_allocator_win.c | 184 ++++++++++++++++++ 4 files changed, 364 insertions(+) create mode 100644 src/native/external/zlib/dotnet_allocator_unix.c create mode 100644 src/native/external/zlib/dotnet_allocator_win.c diff --git a/src/native/external/zlib-intel.cmake b/src/native/external/zlib-intel.cmake index a9d0be1fa4b05..0c156a16afa12 100644 --- a/src/native/external/zlib-intel.cmake +++ b/src/native/external/zlib-intel.cmake @@ -22,4 +22,13 @@ set(ZLIB_SOURCES_BASE zutil.c ) +# Configure our custom allocator +if(MSVC) + set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} ../zlib/dotnet_allocator_win.c) + add_compile_options($<$:/DMY_ZCALLOC>) +else() + set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} ../zlib/dotnet_allocator_unix.c) + add_definitions(-DMY_ZCALLOC) +endif() + addprefix(ZLIB_SOURCES "${CMAKE_CURRENT_LIST_DIR}/zlib-intel" "${ZLIB_SOURCES_BASE}") diff --git a/src/native/external/zlib.cmake b/src/native/external/zlib.cmake index 80b5f7b1a5438..7e68ec45dd700 100644 --- a/src/native/external/zlib.cmake +++ b/src/native/external/zlib.cmake @@ -29,4 +29,13 @@ set(ZLIB_SOURCES_BASE zutil.h ) +# Configure our custom allocator +if(MSVC) + set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} dotnet_allocator_win.c) + add_compile_options($<$:/DMY_ZCALLOC>) +else() + set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} dotnet_allocator_unix.c) + add_definitions(-DMY_ZCALLOC) +endif() + addprefix(ZLIB_SOURCES "${CMAKE_CURRENT_LIST_DIR}/zlib" "${ZLIB_SOURCES_BASE}") diff --git a/src/native/external/zlib/dotnet_allocator_unix.c b/src/native/external/zlib/dotnet_allocator_unix.c new file mode 100644 index 0000000000000..bc2b572448b58 --- /dev/null +++ b/src/native/external/zlib/dotnet_allocator_unix.c @@ -0,0 +1,162 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include +#include "zutil.h" + +/* A custom allocator for zlib that provides some defense-in-depth over standard malloc / free. + * (non-Windows version) + * + * 1. When zlib allocates fixed-length data structures for containing stream metadata, we zero + * the memory before using it, preventing use of uninitialized memory within these structures. + * Ideally we would do this for dynamically-sized buffers as well, but there is a measurable + * perf impact to doing this. Zeroing fixed structures seems like a good trade-off here, since + * these data structures contain most of the metadata used for managing the variable-length + * dynamically allocated buffers. + * + * 2. We put a cookie both before and after any allocated memory, which allows us to detect local + * buffer overruns on the call to free(). The cookie values are tied to the addresses where + * the data is located in memory. + * + * 3. We trash the aforementioned cookie on free(), which allows us to detect double-free. + * + * If any of these checks fails, the application raises SIGABRT. + */ + +#ifndef MEMORY_ALLOCATION_ALIGNMENT +// malloc() returns an address suitably aligned for any built-in data type. +// Historically, this has been twice the arch's natural word size. +#ifdef HOST_64BIT +#define MEMORY_ALLOCATION_ALIGNMENT 16 +#else +#define MEMORY_ALLOCATION_ALIGNMENT 8 +#endif +#endif + +#ifndef BOOL +#define BOOL int +#define TRUE 1 +#define FALSE 0 +#endif + +#ifndef BYTE +#define BYTE unsigned char +#endif + +typedef struct _DOTNET_ALLOC_COOKIE +{ + void* Address; + size_t Size; +} DOTNET_ALLOC_COOKIE; + +static BOOL SafeAdd(size_t a, size_t b, size_t* sum) +{ + if (SIZE_MAX - a >= b) { *sum = a + b; return TRUE; } + else { *sum = 0; return FALSE; } +} + +static BOOL SafeMult(size_t a, size_t b, size_t* product) +{ + if (SIZE_MAX / a >= b) { *product = a * b; return TRUE; } + else { *product = 0; return FALSE; } +} + +static DOTNET_ALLOC_COOKIE ReadAllocCookieUnaligned(const void* pSrc) +{ + DOTNET_ALLOC_COOKIE vCookie; + memcpy(&vCookie, pSrc, sizeof(DOTNET_ALLOC_COOKIE)); + return vCookie; +} + +static void WriteAllocCookieUnaligned(void* pDest, DOTNET_ALLOC_COOKIE vCookie) +{ + memcpy(pDest, &vCookie, sizeof(DOTNET_ALLOC_COOKIE)); +} + +// Historically, the memory allocator always returns addresses aligned to some +// particular boundary. We'll make that same guarantee here just in case somebody +// depends on it. +const size_t DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING = (sizeof(DOTNET_ALLOC_COOKIE) + MEMORY_ALLOCATION_ALIGNMENT - 1) & ~((size_t)MEMORY_ALLOCATION_ALIGNMENT - 1); +const size_t DOTNET_ALLOC_TRAILER_COOKIE_SIZE = sizeof(DOTNET_ALLOC_COOKIE); + +voidpf ZLIB_INTERNAL zcalloc (opaque, items, size) + voidpf opaque; + unsigned items; + unsigned size; +{ + (void)opaque; // unreferenced formal parameter + + // If initializing a fixed-size structure, zero the memory. + BOOL fZeroMemory = (items == 1); + + size_t cbRequested; + if (sizeof(items) + sizeof(size) <= sizeof(cbRequested)) + { + // multiplication can't overflow; no need for safeint + cbRequested = (size_t)items * (size_t)size; + } + else + { + // multiplication can overflow; go through safeint + if (!SafeMult((size_t)items, (size_t)size, &cbRequested)) { return NULL; } + } + + // Make sure the actual allocation has enough room for our frontside & backside cookies. + size_t cbActualAllocationSize; + if (!SafeAdd(cbRequested, DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING + DOTNET_ALLOC_TRAILER_COOKIE_SIZE, &cbActualAllocationSize)) { return NULL; } + + void* pAlloced = (fZeroMemory) ? calloc(1, cbActualAllocationSize) : malloc(cbActualAllocationSize); + if (pAlloced == NULL) { return NULL; } // OOM + + DOTNET_ALLOC_COOKIE* pHeaderCookie = (DOTNET_ALLOC_COOKIE*)pAlloced; + BYTE* pReturnToCaller = (BYTE*)pAlloced + DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING; + BYTE* pTrailerCookie = pReturnToCaller + cbRequested; + + // Write out the same cookie for the header & the trailer, then we're done. + + DOTNET_ALLOC_COOKIE vCookie = { 0 }; + vCookie.Address = pReturnToCaller; + vCookie.Size = cbRequested; + *pHeaderCookie = vCookie; // aligned + WriteAllocCookieUnaligned(pTrailerCookie, vCookie); + + return pReturnToCaller; +} + +static void zcfree_trash_cookie(void* pCookie) +{ + memset(pCookie, 0, sizeof(DOTNET_ALLOC_COOKIE)); +} + +void ZLIB_INTERNAL zcfree (opaque, ptr) + voidpf opaque; + voidpf ptr; +{ + (void)opaque; // unreferenced formal parameter + + if (ptr == NULL) { return; } // ok to free nullptr + + // Check cookie at beginning + + DOTNET_ALLOC_COOKIE* pHeaderCookie = (DOTNET_ALLOC_COOKIE*)((BYTE*)ptr - DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING); + if (pHeaderCookie->Address != ptr) { goto Fail; } + size_t cbRequested = pHeaderCookie->Size; + + // Check cookie at end + + BYTE* pTrailerCookie = (BYTE*)ptr + cbRequested; + DOTNET_ALLOC_COOKIE vTrailerCookie = ReadAllocCookieUnaligned(pTrailerCookie); + if (vTrailerCookie.Address != ptr) { goto Fail; } + if (vTrailerCookie.Size != cbRequested) { goto Fail; } + + // Checks passed - now trash the cookies and free memory + + zcfree_trash_cookie(pHeaderCookie); + zcfree_trash_cookie(pTrailerCookie); + + free(pHeaderCookie); + return; + +Fail: + abort(); // cookie check failed +} diff --git a/src/native/external/zlib/dotnet_allocator_win.c b/src/native/external/zlib/dotnet_allocator_win.c new file mode 100644 index 0000000000000..c4aa70e5a0e8a --- /dev/null +++ b/src/native/external/zlib/dotnet_allocator_win.c @@ -0,0 +1,184 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include +#include +#include +#include +#include "zutil.h" + +/* A custom allocator for zlib that provides some defense-in-depth over standard malloc / free. + * (Windows-specific version) + * + * 1. In 64-bit processes, we use a custom heap rather than relying on the standard process heap. + * This should cause zlib's buffers to go into a separate address range from the rest of app + * data, making it more difficult for buffer overruns to affect non-zlib-related data structures. + * + * 2. When zlib allocates fixed-length data structures for containing stream metadata, we zero + * the memory before using it, preventing use of uninitialized memory within these structures. + * Ideally we would do this for dynamically-sized buffers as well, but there is a measurable + * perf impact to doing this. Zeroing fixed structures seems like a good trade-off here, since + * these data structures contain most of the metadata used for managing the variable-length + * dynamically allocated buffers. + * + * 3. We put a cookie both before and after any allocated memory, which allows us to detect local + * buffer overruns on the call to free(). The cookie values are enciphered to make it more + * difficult for somebody to guess a correct value. + * + * 4. We trash the aforementioned cookie on free(), which allows us to detect double-free. + * + * If any of these checks fails, the application terminates immediately, optionally triggering a + * crash dump. We use a special code that's easy to search for in Watson. + */ + +// Gets the special heap we'll allocate from. +HANDLE GetZlibHeap() +{ +#ifdef _WIN64 + static HANDLE s_hPublishedHeap = NULL; + + // If already initialized, return immediately. + // We don't need a volatile read here since the publish is performed with release semantics. + if (s_hPublishedHeap != NULL) { return s_hPublishedHeap; } + + // Attempt to create a new heap. If we can't, fall back to the standard process heap. + // The heap will be dynamically sized. + BOOL fDefaultedToProcessHeap = FALSE; + HANDLE hNewHeap = HeapCreate(0, 0, 0); + + if (hNewHeap != NULL) + { + // Attempt to set the LFH flag on our new heap. Since it's just an optimization, ignore failures. + // Ref: https://learn.microsoft.com/windows/win32/api/heapapi/nf-heapapi-heapsetinformation + ULONG ulHeapInformation = 2; // LFH + HeapSetInformation(hNewHeap, HeapCompatibilityInformation, &ulHeapInformation, sizeof(ulHeapInformation)); + } + else + { + hNewHeap = GetProcessHeap(); + fDefaultedToProcessHeap = TRUE; + } + + HANDLE hExistingPublishedHeap = InterlockedCompareExchangePointer(&s_hPublishedHeap, hNewHeap, NULL); + if (hExistingPublishedHeap == NULL) + { + // We successfully published our heap handle - use it. + return hNewHeap; + } + else + { + // Another thread already created the heap handle and published it. + // We should destroy any custom heap we created and fall back to the existing published handle. + if (!fDefaultedToProcessHeap) { HeapDestroy(hNewHeap); } + return hExistingPublishedHeap; + } +#else + // We don't want to create a new heap in a 32-bit process because it could end up + // reserving too much of the address space. Instead, fall back to the normal process heap. + return GetProcessHeap(); +#endif +} + +typedef struct _DOTNET_ALLOC_COOKIE +{ + PVOID CookieValue; + union _Size + { + SIZE_T RawValue; + LPVOID EncodedValue; + } Size; +} DOTNET_ALLOC_COOKIE; + +// Historically, the Windows memory allocator always returns addresses aligned to some +// particular boundary. We'll make that same guarantee here just in case somebody +// depends on it. +const SIZE_T DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING = (sizeof(DOTNET_ALLOC_COOKIE) + MEMORY_ALLOCATION_ALIGNMENT - 1) & ~((SIZE_T)MEMORY_ALLOCATION_ALIGNMENT - 1); +const SIZE_T DOTNET_ALLOC_TRAILER_COOKIE_SIZE = sizeof(DOTNET_ALLOC_COOKIE); + +voidpf ZLIB_INTERNAL zcalloc (opaque, items, size) + voidpf opaque; + unsigned items; + unsigned size; +{ + (void)opaque; // suppress C4100 - unreferenced formal parameter + + // If initializing a fixed-size structure, zero the memory. + DWORD dwFlags = (items == 1) ? HEAP_ZERO_MEMORY : 0; + + SIZE_T cbRequested; + if (sizeof(items) + sizeof(size) <= sizeof(cbRequested)) + { + // multiplication can't overflow; no need for safeint + cbRequested = (SIZE_T)items * (SIZE_T)size; + } + else + { + // multiplication can overflow; go through safeint + if (FAILED(SIZETMult(items, size, &cbRequested))) { return NULL; } + } + + // Make sure the actual allocation has enough room for our frontside & backside cookies. + SIZE_T cbActualAllocationSize; + if (FAILED(SIZETAdd(cbRequested, DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING + DOTNET_ALLOC_TRAILER_COOKIE_SIZE, &cbActualAllocationSize))) { return NULL; } + + LPVOID pAlloced = HeapAlloc(GetZlibHeap(), dwFlags, cbActualAllocationSize); + if (pAlloced == NULL) { return NULL; } // OOM + + // Now set the header & trailer cookies + DOTNET_ALLOC_COOKIE* pHeaderCookie = (DOTNET_ALLOC_COOKIE*)pAlloced; + pHeaderCookie->CookieValue = EncodePointer(&pHeaderCookie->CookieValue); + pHeaderCookie->Size.RawValue = cbRequested; + + LPBYTE pReturnToCaller = (LPBYTE)pHeaderCookie + DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING; + + UNALIGNED DOTNET_ALLOC_COOKIE* pTrailerCookie = (UNALIGNED DOTNET_ALLOC_COOKIE*)(pReturnToCaller + cbRequested); + pTrailerCookie->CookieValue = EncodePointer(&pTrailerCookie->CookieValue); + pTrailerCookie->Size.EncodedValue = EncodePointer((PVOID)cbRequested); + + return pReturnToCaller; +} + +FORCEINLINE +void zcfree_trash_cookie(UNALIGNED DOTNET_ALLOC_COOKIE* pCookie) +{ + memset(pCookie, 0, sizeof(*pCookie)); + pCookie->CookieValue = (PVOID)(SIZE_T)0xDEADBEEF; +} + +// Marked noinline to keep it on the call stack during crash reports. +DECLSPEC_NOINLINE +DECLSPEC_NORETURN +void zcfree_cookie_check_failed() +{ + __fastfail(FAST_FAIL_HEAP_METADATA_CORRUPTION); +} + +void ZLIB_INTERNAL zcfree (opaque, ptr) + voidpf opaque; + voidpf ptr; +{ + (void)opaque; // suppress C4100 - unreferenced formal parameter + + if (ptr == NULL) { return; } // ok to free nullptr + + // Check cookie at beginning and end + + DOTNET_ALLOC_COOKIE* pHeaderCookie = (DOTNET_ALLOC_COOKIE*)((LPBYTE)ptr - DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING); + if (DecodePointer(pHeaderCookie->CookieValue) != &pHeaderCookie->CookieValue) { goto Fail; } + SIZE_T cbRequested = pHeaderCookie->Size.RawValue; + + UNALIGNED DOTNET_ALLOC_COOKIE* pTrailerCookie = (UNALIGNED DOTNET_ALLOC_COOKIE*)((LPBYTE)ptr + cbRequested); + if (DecodePointer(pTrailerCookie->CookieValue) != &pTrailerCookie->CookieValue) { goto Fail; } + if (DecodePointer(pTrailerCookie->Size.EncodedValue) != (LPVOID)cbRequested) { goto Fail; } + + // Checks passed - now trash the cookies and free memory + + zcfree_trash_cookie(pHeaderCookie); + zcfree_trash_cookie(pTrailerCookie); + + if (!HeapFree(GetZlibHeap(), 0, pHeaderCookie)) { goto Fail; } + return; + +Fail: + zcfree_cookie_check_failed(); +} From 94af6fa6137f7ac9e41cbb819ea6cb27de25cffc Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 12 Apr 2023 16:02:20 -0700 Subject: [PATCH 2/9] Fix whitespace --- src/native/external/zlib/dotnet_allocator_unix.c | 4 ++-- src/native/external/zlib/dotnet_allocator_win.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/native/external/zlib/dotnet_allocator_unix.c b/src/native/external/zlib/dotnet_allocator_unix.c index bc2b572448b58..679606a59c53f 100644 --- a/src/native/external/zlib/dotnet_allocator_unix.c +++ b/src/native/external/zlib/dotnet_allocator_unix.c @@ -79,7 +79,7 @@ static void WriteAllocCookieUnaligned(void* pDest, DOTNET_ALLOC_COOKIE vCookie) const size_t DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING = (sizeof(DOTNET_ALLOC_COOKIE) + MEMORY_ALLOCATION_ALIGNMENT - 1) & ~((size_t)MEMORY_ALLOCATION_ALIGNMENT - 1); const size_t DOTNET_ALLOC_TRAILER_COOKIE_SIZE = sizeof(DOTNET_ALLOC_COOKIE); -voidpf ZLIB_INTERNAL zcalloc (opaque, items, size) +voidpf ZLIB_INTERNAL zcalloc(opaque, items, size) voidpf opaque; unsigned items; unsigned size; @@ -128,7 +128,7 @@ static void zcfree_trash_cookie(void* pCookie) memset(pCookie, 0, sizeof(DOTNET_ALLOC_COOKIE)); } -void ZLIB_INTERNAL zcfree (opaque, ptr) +void ZLIB_INTERNAL zcfree(opaque, ptr) voidpf opaque; voidpf ptr; { diff --git a/src/native/external/zlib/dotnet_allocator_win.c b/src/native/external/zlib/dotnet_allocator_win.c index c4aa70e5a0e8a..0016cec286636 100644 --- a/src/native/external/zlib/dotnet_allocator_win.c +++ b/src/native/external/zlib/dotnet_allocator_win.c @@ -95,7 +95,7 @@ typedef struct _DOTNET_ALLOC_COOKIE const SIZE_T DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING = (sizeof(DOTNET_ALLOC_COOKIE) + MEMORY_ALLOCATION_ALIGNMENT - 1) & ~((SIZE_T)MEMORY_ALLOCATION_ALIGNMENT - 1); const SIZE_T DOTNET_ALLOC_TRAILER_COOKIE_SIZE = sizeof(DOTNET_ALLOC_COOKIE); -voidpf ZLIB_INTERNAL zcalloc (opaque, items, size) +voidpf ZLIB_INTERNAL zcalloc(opaque, items, size) voidpf opaque; unsigned items; unsigned size; @@ -153,7 +153,7 @@ void zcfree_cookie_check_failed() __fastfail(FAST_FAIL_HEAP_METADATA_CORRUPTION); } -void ZLIB_INTERNAL zcfree (opaque, ptr) +void ZLIB_INTERNAL zcfree(opaque, ptr) voidpf opaque; voidpf ptr; { From 1b7029a165877062c656fc1bb50b710a38760d86 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 12 Apr 2023 16:19:09 -0700 Subject: [PATCH 3/9] Remove heap LFH flag, simplify publishing logic --- .../external/zlib/dotnet_allocator_win.c | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/native/external/zlib/dotnet_allocator_win.c b/src/native/external/zlib/dotnet_allocator_win.c index 0016cec286636..414bf6b33056f 100644 --- a/src/native/external/zlib/dotnet_allocator_win.c +++ b/src/native/external/zlib/dotnet_allocator_win.c @@ -5,6 +5,7 @@ #include #include #include +#include /* _ASSERTE */ #include "zutil.h" /* A custom allocator for zlib that provides some defense-in-depth over standard malloc / free. @@ -41,37 +42,28 @@ HANDLE GetZlibHeap() // We don't need a volatile read here since the publish is performed with release semantics. if (s_hPublishedHeap != NULL) { return s_hPublishedHeap; } - // Attempt to create a new heap. If we can't, fall back to the standard process heap. - // The heap will be dynamically sized. - BOOL fDefaultedToProcessHeap = FALSE; + // Attempt to create a new heap. The heap will be dynamically sized. HANDLE hNewHeap = HeapCreate(0, 0, 0); if (hNewHeap != NULL) { - // Attempt to set the LFH flag on our new heap. Since it's just an optimization, ignore failures. - // Ref: https://learn.microsoft.com/windows/win32/api/heapapi/nf-heapapi-heapsetinformation - ULONG ulHeapInformation = 2; // LFH - HeapSetInformation(hNewHeap, HeapCompatibilityInformation, &ulHeapInformation, sizeof(ulHeapInformation)); + // We created a new heap. Attempt to publish it. + if (InterlockedCompareExchangePointer(&s_hPublishedHeap, hNewHeap, NULL) != NULL) + { + HeapDestroy(hNewHeap); // Somebody published before us. Destroy our heap. + hNewHeap = NULL; // Guard against accidental use later in the method. + } } else { - hNewHeap = GetProcessHeap(); - fDefaultedToProcessHeap = TRUE; + // If we can't create a new heap, fall back to the process default heap. + InterlockedCompareExchangePointer(&s_hPublishedHeap, GetProcessHeap(), NULL); } - HANDLE hExistingPublishedHeap = InterlockedCompareExchangePointer(&s_hPublishedHeap, hNewHeap, NULL); - if (hExistingPublishedHeap == NULL) - { - // We successfully published our heap handle - use it. - return hNewHeap; - } - else - { - // Another thread already created the heap handle and published it. - // We should destroy any custom heap we created and fall back to the existing published handle. - if (!fDefaultedToProcessHeap) { HeapDestroy(hNewHeap); } - return hExistingPublishedHeap; - } + // Some thread - perhaps us, perhaps somebody else - published the heap. Return it. + // We don't need a volatile read here since the publish is performed with release semantics. + _ASSERTE(s_hPublishedHeap != NULL); + return s_hPublishedHeap; #else // We don't want to create a new heap in a 32-bit process because it could end up // reserving too much of the address space. Instead, fall back to the normal process heap. From 20dc0fb8bd396e9138fd6251b862c5200e87aabf Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 12 Apr 2023 16:42:01 -0700 Subject: [PATCH 4/9] Remove dead code from cmake file --- src/native/external/zlib-intel.cmake | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/native/external/zlib-intel.cmake b/src/native/external/zlib-intel.cmake index 0c156a16afa12..a08addf8b9eee 100644 --- a/src/native/external/zlib-intel.cmake +++ b/src/native/external/zlib-intel.cmake @@ -1,7 +1,11 @@ if(MSVC) + add_compile_options($<$:/DMY_ZCALLOC>) # custom zlib allocator add_compile_options($<$:/wd4244>) # conversion from 'type1' to 'type2', possible loss of data -else(CMAKE_C_COMPILER_ID MATCHES "Clang") - add_compile_options($<$:-Wno-implicit-int-conversion>) +else() + add_definitions(-DMY_ZCALLOC) # custom zlib allocator + if(CMAKE_C_COMPILER_ID MATCHES "Clang") + add_compile_options($<$:-Wno-implicit-int-conversion>) + endif() endif() set(ZLIB_SOURCES_BASE @@ -20,15 +24,7 @@ set(ZLIB_SOURCES_BASE trees.c x86.c zutil.c + ../zlib/dotnet_allocator_win.c ) -# Configure our custom allocator -if(MSVC) - set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} ../zlib/dotnet_allocator_win.c) - add_compile_options($<$:/DMY_ZCALLOC>) -else() - set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} ../zlib/dotnet_allocator_unix.c) - add_definitions(-DMY_ZCALLOC) -endif() - addprefix(ZLIB_SOURCES "${CMAKE_CURRENT_LIST_DIR}/zlib-intel" "${ZLIB_SOURCES_BASE}") From 061a65536031eea9114777da030fe53020de9811 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 12 Apr 2023 17:17:42 -0700 Subject: [PATCH 5/9] Normalize cmake files --- src/native/external/zlib-intel.cmake | 11 +++++------ src/native/external/zlib.cmake | 7 +++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/native/external/zlib-intel.cmake b/src/native/external/zlib-intel.cmake index a08addf8b9eee..8a19f97ed098b 100644 --- a/src/native/external/zlib-intel.cmake +++ b/src/native/external/zlib-intel.cmake @@ -1,11 +1,7 @@ if(MSVC) - add_compile_options($<$:/DMY_ZCALLOC>) # custom zlib allocator add_compile_options($<$:/wd4244>) # conversion from 'type1' to 'type2', possible loss of data -else() - add_definitions(-DMY_ZCALLOC) # custom zlib allocator - if(CMAKE_C_COMPILER_ID MATCHES "Clang") - add_compile_options($<$:-Wno-implicit-int-conversion>) - endif() +else(CMAKE_C_COMPILER_ID MATCHES "Clang") + add_compile_options($<$:-Wno-implicit-int-conversion>) endif() set(ZLIB_SOURCES_BASE @@ -27,4 +23,7 @@ set(ZLIB_SOURCES_BASE ../zlib/dotnet_allocator_win.c ) +# enable custom zlib allocator +add_definitions(-DMY_ZCALLOC) + addprefix(ZLIB_SOURCES "${CMAKE_CURRENT_LIST_DIR}/zlib-intel" "${ZLIB_SOURCES_BASE}") diff --git a/src/native/external/zlib.cmake b/src/native/external/zlib.cmake index 7e68ec45dd700..16034e7767025 100644 --- a/src/native/external/zlib.cmake +++ b/src/native/external/zlib.cmake @@ -29,13 +29,12 @@ set(ZLIB_SOURCES_BASE zutil.h ) -# Configure our custom allocator -if(MSVC) +# enable custom zlib allocator +add_definitions(-DMY_ZCALLOC) +if(CLR_CMAKE_HOST_WIN32) set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} dotnet_allocator_win.c) - add_compile_options($<$:/DMY_ZCALLOC>) else() set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} dotnet_allocator_unix.c) - add_definitions(-DMY_ZCALLOC) endif() addprefix(ZLIB_SOURCES "${CMAKE_CURRENT_LIST_DIR}/zlib" "${ZLIB_SOURCES_BASE}") From cec42144ad7a31af5f824fb57b45ca8e70ac93b8 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 13 Apr 2023 12:38:30 -0700 Subject: [PATCH 6/9] Slight zlib PAL & allocator restructuring - Move allocator files under Compression.Native dir - Update pal_zlib includes, use calloc instead of malloc - Remove custom typedefs from zlib unix allocator --- src/native/external/zlib-intel.cmake | 2 +- src/native/external/zlib.cmake | 4 +-- .../CMakeLists.txt | 1 + .../System.IO.Compression.Native/pal_zlib.c | 11 +++--- .../zlib_allocator_unix.c} | 35 +++++++------------ .../zlib_allocator_win.c} | 7 +++- 6 files changed, 29 insertions(+), 31 deletions(-) rename src/native/{external/zlib/dotnet_allocator_unix.c => libs/System.IO.Compression.Native/zlib_allocator_unix.c} (86%) rename src/native/{external/zlib/dotnet_allocator_win.c => libs/System.IO.Compression.Native/zlib_allocator_win.c} (98%) diff --git a/src/native/external/zlib-intel.cmake b/src/native/external/zlib-intel.cmake index 8a19f97ed098b..1b6fa0cb4765b 100644 --- a/src/native/external/zlib-intel.cmake +++ b/src/native/external/zlib-intel.cmake @@ -20,7 +20,7 @@ set(ZLIB_SOURCES_BASE trees.c x86.c zutil.c - ../zlib/dotnet_allocator_win.c + ../../libs/System.IO.Compression.Native/zlib_allocator_win.c ) # enable custom zlib allocator diff --git a/src/native/external/zlib.cmake b/src/native/external/zlib.cmake index 16034e7767025..b08d2574df674 100644 --- a/src/native/external/zlib.cmake +++ b/src/native/external/zlib.cmake @@ -32,9 +32,9 @@ set(ZLIB_SOURCES_BASE # enable custom zlib allocator add_definitions(-DMY_ZCALLOC) if(CLR_CMAKE_HOST_WIN32) - set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} dotnet_allocator_win.c) + set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} ../../libs/System.IO.Compression.Native/zlib_allocator_win.c) else() - set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} dotnet_allocator_unix.c) + set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} ../../libs/System.IO.Compression.Native/zlib_allocator_unix.c) endif() addprefix(ZLIB_SOURCES "${CMAKE_CURRENT_LIST_DIR}/zlib" "${ZLIB_SOURCES_BASE}") diff --git a/src/native/libs/System.IO.Compression.Native/CMakeLists.txt b/src/native/libs/System.IO.Compression.Native/CMakeLists.txt index f4d2e315f4965..89e00a6f4c74c 100644 --- a/src/native/libs/System.IO.Compression.Native/CMakeLists.txt +++ b/src/native/libs/System.IO.Compression.Native/CMakeLists.txt @@ -101,6 +101,7 @@ else () if (CLR_CMAKE_HOST_ARCH_I386 OR CLR_CMAKE_HOST_ARCH_AMD64) include(${CLR_SRC_NATIVE_DIR}/external/zlib-intel.cmake) + add_definitions(-DINTERNAL_ZLIB_INTEL) else () include(${CLR_SRC_NATIVE_DIR}/external/zlib.cmake) endif () diff --git a/src/native/libs/System.IO.Compression.Native/pal_zlib.c b/src/native/libs/System.IO.Compression.Native/pal_zlib.c index f5d25d66106b0..1fd84662264a6 100644 --- a/src/native/libs/System.IO.Compression.Native/pal_zlib.c +++ b/src/native/libs/System.IO.Compression.Native/pal_zlib.c @@ -9,7 +9,11 @@ #ifdef _WIN32 #define c_static_assert(e) static_assert((e),"") #endif - #include + #ifdef INTERNAL_ZLIB_INTEL + #include + #else + #include + #endif #else #include "pal_utilities.h" #include @@ -39,14 +43,11 @@ Initializes the PAL_ZStream by creating and setting its underlying z_stream. */ static int32_t Init(PAL_ZStream* stream) { - z_stream* zStream = (z_stream*)malloc(sizeof(z_stream)); + z_stream* zStream = (z_stream*)calloc(1, sizeof(z_stream)); stream->internalState = zStream; if (zStream != NULL) { - zStream->zalloc = Z_NULL; - zStream->zfree = Z_NULL; - zStream->opaque = Z_NULL; return PAL_Z_OK; } else diff --git a/src/native/external/zlib/dotnet_allocator_unix.c b/src/native/libs/System.IO.Compression.Native/zlib_allocator_unix.c similarity index 86% rename from src/native/external/zlib/dotnet_allocator_unix.c rename to src/native/libs/System.IO.Compression.Native/zlib_allocator_unix.c index 679606a59c53f..9eb4bbf267105 100644 --- a/src/native/external/zlib/dotnet_allocator_unix.c +++ b/src/native/libs/System.IO.Compression.Native/zlib_allocator_unix.c @@ -1,8 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +#include #include -#include "zutil.h" +#include /* A custom allocator for zlib that provides some defense-in-depth over standard malloc / free. * (non-Windows version) @@ -33,32 +34,22 @@ #endif #endif -#ifndef BOOL -#define BOOL int -#define TRUE 1 -#define FALSE 0 -#endif - -#ifndef BYTE -#define BYTE unsigned char -#endif - typedef struct _DOTNET_ALLOC_COOKIE { void* Address; size_t Size; } DOTNET_ALLOC_COOKIE; -static BOOL SafeAdd(size_t a, size_t b, size_t* sum) +static bool SafeAdd(size_t a, size_t b, size_t* sum) { - if (SIZE_MAX - a >= b) { *sum = a + b; return TRUE; } - else { *sum = 0; return FALSE; } + if (SIZE_MAX - a >= b) { *sum = a + b; return true; } + else { *sum = 0; return false; } } -static BOOL SafeMult(size_t a, size_t b, size_t* product) +static bool SafeMult(size_t a, size_t b, size_t* product) { - if (SIZE_MAX / a >= b) { *product = a * b; return TRUE; } - else { *product = 0; return FALSE; } + if (SIZE_MAX / a >= b) { *product = a * b; return true; } + else { *product = 0; return false; } } static DOTNET_ALLOC_COOKIE ReadAllocCookieUnaligned(const void* pSrc) @@ -87,7 +78,7 @@ voidpf ZLIB_INTERNAL zcalloc(opaque, items, size) (void)opaque; // unreferenced formal parameter // If initializing a fixed-size structure, zero the memory. - BOOL fZeroMemory = (items == 1); + bool fZeroMemory = (items == 1); size_t cbRequested; if (sizeof(items) + sizeof(size) <= sizeof(cbRequested)) @@ -109,8 +100,8 @@ voidpf ZLIB_INTERNAL zcalloc(opaque, items, size) if (pAlloced == NULL) { return NULL; } // OOM DOTNET_ALLOC_COOKIE* pHeaderCookie = (DOTNET_ALLOC_COOKIE*)pAlloced; - BYTE* pReturnToCaller = (BYTE*)pAlloced + DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING; - BYTE* pTrailerCookie = pReturnToCaller + cbRequested; + uint8_t* pReturnToCaller = (uint8_t*)pAlloced + DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING; + uint8_t* pTrailerCookie = pReturnToCaller + cbRequested; // Write out the same cookie for the header & the trailer, then we're done. @@ -138,13 +129,13 @@ void ZLIB_INTERNAL zcfree(opaque, ptr) // Check cookie at beginning - DOTNET_ALLOC_COOKIE* pHeaderCookie = (DOTNET_ALLOC_COOKIE*)((BYTE*)ptr - DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING); + DOTNET_ALLOC_COOKIE* pHeaderCookie = (DOTNET_ALLOC_COOKIE*)((uint8_t*)ptr - DOTNET_ALLOC_HEADER_COOKIE_SIZE_WITH_PADDING); if (pHeaderCookie->Address != ptr) { goto Fail; } size_t cbRequested = pHeaderCookie->Size; // Check cookie at end - BYTE* pTrailerCookie = (BYTE*)ptr + cbRequested; + uint8_t* pTrailerCookie = (uint8_t*)ptr + cbRequested; DOTNET_ALLOC_COOKIE vTrailerCookie = ReadAllocCookieUnaligned(pTrailerCookie); if (vTrailerCookie.Address != ptr) { goto Fail; } if (vTrailerCookie.Size != cbRequested) { goto Fail; } diff --git a/src/native/external/zlib/dotnet_allocator_win.c b/src/native/libs/System.IO.Compression.Native/zlib_allocator_win.c similarity index 98% rename from src/native/external/zlib/dotnet_allocator_win.c rename to src/native/libs/System.IO.Compression.Native/zlib_allocator_win.c index 414bf6b33056f..9bdf694495e68 100644 --- a/src/native/external/zlib/dotnet_allocator_win.c +++ b/src/native/libs/System.IO.Compression.Native/zlib_allocator_win.c @@ -6,7 +6,12 @@ #include #include #include /* _ASSERTE */ -#include "zutil.h" + +#ifdef INTERNAL_ZLIB_INTEL +#include +#else +#include +#endif /* A custom allocator for zlib that provides some defense-in-depth over standard malloc / free. * (Windows-specific version) From 52d407bc82ce812b4621e827b286d87cb1737acc Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Mon, 31 Jul 2023 18:56:18 -0700 Subject: [PATCH 7/9] Add env var for Windows --- .../zlib_allocator_win.c | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/native/libs/System.IO.Compression.Native/zlib_allocator_win.c b/src/native/libs/System.IO.Compression.Native/zlib_allocator_win.c index 9bdf694495e68..2637d6d9a7782 100644 --- a/src/native/libs/System.IO.Compression.Native/zlib_allocator_win.c +++ b/src/native/libs/System.IO.Compression.Native/zlib_allocator_win.c @@ -37,6 +37,45 @@ * crash dump. We use a special code that's easy to search for in Watson. */ +BOOL IsMitigationDisabled() +{ + enum _MitigationEnablementTristate + { + MITIGATION_NOT_YET_QUERIED = 0, + MITIGATION_DISABLED = 1, + MITIGATION_ENABLED = 2 // really, anything other than 0 or 1 + }; + static long s_fMitigationEnablementState = MITIGATION_NOT_YET_QUERIED; + + // If already initialized, return immediately. + // We don't need a volatile read here since the publish is performed with release semantics. + if (s_fMitigationEnablementState != MITIGATION_NOT_YET_QUERIED) + { + return (s_fMitigationEnablementState == MITIGATION_DISABLED); + } + + // Initialize the tri-state now. + // It's ok for multiple threads to do this simultaneously. Only one thread will win. + // Valid env var values to disable mitigation: "true" and "1" + // All other env var values (or error) leaves mitigation enabled. + // + // Buffer needs to be large enough to hold null terminator, but returned cch does not include + // null terminator. Note *exclusive* bounds of 0 and buffer length on returned cch. + // Ref: https://learn.microsoft.com/windows/win32/api/winbase/nf-winbase-getenvironmentvariable + CHAR pchBuffer[5]; // enough to hold "true" and a terminator + DWORD dwEnvVarLength = GetEnvironmentVariableA("DOTNET_SYSTEM_IO_COMPRESSION_DISABLEZLIBMITIGATIONS", pchBuffer, _countof(pchBuffer)); + BOOL fMitigationDisabled = (dwEnvVarLength > 0 && dwEnvVarLength < _countof(pchBuffer)) + && (strcmp(pchBuffer, "1") == 0 || strcmp(pchBuffer, "true") == 0); + + // We really don't care about the return value of the ICE operation. If another thread + // beat us to it, so be it. The recursive call will figure it out. + InterlockedCompareExchange( + /* destination: */ &s_fMitigationEnablementState, + /* exchange: */ fMitigationDisabled ? MITIGATION_DISABLED : MITIGATION_ENABLED, + /* comparand: */ MITIGATION_NOT_YET_QUERIED); + return IsMitigationDisabled(); +} + // Gets the special heap we'll allocate from. HANDLE GetZlibHeap() { @@ -99,6 +138,13 @@ voidpf ZLIB_INTERNAL zcalloc(opaque, items, size) { (void)opaque; // suppress C4100 - unreferenced formal parameter + if (IsMitigationDisabled()) + { + // fallback logic copied from zutil.c + return sizeof(uInt) > 2 ? (voidpf)malloc(items * size) : + (voidpf)calloc(items, size); + } + // If initializing a fixed-size structure, zero the memory. DWORD dwFlags = (items == 1) ? HEAP_ZERO_MEMORY : 0; @@ -156,6 +202,13 @@ void ZLIB_INTERNAL zcfree(opaque, ptr) { (void)opaque; // suppress C4100 - unreferenced formal parameter + if (IsMitigationDisabled()) + { + // fallback logic copied from zutil.c + free(ptr); + return; + } + if (ptr == NULL) { return; } // ok to free nullptr // Check cookie at beginning and end From e6c9b9745d26a8514618b64c87bc074c4972ce8d Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Mon, 31 Jul 2023 20:28:22 -0700 Subject: [PATCH 8/9] Add env var for Unix --- .../zlib_allocator_unix.c | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/native/libs/System.IO.Compression.Native/zlib_allocator_unix.c b/src/native/libs/System.IO.Compression.Native/zlib_allocator_unix.c index 9eb4bbf267105..1ab7568ac8ec9 100644 --- a/src/native/libs/System.IO.Compression.Native/zlib_allocator_unix.c +++ b/src/native/libs/System.IO.Compression.Native/zlib_allocator_unix.c @@ -24,6 +24,40 @@ * If any of these checks fails, the application raises SIGABRT. */ +static bool IsMitigationDisabled() +{ + enum _MitigationEnablementTristate + { + MITIGATION_NOT_YET_QUERIED = 0, + MITIGATION_DISABLED = 1, + MITIGATION_ENABLED = 2 // really, anything other than 0 or 1 + }; + static int s_fMitigationEnablementState = MITIGATION_NOT_YET_QUERIED; + + // If already initialized, return immediately. + // We don't need a volatile read here since the publish is performed with release semantics. + if (s_fMitigationEnablementState != MITIGATION_NOT_YET_QUERIED) + { + return (s_fMitigationEnablementState == MITIGATION_DISABLED); + } + + // Initialize the tri-state now. + // It's ok for multiple threads to do this simultaneously. Only one thread will win. + // Valid env var values to disable mitigation: "true" and "1" + // All other env var values (or error) leaves mitigation enabled. + + char* pchEnvVar = getenv("DOTNET_SYSTEM_IO_COMPRESSION_DISABLEZLIBMITIGATIONS"); + bool fMitigationDisabled = (pchEnvVar && (strcmp(pchEnvVar, "1") == 0 || strcmp(pchEnvVar, "true") == 0)); + + // We really don't care about the return value of the ICE operation. If another thread + // beat us to it, so be it. The recursive call will figure it out. + __sync_val_compare_and_swap( + /* destination: */ &s_fMitigationEnablementState, + /* comparand: */ MITIGATION_NOT_YET_QUERIED, + /* exchange: */ fMitigationDisabled ? MITIGATION_DISABLED : MITIGATION_ENABLED); + return IsMitigationDisabled(); +} + #ifndef MEMORY_ALLOCATION_ALIGNMENT // malloc() returns an address suitably aligned for any built-in data type. // Historically, this has been twice the arch's natural word size. @@ -77,6 +111,13 @@ voidpf ZLIB_INTERNAL zcalloc(opaque, items, size) { (void)opaque; // unreferenced formal parameter + if (IsMitigationDisabled()) + { + // fallback logic copied from zutil.c + return sizeof(uInt) > 2 ? (voidpf)malloc(items * size) : + (voidpf)calloc(items, size); + } + // If initializing a fixed-size structure, zero the memory. bool fZeroMemory = (items == 1); @@ -125,6 +166,13 @@ void ZLIB_INTERNAL zcfree(opaque, ptr) { (void)opaque; // unreferenced formal parameter + if (IsMitigationDisabled()) + { + // fallback logic copied from zutil.c + free(ptr); + return; + } + if (ptr == NULL) { return; } // ok to free nullptr // Check cookie at beginning From 21113a1e83fbc643ae05960d5817402240bcd6d6 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Tue, 1 Aug 2023 11:48:41 -0700 Subject: [PATCH 9/9] Fix mono compilation - Mono defines HOST_WIN32, not CLR_CMAKE_HOST_WIN32 - Follow same pattern from src\native\eventpipe\CMakeLists.txt --- src/native/external/zlib.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/external/zlib.cmake b/src/native/external/zlib.cmake index b08d2574df674..862e10118cd72 100644 --- a/src/native/external/zlib.cmake +++ b/src/native/external/zlib.cmake @@ -31,7 +31,7 @@ set(ZLIB_SOURCES_BASE # enable custom zlib allocator add_definitions(-DMY_ZCALLOC) -if(CLR_CMAKE_HOST_WIN32) +if(HOST_WIN32 OR CLR_CMAKE_TARGET_WIN32) set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} ../../libs/System.IO.Compression.Native/zlib_allocator_win.c) else() set(ZLIB_SOURCES_BASE ${ZLIB_SOURCES_BASE} ../../libs/System.IO.Compression.Native/zlib_allocator_unix.c)