Skip to content

Commit 6ae457d

Browse files
thurstondthurstond
authored
[msan] Mark allocator padding as uninitialized, with new origin tag (#157187)
This is follow-up work per discussion in #155944 (comment). If the allocator reserves more space than the user requested (e.g., `malloc(7)` and `calloc(7,1)` actually have 16 bytes reserved), the padding bytes will now be marked as uninitialized. Padding poisoning is controlled by the existing flag `poison_in_malloc` (which applies to all allocation functions, not only malloc). Origin tag: - For `calloc` or with track-origins > 1, the origin will be set as a new tag, `ALLOC_PADDING` - Otherwise, the existing `ALLOC` tag will be used. - In the case of ambiguity caused by origin granularity, `ALLOC` will take precedence.
1 parent 19a58a5 commit 6ae457d

File tree

5 files changed

+145
-9
lines changed

5 files changed

+145
-9
lines changed

‎compiler-rt/lib/msan/msan.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ u32 ChainOrigin(u32 id, StackTrace *stack);
303303
const int STACK_TRACE_TAG_POISON = StackTrace::TAG_CUSTOM + 1;
304304
const int STACK_TRACE_TAG_FIELDS = STACK_TRACE_TAG_POISON + 1;
305305
const int STACK_TRACE_TAG_VPTR = STACK_TRACE_TAG_FIELDS + 1;
306+
const int STACK_TRACE_TAG_ALLOC_PADDING = STACK_TRACE_TAG_VPTR + 1;
306307

307308
#define GET_MALLOC_STACK_TRACE \
308309
UNINITIALIZED BufferedStackTrace stack; \

‎compiler-rt/lib/msan/msan_allocator.cpp‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -217,25 +217,52 @@ static void *MsanAllocate(BufferedStackTrace *stack, uptr size, uptr alignment,
217217
}
218218
auto *meta = reinterpret_cast<Metadata *>(allocator.GetMetaData(allocated));
219219
meta->requested_size = size;
220+
uptr actually_allocated_size = allocator.GetActuallyAllocatedSize(allocated);
221+
void* padding_start = reinterpret_cast<char*>(allocated) + size;
222+
uptr padding_size = actually_allocated_size - size;
223+
224+
// - With calloc(7,1), we can set the ideal tagging:
225+
// bytes 0-6: initialized, origin not set (and irrelevant)
226+
// byte 7: uninitialized, origin TAG_ALLOC_PADDING
227+
// bytes 8-15: uninitialized, origin TAG_ALLOC_PADDING
228+
// - If we have malloc(7) and __msan_get_track_origins() > 1, the 4-byte
229+
// origin granularity only allows the slightly suboptimal tagging:
230+
// bytes 0-6: uninitialized, origin TAG_ALLOC
231+
// byte 7: uninitialized, origin TAG_ALLOC (suboptimal)
232+
// bytes 8-15: uninitialized, origin TAG_ALLOC_PADDING
233+
// - If we have malloc(7) and __msan_get_track_origins() == 1, we use a
234+
// single origin bean to reduce overhead:
235+
// bytes 0-6: uninitialized, origin TAG_ALLOC
236+
// byte 7: uninitialized, origin TAG_ALLOC (suboptimal)
237+
// bytes 8-15: uninitialized, origin TAG_ALLOC (suboptimal)
238+
if (__msan_get_track_origins() && flags()->poison_in_malloc &&
239+
(zero || (__msan_get_track_origins() > 1))) {
240+
stack->tag = STACK_TRACE_TAG_ALLOC_PADDING;
241+
Origin o2 = Origin::CreateHeapOrigin(stack);
242+
__msan_set_origin(padding_start, padding_size, o2.raw_id());
243+
}
244+
220245
if (zero) {
221246
if (allocator.FromPrimary(allocated))
222247
__msan_clear_and_unpoison(allocated, size);
223248
else
224249
__msan_unpoison(allocated, size); // Mem is already zeroed.
250+
251+
if (flags()->poison_in_malloc)
252+
__msan_poison(padding_start, padding_size);
225253
} else if (flags()->poison_in_malloc) {
226-
__msan_poison(allocated, size);
254+
__msan_poison(allocated, actually_allocated_size);
255+
227256
if (__msan_get_track_origins()) {
228257
stack->tag = StackTrace::TAG_ALLOC;
229258
Origin o = Origin::CreateHeapOrigin(stack);
230-
__msan_set_origin(allocated, size, o.raw_id());
259+
__msan_set_origin(
260+
allocated,
261+
__msan_get_track_origins() == 1 ? actually_allocated_size : size,
262+
o.raw_id());
231263
}
232264
}
233265

234-
uptr actually_allocated_size = allocator.GetActuallyAllocatedSize(allocated);
235-
// For compatibility, the allocator converted 0-sized allocations into 1 byte
236-
if (size == 0 && actually_allocated_size > 0 && flags()->poison_in_malloc)
237-
__msan_poison(allocated, 1);
238-
239266
UnpoisonParam(2);
240267
RunMallocHooks(allocated, size);
241268
return allocated;
@@ -255,9 +282,10 @@ void __msan::MsanDeallocate(BufferedStackTrace *stack, void *p) {
255282
if (flags()->poison_in_free && allocator.FromPrimary(p)) {
256283
__msan_poison(p, size);
257284
if (__msan_get_track_origins()) {
285+
uptr actually_allocated_size = allocator.GetActuallyAllocatedSize(p);
258286
stack->tag = StackTrace::TAG_DEALLOC;
259287
Origin o = Origin::CreateHeapOrigin(stack);
260-
__msan_set_origin(p, size, o.raw_id());
288+
__msan_set_origin(p, actually_allocated_size, o.raw_id());
261289
}
262290
}
263291
if (MsanThread *t = GetCurrentThread()) {

‎compiler-rt/lib/msan/msan_report.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ static void DescribeOrigin(u32 id) {
9090
Printf(" %sVirtual table ptr was destroyed%s\n", d.Origin(),
9191
d.Default());
9292
break;
93+
case STACK_TRACE_TAG_ALLOC_PADDING:
94+
Printf(" %sUninitialized value is outside of heap allocation%s\n",
95+
d.Origin(), d.Default());
96+
break;
9397
default:
9498
Printf(" %sUninitialized value was created%s\n", d.Origin(),
9599
d.Default());

‎compiler-rt/test/msan/allocator_padding.cpp‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// *** malloc: all bytes are uninitialized
2+
// * malloc byte 0
3+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 0 2>&1 \
4+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
5+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 0 2>&1 \
6+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
7+
//
8+
// * malloc byte 6
9+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 6 2>&1 \
10+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
11+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 6 2>&1 \
12+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
13+
//
14+
// This test assumes the allocator allocates 16 bytes for malloc(7). Bytes
15+
// 7-15 are padding.
16+
//
17+
// * malloc byte 7
18+
// Edge case: when the origin granularity spans both ALLOC and ALLOC_PADDING,
19+
// ALLOC always takes precedence.
20+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 7 2>&1 \
21+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
22+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 7 2>&1 \
23+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
24+
//
25+
// Bytes 8-15 are padding
26+
// For track-origins=1, ALLOC is used instead of ALLOC_PADDING.
27+
//
28+
// * malloc byte 8
29+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 8 2>&1 \
30+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
31+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 8 2>&1 \
32+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
33+
//
34+
// * malloc byte 15
35+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 15 2>&1 \
36+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC
37+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 15 2>&1 \
38+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
39+
40+
// *** calloc
41+
// Bytes 0-6 are fully initialized, so no MSan report should happen.
42+
//
43+
// * calloc byte 0
44+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && %run %t 0 2>&1
45+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && %run %t 0 2>&1
46+
//
47+
// * calloc byte 6
48+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && %run %t 6 2>&1
49+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && %run %t 6 2>&1
50+
//
51+
// * calloc byte 7
52+
// Byte 7 is uninitialized. Unlike malloc, this is tagged as ALLOC_PADDING
53+
// (since the origin does not need to track bytes 4-6).
54+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && not %run %t 7 2>&1 \
55+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
56+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && not %run %t 7 2>&1 \
57+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
58+
//
59+
// * calloc byte 8
60+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && not %run %t 8 2>&1 \
61+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
62+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && not %run %t 8 2>&1 \
63+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
64+
//
65+
// * calloc byte 15
66+
// RUN: %clang_msan -fsanitize-memory-track-origins=1 -DUSE_CALLOC %s -o %t && not %run %t 15 2>&1 \
67+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
68+
// RUN: %clang_msan -fsanitize-memory-track-origins=2 -DUSE_CALLOC %s -o %t && not %run %t 15 2>&1 \
69+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGIN-ALLOC-PADDING
70+
71+
#include <assert.h>
72+
#include <stdio.h>
73+
#include <stdlib.h>
74+
75+
int main(int argc, char **argv) {
76+
#ifdef USE_CALLOC
77+
char *p = (char *)calloc(7, 1);
78+
#else
79+
char *p = (char *)malloc(7);
80+
#endif
81+
82+
if (argc == 2) {
83+
int index = atoi(argv[1]);
84+
85+
printf("p[%d] = %d\n", index, p[index]);
86+
// CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
87+
// CHECK: {{#0 0x.* in main .*allocator_padding.cpp:}}[[@LINE-2]]
88+
// ORIGIN-ALLOC: Uninitialized value was created by a heap allocation
89+
// ORIGIN-ALLOC-PADDING: Uninitialized value is outside of heap allocation
90+
free(p);
91+
}
92+
93+
return 0;
94+
}

‎compiler-rt/test/msan/zero_alloc.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
// RUN: %clang_msan -Wno-alloc-size -fsanitize-recover=memory %s -o %t && not %run %t 2>&1 | FileCheck %s
1+
// RUN: %clang_msan -Wno-alloc-size -fsanitize-recover=memory %s -o %t && not %run %t 2>&1 \
2+
// RUN: | FileCheck %s --check-prefix=CHECK
3+
// RUN: %clang_msan -Wno-alloc-size -fsanitize-recover=memory -fsanitize-memory-track-origins=1 %s -o %t && not %run %t 2>&1 \
4+
// RUN: | FileCheck %s --check-prefixes=CHECK,DISCOUNT
5+
// RUN: %clang_msan -Wno-alloc-size -fsanitize-recover=memory -fsanitize-memory-track-origins=2 %s -o %t && not %run %t 2>&1 \
6+
// RUN: | FileCheck %s --check-prefixes=CHECK,ORIGINS
27

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

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

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

0 commit comments

Comments
 (0)