From effdc7e483708cfa4dc597c21f246c5dbc09daa0 Mon Sep 17 00:00:00 2001 From: Evgeniy Stepanov Date: Mon, 16 Sep 2013 11:03:31 +0000 Subject: [msan] Fix origin of deallocated memory. MSan poisons deallocated memory but it used to give it an invalid origin value, resulting in confusing reports. This change associates deallocation stack trace with such memory. Note that MSan does not have quarantine, and use-after-free detection is very limited. git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@190781 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/msan/lit_tests/use-after-free.cc | 34 ++++++++++++++++++++++++++++++++++ lib/msan/msan.cc | 2 ++ lib/msan/msan.h | 2 +- lib/msan/msan_allocator.cc | 35 ++++++++++++++++++++++------------- lib/msan/msan_flags.h | 1 + lib/msan/msan_interceptors.cc | 4 ++-- lib/msan/msan_new_delete.cc | 3 ++- 7 files changed, 64 insertions(+), 17 deletions(-) create mode 100644 lib/msan/lit_tests/use-after-free.cc diff --git a/lib/msan/lit_tests/use-after-free.cc b/lib/msan/lit_tests/use-after-free.cc new file mode 100644 index 000000000..ac47c0233 --- /dev/null +++ b/lib/msan/lit_tests/use-after-free.cc @@ -0,0 +1,34 @@ +// RUN: %clangxx_msan -m64 -O0 %s -o %t && not %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out +// RUN: %clangxx_msan -m64 -O1 %s -o %t && not %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out +// RUN: %clangxx_msan -m64 -O2 %s -o %t && not %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out +// RUN: %clangxx_msan -m64 -O3 %s -o %t && not %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out + +// RUN: %clangxx_msan -fsanitize-memory-track-origins -m64 -O0 %s -o %t && not %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out +// RUN: %clangxx_msan -fsanitize-memory-track-origins -m64 -O1 %s -o %t && not %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out +// RUN: %clangxx_msan -fsanitize-memory-track-origins -m64 -O2 %s -o %t && not %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out +// RUN: %clangxx_msan -fsanitize-memory-track-origins -m64 -O3 %s -o %t && not %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out + +#include +int main(int argc, char **argv) { + int *volatile p = (int *)malloc(sizeof(int)); + *p = 42; + free(p); + + if (*p) + exit(0); + // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value + // CHECK: {{#0 0x.* in main .*use-after-free.cc:}}[[@LINE-3]] + + // CHECK-ORIGINS: Uninitialized value was created by a heap allocation + // CHECK-ORIGINS: {{#0 0x.* in .*free}} + // CHECK-ORIGINS: {{#1 0x.* in main .*use-after-free.cc:}}[[@LINE-9]] + return 0; +} diff --git a/lib/msan/msan.cc b/lib/msan/msan.cc index 0a4af7cdd..2b9324aa3 100644 --- a/lib/msan/msan.cc +++ b/lib/msan/msan.cc @@ -122,6 +122,7 @@ static void ParseFlagsFromString(Flags *f, const char *str) { ParseFlag(str, &f->poison_heap_with_zeroes, "poison_heap_with_zeroes"); ParseFlag(str, &f->poison_stack_with_zeroes, "poison_stack_with_zeroes"); ParseFlag(str, &f->poison_in_malloc, "poison_in_malloc"); + ParseFlag(str, &f->poison_in_malloc, "poison_in_free"); ParseFlag(str, &f->exit_code, "exit_code"); if (f->exit_code < 0 || f->exit_code > 127) { Printf("Exit code not in [0, 128) range: %d\n", f->exit_code); @@ -153,6 +154,7 @@ static void InitializeFlags(Flags *f, const char *options) { f->poison_heap_with_zeroes = false; f->poison_stack_with_zeroes = false; f->poison_in_malloc = true; + f->poison_in_free = true; f->exit_code = 77; f->report_umrs = true; f->verbosity = 0; diff --git a/lib/msan/msan.h b/lib/msan/msan.h index 16dfc0cf6..8c24de647 100644 --- a/lib/msan/msan.h +++ b/lib/msan/msan.h @@ -46,7 +46,7 @@ void InitializeInterceptors(); void *MsanReallocate(StackTrace *stack, void *oldp, uptr size, uptr alignment, bool zeroise); -void MsanDeallocate(void *ptr); +void MsanDeallocate(StackTrace *stack, void *ptr); void InstallTrapHandler(); void InstallAtExitHandler(); void ReplaceOperatorsNewAndDelete(); diff --git a/lib/msan/msan_allocator.cc b/lib/msan/msan_allocator.cc index 050a55c50..0fe1297a8 100644 --- a/lib/msan/msan_allocator.cc +++ b/lib/msan/msan_allocator.cc @@ -51,21 +51,23 @@ static void *MsanAllocate(StackTrace *stack, uptr size, void *res = allocator.Allocate(&cache, size, alignment, false); Metadata *meta = reinterpret_cast(allocator.GetMetaData(res)); meta->requested_size = size; - if (zeroise) + if (zeroise) { __msan_clear_and_unpoison(res, size); - else if (flags()->poison_in_malloc) + } else if (flags()->poison_in_malloc) { __msan_poison(res, size); - if (__msan_get_track_origins()) { - u32 stack_id = StackDepotPut(stack->trace, stack->size); - CHECK(stack_id); - CHECK_EQ((stack_id >> 31), 0); // Higher bit is occupied by stack origins. - __msan_set_origin(res, size, stack_id); + if (__msan_get_track_origins()) { + u32 stack_id = StackDepotPut(stack->trace, stack->size); + CHECK(stack_id); + CHECK_EQ((stack_id >> 31), + 0); // Higher bit is occupied by stack origins. + __msan_set_origin(res, size, stack_id); + } } MSAN_MALLOC_HOOK(res, size); return res; } -void MsanDeallocate(void *p) { +void MsanDeallocate(StackTrace *stack, void *p) { CHECK(p); Init(); MSAN_FREE_HOOK(p); @@ -74,9 +76,16 @@ void MsanDeallocate(void *p) { meta->requested_size = 0; // This memory will not be reused by anyone else, so we are free to keep it // poisoned. - __msan_poison(p, size); - if (__msan_get_track_origins()) - __msan_set_origin(p, size, -1); + if (flags()->poison_in_free) { + __msan_poison(p, size); + if (__msan_get_track_origins()) { + u32 stack_id = StackDepotPut(stack->trace, stack->size); + CHECK(stack_id); + CHECK_EQ((stack_id >> 31), + 0); // Higher bit is occupied by stack origins. + __msan_set_origin(p, size, stack_id); + } + } allocator.Deallocate(&cache, p); } @@ -85,7 +94,7 @@ void *MsanReallocate(StackTrace *stack, void *old_p, uptr new_size, if (!old_p) return MsanAllocate(stack, new_size, alignment, zeroise); if (!new_size) { - MsanDeallocate(old_p); + MsanDeallocate(stack, old_p); return 0; } Metadata *meta = reinterpret_cast(allocator.GetMetaData(old_p)); @@ -103,7 +112,7 @@ void *MsanReallocate(StackTrace *stack, void *old_p, uptr new_size, // Printf("realloc: old_size %zd new_size %zd\n", old_size, new_size); if (new_p) __msan_memcpy(new_p, old_p, memcpy_size); - MsanDeallocate(old_p); + MsanDeallocate(stack, old_p); return new_p; } diff --git a/lib/msan/msan_flags.h b/lib/msan/msan_flags.h index f05263247..318ea7cf4 100644 --- a/lib/msan/msan_flags.h +++ b/lib/msan/msan_flags.h @@ -23,6 +23,7 @@ struct Flags { bool poison_heap_with_zeroes; // default: false bool poison_stack_with_zeroes; // default: false bool poison_in_malloc; // default: true + bool poison_in_free; // default: true bool report_umrs; bool wrap_signals; bool halt_on_error; diff --git a/lib/msan/msan_interceptors.cc b/lib/msan/msan_interceptors.cc index 0431fed6c..d15618316 100644 --- a/lib/msan/msan_interceptors.cc +++ b/lib/msan/msan_interceptors.cc @@ -165,9 +165,9 @@ INTERCEPTOR(void *, pvalloc, SIZE_T size) { } INTERCEPTOR(void, free, void *ptr) { - ENSURE_MSAN_INITED(); + GET_MALLOC_STACK_TRACE; if (ptr == 0) return; - MsanDeallocate(ptr); + MsanDeallocate(&stack, ptr); } INTERCEPTOR(SIZE_T, strlen, const char *s) { diff --git a/lib/msan/msan_new_delete.cc b/lib/msan/msan_new_delete.cc index 88d4364f6..17687ddfc 100644 --- a/lib/msan/msan_new_delete.cc +++ b/lib/msan/msan_new_delete.cc @@ -43,7 +43,8 @@ void *operator new(size_t size, std::nothrow_t const&) { OPERATOR_NEW_BODY; } void *operator new[](size_t size, std::nothrow_t const&) { OPERATOR_NEW_BODY; } #define OPERATOR_DELETE_BODY \ - if (ptr) MsanDeallocate(ptr) + GET_MALLOC_STACK_TRACE; \ + if (ptr) MsanDeallocate(&stack, ptr) void operator delete(void *ptr) { OPERATOR_DELETE_BODY; } void operator delete[](void *ptr) { OPERATOR_DELETE_BODY; } -- cgit v1.2.3