Skip to content
Merged
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
16 changes: 16 additions & 0 deletions compiler-rt/lib/asan/asan_malloc_win.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,22 @@ void *SharedReAlloc(ReAllocFunction reallocFunc, SizeFunction heapSizeFunc,
}
}

if (dwFlags & HEAP_REALLOC_IN_PLACE_ONLY) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for doing the check here, instead of on line 367 (which would become redundant)?

Copy link
Contributor Author

@bernhardu bernhardu Jul 27, 2025

Choose a reason for hiding this comment

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

This is because line 367 is not reached. Instead the function is left via reallocFunc(hHeap, dwFlags, lpMem, dwBytes); in line 321.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, correction, not line 321, the function is left in line 352, so line 367 is not reached.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I see. I had misunderstood the code.

size_t old_usable_size = asan_malloc_usable_size(lpMem, pc, bp);
if (dwBytes == old_usable_size) {
// Nothing to change, return the current pointer.
return lpMem;
} else if (dwBytes >= old_usable_size) {
// Growing with HEAP_REALLOC_IN_PLACE_ONLY is not supported.
return nullptr;
} else {
// Shrinking with HEAP_REALLOC_IN_PLACE_ONLY is not yet supported.
// For now return the current pointer and
// leave the allocation size as it is.
return lpMem;
}
}
Comment on lines +327 to +339
Copy link
Contributor

Choose a reason for hiding this comment

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

What if this were all replaced with return nullptr;'? Would that provide stronger protection, while still staying spec-compliant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay, it took me a while to run some tests.
And I found following "real world" location as an example [1] in the Wine tree to fail when always returning nullptr with HEAP_REALLOC_IN_PLACE_ONLY.

So in the end it boils down to the question if an application is allowed to rely on at least shrinking to succeed?
What do you think?

[1] https://github.com/wine-mirror/wine/blob/288a40d05c8cddf62d0b12524a90d2d4f5ce114d/dlls/kernelbase/locale.c#L5493-L5517

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for investigating! Given this, I concur with the approach of your patch.


if (ownershipState == ASAN && !only_asan_supported_flags) {
// Conversion to unsupported flags allocation,
// transfer this allocation back to the original allocator.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// RUN: %clang_cl_asan %Od %s %Fe%t %MD
// RUN: %env_asan_opts=windows_hook_rtl_allocators=true not %run %t 2>&1 | FileCheck %s

#include <stdio.h>
#include <windows.h>

using AllocateFunctionPtr = PVOID(__stdcall *)(PVOID, ULONG, SIZE_T);
using ReAllocateFunctionPtr = PVOID(__stdcall *)(PVOID, ULONG, PVOID, SIZE_T);
using FreeFunctionPtr = PVOID(__stdcall *)(PVOID, ULONG, PVOID);

int main() {
HMODULE NtDllHandle = GetModuleHandle("ntdll.dll");
if (!NtDllHandle) {
fputs("Couldn't load ntdll??\n", stderr);
return -1;
}

auto RtlAllocateHeap_ptr =
(AllocateFunctionPtr)GetProcAddress(NtDllHandle, "RtlAllocateHeap");
if (RtlAllocateHeap_ptr == 0) {
fputs("Couldn't RtlAllocateHeap\n", stderr);
return -1;
}

auto RtlReAllocateHeap_ptr =
(ReAllocateFunctionPtr)GetProcAddress(NtDllHandle, "RtlReAllocateHeap");
if (RtlReAllocateHeap_ptr == 0) {
fputs("Couldn't find RtlReAllocateHeap\n", stderr);
return -1;
}

auto RtlFreeHeap_ptr =
(FreeFunctionPtr)GetProcAddress(NtDllHandle, "RtlFreeHeap");
if (RtlFreeHeap_ptr == 0) {
fputs("Couldn't RtlFreeHeap\n", stderr);
return -1;
}

char *ptr1;
char *ptr2;
ptr2 = ptr1 = (char *)RtlAllocateHeap_ptr(GetProcessHeap(), 0, 15);
if (ptr1)
fputs("Okay alloc\n", stderr);
Copy link
Contributor

Choose a reason for hiding this comment

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

// CHECK: Okay alloc

// TODO: Growing is currently not supported
ptr2 = (char *)RtlReAllocateHeap_ptr(GetProcessHeap(),
HEAP_REALLOC_IN_PLACE_ONLY, ptr1, 23);
if (ptr2 == NULL)
fputs("Okay grow failed\n", stderr);
// CHECK: Okay grow failed

// TODO: Shrinking is currently not supported
ptr2 = (char *)RtlReAllocateHeap_ptr(GetProcessHeap(),
HEAP_REALLOC_IN_PLACE_ONLY, ptr1, 7);
if (ptr2 == ptr1)
fputs("Okay shrinking return the original pointer\n", stderr);
// CHECK: Okay shrinking return the original pointer

ptr1[7] = 'a';
fputs("Okay 7\n", stderr);
// CHECK: Okay 7

// TODO: Writing behind the shrinked part is currently not detected.
// Therefore test writing behind the original allocation for now.
ptr1[16] = 'a';
// CHECK: AddressSanitizer: heap-buffer-overflow on address [[ADDR:0x[0-9a-f]+]]
// CHECK: WRITE of size 1 at [[ADDR]] thread T0

RtlFreeHeap_ptr(GetProcessHeap(), 0, ptr1);
}