Skip to content

[msan] Mark allocator padding as uninitialized, with new origin tag #157187

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Oct 14, 2025
Merged

[msan] Mark allocator padding as uninitialized, with new origin tag #157187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ed47840
[msan] Mark allocator padding as uninitialized, with new origin tag
thurstond Sep 5, 2025
1b1e1b8
Update comment
thurstond Sep 5, 2025
7105241
Remove poison_in_calloc, add poison_in_padding (shared between calloc…
thurstond Sep 5, 2025
ff736b1
Add description to flags
thurstond Sep 5, 2025
6fc1378
Tweak wording
thurstond Sep 5, 2025
dea7c28
Address Vitaly's feedback
thurstond Sep 5, 2025
7b98f59
Change message to "Uninitialized value is outside of heap allocation"
thurstond Sep 8, 2025
69bf311
Use ALLOC tag instead of ALLOC_PADDING when track_origins() == 1
thurstond Sep 8, 2025
aad1b12
clang-format
thurstond Sep 8, 2025
cf66f50
Test track-origins=1 for zero_alloc.cpp
thurstond Sep 8, 2025
322586b
reinterpret_cast<Florian>
thurstond Sep 15, 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
1 change: 1 addition & 0 deletions compiler-rt/lib/msan/msan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ u32 ChainOrigin(u32 id, StackTrace *stack);
const int STACK_TRACE_TAG_POISON = StackTrace::TAG_CUSTOM + 1;
const int STACK_TRACE_TAG_FIELDS = STACK_TRACE_TAG_POISON + 1;
const int STACK_TRACE_TAG_VPTR = STACK_TRACE_TAG_FIELDS + 1;
const int STACK_TRACE_TAG_ALLOC_PADDING = STACK_TRACE_TAG_VPTR + 1;

#define GET_MALLOC_STACK_TRACE \
UNINITIALIZED BufferedStackTrace stack; \
Expand Down
44 changes: 36 additions & 8 deletions compiler-rt/lib/msan/msan_allocator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,25 +217,52 @@ static void *MsanAllocate(BufferedStackTrace *stack, uptr size, uptr alignment,
}
auto *meta = reinterpret_cast<Metadata *>(allocator.GetMetaData(allocated));
meta->requested_size = size;
uptr actually_allocated_size = allocator.GetActuallyAllocatedSize(allocated);
void* padding_start = reinterpret_cast<char*>(allocated) + size;
Copy link
Collaborator

@vitalybuka vitalybuka Oct 14, 2025

Choose a reason for hiding this comment

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

These used only in one branch of zero, declare it there

Copy link
Contributor Author

@thurstond thurstond Oct 14, 2025

Choose a reason for hiding this comment

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

Oops, I missed this comment before merging. I will send a quick followup patch.

Copy link
Contributor Author

@thurstond thurstond Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think they can't be moved.

padding_start and padding_size are both used on lines 242 and 252 (separate if blocks), hence they must be defined above.

padding_size is defined in terms of actually_allocated_size, hence actually_allocated_size also can't be moved later.

uptr padding_size = actually_allocated_size - size;

// - With calloc(7,1), we can set the ideal tagging:
// bytes 0-6: initialized, origin not set (and irrelevant)
// byte 7: uninitialized, origin TAG_ALLOC_PADDING
// bytes 8-15: uninitialized, origin TAG_ALLOC_PADDING
// - If we have malloc(7) and __msan_get_track_origins() > 1, the 4-byte
// origin granularity only allows the slightly suboptimal tagging:
// bytes 0-6: uninitialized, origin TAG_ALLOC
// byte 7: uninitialized, origin TAG_ALLOC (suboptimal)
// bytes 8-15: uninitialized, origin TAG_ALLOC_PADDING
// - If we have malloc(7) and __msan_get_track_origins() == 1, we use a
// single origin bean to reduce overhead:
// bytes 0-6: uninitialized, origin TAG_ALLOC
// byte 7: uninitialized, origin TAG_ALLOC (suboptimal)
// bytes 8-15: uninitialized, origin TAG_ALLOC (suboptimal)
if (__msan_get_track_origins() && flags()->poison_in_malloc &&
(zero || (__msan_get_track_origins() > 1))) {
stack->tag = STACK_TRACE_TAG_ALLOC_PADDING;
Origin o2 = Origin::CreateHeapOrigin(stack);
__msan_set_origin(padding_start, padding_size, o2.raw_id());
}

if (zero) {
if (allocator.FromPrimary(allocated))
__msan_clear_and_unpoison(allocated, size);
else
__msan_unpoison(allocated, size); // Mem is already zeroed.

if (flags()->poison_in_malloc)
__msan_poison(padding_start, padding_size);
} else if (flags()->poison_in_malloc) {
__msan_poison(allocated, size);
__msan_poison(allocated, actually_allocated_size);

if (__msan_get_track_origins()) {
stack->tag = StackTrace::TAG_ALLOC;
Origin o = Origin::CreateHeapOrigin(stack);
__msan_set_origin(allocated, size, o.raw_id());
__msan_set_origin(
allocated,
__msan_get_track_origins() == 1 ? actually_allocated_size : size,
o.raw_id());
}
}

uptr actually_allocated_size = allocator.GetActuallyAllocatedSize(allocated);
// For compatibility, the allocator converted 0-sized allocations into 1 byte
if (size == 0 && actually_allocated_size > 0 && flags()->poison_in_malloc)
__msan_poison(allocated, 1);

UnpoisonParam(2);
RunMallocHooks(allocated, size);
return allocated;
Expand All @@ -255,9 +282,10 @@ void __msan::MsanDeallocate(BufferedStackTrace *stack, void *p) {
if (flags()->poison_in_free && allocator.FromPrimary(p)) {
__msan_poison(p, size);
if (__msan_get_track_origins()) {
uptr actually_allocated_size = allocator.GetActuallyAllocatedSize(p);
stack->tag = StackTrace::TAG_DEALLOC;
Origin o = Origin::CreateHeapOrigin(stack);
__msan_set_origin(p, size, o.raw_id());
__msan_set_origin(p, actually_allocated_size, o.raw_id());
}
}
if (MsanThread *t = GetCurrentThread()) {
Expand Down
4 changes: 4 additions & 0 deletions compiler-rt/lib/msan/msan_report.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ static void DescribeOrigin(u32 id) {
Printf(" %sVirtual table ptr was destroyed%s\n", d.Origin(),
d.Default());
break;
case STACK_TRACE_TAG_ALLOC_PADDING:
Printf(" %sUninitialized value is outside of heap allocation%s\n",
d.Origin(), d.Default());
break;
default:
Printf(" %sUninitialized value was created%s\n", d.Origin(),
d.Default());
Expand Down
94 changes: 94 additions & 0 deletions compiler-rt/test/msan/allocator_padding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// *** malloc: all bytes are uninitialized
// * malloc byte 0
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 0 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 0 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
//
// * malloc byte 6
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 6 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 6 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
//
// This test assumes the allocator allocates 16 bytes for malloc(7). Bytes
// 7-15 are padding.
//
// * malloc byte 7
// Edge case: when the origin granularity spans both ALLOC and ALLOC_PADDING,
// ALLOC always takes precedence.
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 7 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 7 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
//
// Bytes 8-15 are padding
// For track-origins=1, ALLOC is used instead of ALLOC_PADDING.
//
// * malloc byte 8
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 8 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 8 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
//
// * malloc byte 15
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 15 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 15 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING

// *** calloc
// Bytes 0-6 are fully initialized, so no MSan report should happen.
//
// * calloc byte 0
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && %run %t 0 2>&1
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && %run %t 0 2>&1
//
// * calloc byte 6
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && %run %t 6 2>&1
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && %run %t 6 2>&1
//
// * calloc byte 7
// Byte 7 is uninitialized. Unlike malloc, this is tagged as ALLOC_PADDING
// (since the origin does not need to track bytes 4-6).
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && not %run %t 7 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && not %run %t 7 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
//
// * calloc byte 8
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && not %run %t 8 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && not %run %t 8 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
//
// * calloc byte 15
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && not %run %t 15 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && not %run %t 15 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>

int main(int argc, char **argv) {
#ifdef USE_CALLOC
char *p = (char *)calloc(7, 1);
#else
char *p = (char *)malloc(7);
#endif

if (argc == 2) {
int index = atoi(argv[1]);

printf("p[%d] = %d\n", index, p[index]);
// CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
// CHECK: {{#0 0x.* in main .*allocator_padding.cpp:}}[[@LINE-2]]
// ORIGIN-ALLOC: Uninitialized value was created by a heap allocation
// ORIGIN-ALLOC-PADDING: Uninitialized value is outside of heap allocation
free(p);
}

return 0;
}
11 changes: 10 additions & 1 deletion compiler-rt/test/msan/zero_alloc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
// RUN: %clang_msan -Wno-alloc-size -fsanitize-recover=memory %s -o %t && not %run %t 2>&1 | FileCheck %s
// RUN: %clang_msan -Wno-alloc-size -fsanitize-recover=memory %s -o %t && not %run %t 2>&1 \
// RUN: | FileCheck %s --check-prefix=CHECK
// RUN: %clang_msan -Wno-alloc-size -fsanitize-recover=memory -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,DISCOUNT
// RUN: %clang_msan -Wno-alloc-size -fsanitize-recover=memory -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGINS

#include <stdio.h>
#include <stdlib.h>
Expand All @@ -10,6 +15,7 @@ int main(int argc, char **argv) {
printf("Content of p1 is: %d\n", *p1);
// CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
// CHECK: {{#0 0x.* in main .*zero_alloc.cpp:}}[[@LINE-2]]
// DISCOUNT,ORIGINS: Uninitialized value is outside of heap allocation
free(p1);
}

Expand All @@ -19,6 +25,7 @@ int main(int argc, char **argv) {
printf("Content of p2 is: %d\n", *p2);
// CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
// CHECK: {{#0 0x.* in main .*zero_alloc.cpp:}}[[@LINE-2]]
// DISCOUNT,ORIGINS: Uninitialized value is outside of heap allocation
free(p2);
}

Expand All @@ -28,6 +35,8 @@ int main(int argc, char **argv) {
printf("Content of p2 is: %d\n", *p3);
// CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
// CHECK: {{#0 0x.* in main .*zero_alloc.cpp:}}[[@LINE-2]]
// DISCOUNT: Uninitialized value was created by a heap allocation
// ORIGINS: Uninitialized value is outside of heap allocation
free(p3);
}

Expand Down