From 65fdf677f28407efea358027ecc56c7e1d0c41d8 Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Tue, 25 Jul 2017 21:18:02 +0000 Subject: [PATCH] [scudo] Check for pvalloc overflow Summary: Previously we were rounding up the size passed to `pvalloc` to the next multiple of page size no matter what. There is an overflow possibility that wasn't accounted for. So now, return null in the event of an overflow. The man page doesn't seem to indicate the errno to set in this particular situation, but the glibc unit tests go for ENOMEM (https://code.woboq.org/userspace/glibc/malloc/tst-pvalloc.c.html#54) so we'll do the same. Update the aligned allocation funtions tests to check for properly aligned returned pointers, and the `pvalloc` corner cases. @alekseyshl: do you want me to do the same in the other Sanitizers? Reviewers: alekseyshl Reviewed By: alekseyshl Subscribers: kubamracek, alekseyshl, llvm-commits Differential Revision: https://reviews.llvm.org/D35818 llvm-svn: 309033 --- .../sanitizer_common/sanitizer_allocator_checks.h | 6 +++ compiler-rt/lib/scudo/scudo_allocator.cpp | 4 ++ compiler-rt/test/scudo/memalign.cpp | 14 ++--- compiler-rt/test/scudo/valloc.cpp | 63 ++++++++++++++++++++++ 4 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 compiler-rt/test/scudo/valloc.cpp diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_checks.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_checks.h index 202916e..b72f541 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_checks.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_checks.h @@ -59,6 +59,12 @@ INLINE bool CheckForCallocOverflow(uptr size, uptr n) { return (max / size) < n; } +// Returns true if the size passed to pvalloc overflows when rounded to the next +// multiple of page_size. +INLINE bool CheckForPvallocOverflow(uptr size, uptr page_size) { + return RoundUpTo(size, page_size) < size; +} + } // namespace __sanitizer #endif // SANITIZER_ALLOCATOR_CHECKS_H diff --git a/compiler-rt/lib/scudo/scudo_allocator.cpp b/compiler-rt/lib/scudo/scudo_allocator.cpp index 38522de..e175856 100644 --- a/compiler-rt/lib/scudo/scudo_allocator.cpp +++ b/compiler-rt/lib/scudo/scudo_allocator.cpp @@ -665,6 +665,10 @@ void *scudoValloc(uptr Size) { void *scudoPvalloc(uptr Size) { uptr PageSize = GetPageSizeCached(); + if (UNLIKELY(CheckForPvallocOverflow(Size, PageSize))) { + errno = errno_ENOMEM; + return ScudoAllocator::FailureHandler::OnBadRequest(); + } // pvalloc(0) should allocate one page. Size = Size ? RoundUpTo(Size, PageSize) : PageSize; return SetErrnoOnNull(Instance.allocate(Size, PageSize, FromMemalign)); diff --git a/compiler-rt/test/scudo/memalign.cpp b/compiler-rt/test/scudo/memalign.cpp index 82c54af..72aacff 100644 --- a/compiler-rt/test/scudo/memalign.cpp +++ b/compiler-rt/test/scudo/memalign.cpp @@ -8,17 +8,13 @@ #include #include #include +#include #include #include - -// Reduce the size of the quarantine, or the test can run out of aligned memory -// on 32-bit for the larger alignments. -extern "C" const char *__scudo_default_options() { - return "QuarantineSizeMb=1"; -} +#include // Sometimes the headers may not have this... -extern "C" void *aligned_alloc (size_t alignment, size_t size); +extern "C" void *aligned_alloc(size_t alignment, size_t size); int main(int argc, char **argv) { @@ -32,9 +28,11 @@ int main(int argc, char **argv) if (!strcmp(argv[1], "valid")) { posix_memalign(&p, alignment, size); assert(p); + assert(((uintptr_t)p & (alignment - 1)) == 0); free(p); p = aligned_alloc(alignment, size); assert(p); + assert(((uintptr_t)p & (alignment - 1)) == 0); free(p); // Tests various combinations of alignment and sizes for (int i = (sizeof(void *) == 4) ? 3 : 4; i < 19; i++) { @@ -44,6 +42,7 @@ int main(int argc, char **argv) for (int k = 0; k < 3; k++) { p = memalign(alignment, size - (2 * sizeof(void *) * k)); assert(p); + assert(((uintptr_t)p & (alignment - 1)) == 0); free(p); } } @@ -54,6 +53,7 @@ int main(int argc, char **argv) for (int k = 0; k < 3; k++) { p = memalign(alignment, 0x1000 - (2 * sizeof(void *) * k)); assert(p); + assert(((uintptr_t)p & (alignment - 1)) == 0); free(p); } } diff --git a/compiler-rt/test/scudo/valloc.cpp b/compiler-rt/test/scudo/valloc.cpp new file mode 100644 index 0000000..010dac2 --- /dev/null +++ b/compiler-rt/test/scudo/valloc.cpp @@ -0,0 +1,63 @@ +// RUN: %clang_scudo %s -o %t +// RUN: %run %t valid 2>&1 +// RUN: %run %t invalid 2>&1 + +// Tests that valloc and pvalloc work as intended. + +#include +#include +#include +#include +#include +#include + +size_t round_up_to(size_t size, size_t alignment) { + return (size + alignment - 1) & ~(alignment - 1); +} + +int main(int argc, char **argv) +{ + void *p = nullptr; + size_t size, page_size; + + assert(argc == 2); + + page_size = sysconf(_SC_PAGESIZE); + // Check that the page size is a power of two. + assert((page_size & (page_size - 1)) == 0); + + if (!strcmp(argv[1], "valid")) { + for (int i = (sizeof(void *) == 4) ? 3 : 4; i < 21; i++) { + size = 1U << i; + p = valloc(size - (2 * sizeof(void *))); + assert(p); + assert(((uintptr_t)p & (page_size - 1)) == 0); + free(p); + p = pvalloc(size - (2 * sizeof(void *))); + assert(p); + assert(((uintptr_t)p & (page_size - 1)) == 0); + assert(malloc_usable_size(p) >= round_up_to(size, page_size)); + free(p); + p = valloc(size); + assert(p); + assert(((uintptr_t)p & (page_size - 1)) == 0); + free(p); + p = pvalloc(size); + assert(p); + assert(((uintptr_t)p & (page_size - 1)) == 0); + assert(malloc_usable_size(p) >= round_up_to(size, page_size)); + free(p); + } + } + if (!strcmp(argv[1], "invalid")) { + // Size passed to pvalloc overflows when rounded up. + p = pvalloc((size_t)-1); + assert(!p); + assert(errno == ENOMEM); + errno = 0; + p = pvalloc((size_t)-page_size); + assert(!p); + assert(errno == ENOMEM); + } + return 0; +} -- 2.7.4