[PATCH v5 0/1] MR8852: ntdll: Zero aligned size in initialize_block().

[Paul Gofman (＠gofman)]

--
  v5: ntdll: Zero aligned size in initialize_block().

https://gitlab.winehq.org/wine/wine/-/merge_requests/8852

[PATCH v5 1/1] ntdll: Zero aligned size in initialize_block().

[Paul Gofman]

From: Paul Gofman

---
 dlls/kernel32/tests/heap.c | 46 ++++++++++++++++++++++++++++++++++++++
 dlls/ntdll/heap.c          |  8 ++++---
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c
index f88a140a115..5d43851baa6 100644
--- a/dlls/kernel32/tests/heap.c
+++ b/dlls/kernel32/tests/heap.c
@@ -41,6 +41,8 @@
 #define BLOCK_ALIGN         (2 * sizeof(void *) - 1)
 #define ALIGN_BLOCK_SIZE(x) (((x) + BLOCK_ALIGN) & ~BLOCK_ALIGN)

+#define ROUND_SIZE(size, mask) ((((SIZE_T)(size) + (mask)) & ~(SIZE_T)(mask)))
+
 /* use function pointers to avoid warnings for invalid parameter tests */
 static LPVOID (WINAPI *pHeapAlloc)(HANDLE,DWORD,SIZE_T);
 static LPVOID (WINAPI *pHeapReAlloc)(HANDLE,DWORD,LPVOID,SIZE_T);
@@ -3456,6 +3458,48 @@ static void test_heap_layout( HANDLE handle, DWORD global_flag, DWORD heap_flags
     }
 }

+static void test_heap_tail_zeroing( DWORD heap_flags )
+{
+    static const ULONG_PTR large_block_min_size = 65536 * (2 * sizeof(void *));
+    HANDLE heap = GetProcessHeap();
+    size_t size, size_aligned;
+    ULONG_PTR v, expected;
+    char *p1, *p2;
+
+    if (heap_flags & HEAP_PAGE_ALLOCS)
+    {
+        /* This behaves differently, no support yet. */
+        skip( "Skipping test with HEAP_PAGE_ALLOCS.\n" );
+        return;
+    }
+
+    for (size = 1; size <= 1048576 * 2; size *= 2)
+    {
+        winetest_push_context( "heap_flags %#lx, size %Iu", heap_flags, size );
+        p1 = pHeapAlloc( heap, 0, size + 1 );
+        ok( !!p1, "got NULL.\n" );
+        size_aligned = ROUND_SIZE(size + 1, sizeof(void *) - 1);
+        /* This and read access below is going to make valgrind or ASAN unhappy but the purpose of this test is
+         * to specifically check what happens with the tail bytes following the allocation. */
+        if (!(heap_flags & (HEAP_VALIDATE_PARAMS | HEAP_VALIDATE_ALL)))
+            memset( p1, 0xcc, size_aligned );
+        pHeapFree( heap, 0, p1 );
+
+        /* We are not guarenteed to get the same pointer here but that often happens, especially when the test
+         * is run first, and spoling the data before that adds certainity to the results. */
+        p2 = pHeapAlloc( heap, HEAP_ZERO_MEMORY, size + 1 );
+        ok( !!p2, "got NULL.\n" );
+        v = 0;
+        memcpy( &v, p2 + size, size_aligned - size );
+        expected = 0;
+        if (size_aligned - size > 1 && size + 1 < large_block_min_size && heap_flags & HEAP_TAIL_CHECKING_ENABLED)
+            memset( (char *)&expected + 1, 0xab, size_aligned - size - 1 );
+        ok( v == expected, "got %#Ix, expected %#Ix.\n", v, expected );
+        pHeapFree( heap, 0, p2 );
+        winetest_pop_context();
+    }
+}
+
 static void test_child_heap( const char *arg )
 {
     char buffer[32];
@@ -3535,6 +3579,7 @@ static void test_child_heap( const char *arg )
     ok( ret, "HeapDestroy failed, error %lu\n", GetLastError() );

test_heap_checks( heap_flags );
+    test_heap_tail_zeroing( heap_flags );
 }

static void test_GetPhysicallyInstalledSystemMemory(void)
@@ -3808,6 +3853,7 @@ START_TEST(heap)
     test_GetPhysicallyInstalledSystemMemory();
     test_GlobalMemoryStatus();
     test_HeapSummary();
+    test_heap_tail_zeroing( 0 );

if (pRtlGetNtGlobalFlags)
     {
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index e761268095e..c68c449a524 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -503,14 +503,16 @@ static inline void mark_block_tail( struct block *block, DWORD flags )
 static inline void initialize_block( struct block *block, SIZE_T old_size, SIZE_T size, DWORD flags )
 {
     char *data = (char *)(block + 1);
-    SIZE_T i;
+    SIZE_T i, aligned_size;

if (size <= old_size) return;

if (flags & HEAP_ZERO_MEMORY)
     {
-        valgrind_make_writable( data + old_size, size - old_size );
-        memset( data + old_size, 0, size - old_size );
+        aligned_size = ROUND_SIZE( size, sizeof(void *) - 1 );
+        valgrind_make_writable( data + old_size, aligned_size - old_size );
+        memset( data + old_size, 0, aligned_size - old_size );
+        valgrind_make_noaccess( data + size, aligned_size - size );
     }
     else if (flags & HEAP_FREE_CHECKING_ENABLED)
     {

[Paul Gofman (＠gofman)]

v5:
 - drop Win7 check.

[Elizabeth Figura (＠zfigura)]

On Mon Sep  1 18:52:31 2025 +0000, Paul Gofman wrote:
> changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/8852/diffs?diff_id=204672&start_sha=8003e4fafddd0fb25ff62620b735603905dce727#ca2662222558c506848b42d086c6fa0a5800a233_100_87)

Why are version checks in tests so bad, exactly? It's not like we're testing Wine behaviour here; it's hidden behind broken().