[PATCH] [ASan] fix aligned_alloc false positive

Paul Eggert via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 26 23:46:19 PST 2022


Fix bug that caused aligned_alloc to mistakenly reject valid sizes.
ASan aligned_alloc mistakenly required the size to be a multiple of
the alignment.  However, C11 allows calls like
aligned_alloc(alignof(struct X), offsetof(struct X, M) + N), which
should allocate a struct X with a flexible array member M even when N
is not a multiple of alignof(struct X).  This bug broke an experimental
version of GNU coreutils that was compiled with address sanitization
checking.

Although this patch fixes the bug, it does not attempt to clean up
afterwards by removing functions like CheckAlignedAllocAlignmentAndSize
and ReportInvalidAlignedAllocAlignment that are no longer needed.

Here is a C program that illustrates the bug.  This program
should exit quietly with some status, but AddressSanitizer mistakenly
reports "invalid alignment requested in aligned_alloc: 8, alignment
must be a power of two and the requested size 0x9 must be a multiple
of alignment".

  #include <stdalign.h>
  #include <stddef.h>
  #include <stdlib.h>
  #include <string.h>

  struct namelist
  {
    struct namelist *next;
    char name[];
  };

  char const *sample_argv[] = { "", "a", "bb", "ccc", "dddd", 0 };

  int
  main (void)
  {
    struct namelist *ls = NULL;

    for (char const **argv = sample_argv; *argv; argv++)
      {
	size_t len = strlen (*argv);
	struct namelist *nu =
	  aligned_alloc (alignof (struct namelist),
			 offsetof (struct namelist, name) + len + 1);
	if (!nu)
	  return 1;
	nu->next = ls;
	strcpy (nu->name, *argv);
	ls = nu;
      }

    int status = 0;
    for (; ls; ls = ls->next)
      status ^= ls->name[0];
    return status == 1 ? 0 : status;
  }
---
 compiler-rt/lib/asan/asan_allocator.cpp       | 9 +--------
 compiler-rt/lib/dfsan/dfsan_allocator.cpp     | 9 +--------
 compiler-rt/lib/hwasan/hwasan_allocator.cpp   | 8 +-------
 compiler-rt/lib/lsan/lsan_allocator.cpp       | 8 +-------
 compiler-rt/lib/memprof/memprof_allocator.cpp | 8 +-------
 compiler-rt/lib/msan/msan_allocator.cpp       | 8 +-------
 compiler-rt/lib/scudo/scudo_allocator.cpp     | 8 +-------
 compiler-rt/lib/tsan/rtl-old/tsan_mman.cpp    | 9 +--------
 compiler-rt/lib/tsan/rtl/tsan_mman.cpp        | 9 +--------
 9 files changed, 9 insertions(+), 67 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_allocator.cpp b/compiler-rt/lib/asan/asan_allocator.cpp
index f9f1cfcd9f87..42509ce4dd49 100644
--- a/compiler-rt/lib/asan/asan_allocator.cpp
+++ b/compiler-rt/lib/asan/asan_allocator.cpp
@@ -1014,14 +1014,7 @@ void *asan_memalign(uptr alignment, uptr size, BufferedStackTrace *stack,
 }
 
 void *asan_aligned_alloc(uptr alignment, uptr size, BufferedStackTrace *stack) {
-  if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(alignment, size))) {
-    errno = errno_EINVAL;
-    if (AllocatorMayReturnNull())
-      return nullptr;
-    ReportInvalidAlignedAllocAlignment(size, alignment, stack);
-  }
-  return SetErrnoOnNull(
-      instance.Allocate(size, alignment, stack, FROM_MALLOC, true));
+  return asan_memalign(alignment, size, stack, FROM_MALLOC);
 }
 
 int asan_posix_memalign(void **memptr, uptr alignment, uptr size,
diff --git a/compiler-rt/lib/dfsan/dfsan_allocator.cpp b/compiler-rt/lib/dfsan/dfsan_allocator.cpp
index c50aee7a55a0..e5b1a1dfeb74 100644
--- a/compiler-rt/lib/dfsan/dfsan_allocator.cpp
+++ b/compiler-rt/lib/dfsan/dfsan_allocator.cpp
@@ -229,14 +229,7 @@ void *dfsan_pvalloc(uptr size) {
 }
 
 void *dfsan_aligned_alloc(uptr alignment, uptr size) {
-  if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(alignment, size))) {
-    errno = errno_EINVAL;
-    if (AllocatorMayReturnNull())
-      return nullptr;
-    BufferedStackTrace stack;
-    ReportInvalidAlignedAllocAlignment(size, alignment, &stack);
-  }
-  return SetErrnoOnNull(DFsanAllocate(size, alignment, false /*zeroise*/));
+  return dfsan_memalign(alignment, size);
 }
 
 void *dfsan_memalign(uptr alignment, uptr size) {
diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.cpp b/compiler-rt/lib/hwasan/hwasan_allocator.cpp
index 84e183f2384f..e70b9b691997 100644
--- a/compiler-rt/lib/hwasan/hwasan_allocator.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_allocator.cpp
@@ -410,13 +410,7 @@ void *hwasan_pvalloc(uptr size, StackTrace *stack) {
 }
 
 void *hwasan_aligned_alloc(uptr alignment, uptr size, StackTrace *stack) {
-  if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(alignment, size))) {
-    errno = errno_EINVAL;
-    if (AllocatorMayReturnNull())
-      return nullptr;
-    ReportInvalidAlignedAllocAlignment(size, alignment, stack);
-  }
-  return SetErrnoOnNull(HwasanAllocate(stack, size, alignment, false));
+  return hwasan_memalign(alignment, size, stack);
 }
 
 void *hwasan_memalign(uptr alignment, uptr size, StackTrace *stack) {
diff --git a/compiler-rt/lib/lsan/lsan_allocator.cpp b/compiler-rt/lib/lsan/lsan_allocator.cpp
index ea4c6c9cf647..b64332ace85b 100644
--- a/compiler-rt/lib/lsan/lsan_allocator.cpp
+++ b/compiler-rt/lib/lsan/lsan_allocator.cpp
@@ -170,13 +170,7 @@ int lsan_posix_memalign(void **memptr, uptr alignment, uptr size,
 }
 
 void *lsan_aligned_alloc(uptr alignment, uptr size, const StackTrace &stack) {
-  if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(alignment, size))) {
-    errno = errno_EINVAL;
-    if (AllocatorMayReturnNull())
-      return nullptr;
-    ReportInvalidAlignedAllocAlignment(size, alignment, &stack);
-  }
-  return SetErrnoOnNull(Allocate(stack, size, alignment, kAlwaysClearMemory));
+  return lsan_memalign(alignment, size, stack);
 }
 
 void *lsan_memalign(uptr alignment, uptr size, const StackTrace &stack) {
diff --git a/compiler-rt/lib/memprof/memprof_allocator.cpp b/compiler-rt/lib/memprof/memprof_allocator.cpp
index 0974b898666b..e7eb5ef337e2 100644
--- a/compiler-rt/lib/memprof/memprof_allocator.cpp
+++ b/compiler-rt/lib/memprof/memprof_allocator.cpp
@@ -621,13 +621,7 @@ void *memprof_memalign(uptr alignment, uptr size, BufferedStackTrace *stack,
 
 void *memprof_aligned_alloc(uptr alignment, uptr size,
                             BufferedStackTrace *stack) {
-  if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(alignment, size))) {
-    errno = errno_EINVAL;
-    if (AllocatorMayReturnNull())
-      return nullptr;
-    ReportInvalidAlignedAllocAlignment(size, alignment, stack);
-  }
-  return SetErrnoOnNull(instance.Allocate(size, alignment, stack, FROM_MALLOC));
+  return memprof_memalign(alignment, size, stack, FROM_MALLOC);
 }
 
 int memprof_posix_memalign(void **memptr, uptr alignment, uptr size,
diff --git a/compiler-rt/lib/msan/msan_allocator.cpp b/compiler-rt/lib/msan/msan_allocator.cpp
index dc006457a59f..7f1061c98c1d 100644
--- a/compiler-rt/lib/msan/msan_allocator.cpp
+++ b/compiler-rt/lib/msan/msan_allocator.cpp
@@ -313,13 +313,7 @@ void *msan_pvalloc(uptr size, StackTrace *stack) {
 }
 
 void *msan_aligned_alloc(uptr alignment, uptr size, StackTrace *stack) {
-  if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(alignment, size))) {
-    errno = errno_EINVAL;
-    if (AllocatorMayReturnNull())
-      return nullptr;
-    ReportInvalidAlignedAllocAlignment(size, alignment, stack);
-  }
-  return SetErrnoOnNull(MsanAllocate(stack, size, alignment, false));
+  return msan_memalign(alignment, size, stack);
 }
 
 void *msan_memalign(uptr alignment, uptr size, StackTrace *stack) {
diff --git a/compiler-rt/lib/scudo/scudo_allocator.cpp b/compiler-rt/lib/scudo/scudo_allocator.cpp
index 5b6ac8b35493..10b56c07dc5f 100644
--- a/compiler-rt/lib/scudo/scudo_allocator.cpp
+++ b/compiler-rt/lib/scudo/scudo_allocator.cpp
@@ -759,13 +759,7 @@ int scudoPosixMemalign(void **MemPtr, uptr Alignment, uptr Size) {
 }
 
 void *scudoAlignedAlloc(uptr Alignment, uptr Size) {
-  if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(Alignment, Size))) {
-    errno = EINVAL;
-    if (Instance.canReturnNull())
-      return nullptr;
-    reportInvalidAlignedAllocAlignment(Size, Alignment);
-  }
-  return SetErrnoOnNull(Instance.allocate(Size, Alignment, FromMalloc));
+  return scudoAllocate(Size, Alignment, FromMalloc);
 }
 
 uptr scudoMallocUsableSize(void *Ptr) {
diff --git a/compiler-rt/lib/tsan/rtl-old/tsan_mman.cpp b/compiler-rt/lib/tsan/rtl-old/tsan_mman.cpp
index 86a3dcd332b2..ca2eefc18a14 100644
--- a/compiler-rt/lib/tsan/rtl-old/tsan_mman.cpp
+++ b/compiler-rt/lib/tsan/rtl-old/tsan_mman.cpp
@@ -313,14 +313,7 @@ int user_posix_memalign(ThreadState *thr, uptr pc, void **memptr, uptr align,
 }
 
 void *user_aligned_alloc(ThreadState *thr, uptr pc, uptr align, uptr sz) {
-  if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(align, sz))) {
-    errno = errno_EINVAL;
-    if (AllocatorMayReturnNull())
-      return nullptr;
-    GET_STACK_TRACE_FATAL(thr, pc);
-    ReportInvalidAlignedAllocAlignment(sz, align, &stack);
-  }
-  return SetErrnoOnNull(user_alloc_internal(thr, pc, sz, align));
+  return user_memalign(thr, pc, align, sz);
 }
 
 void *user_valloc(ThreadState *thr, uptr pc, uptr sz) {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
index 00cc3a306fd3..e478ee30eb8b 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
@@ -335,14 +335,7 @@ int user_posix_memalign(ThreadState *thr, uptr pc, void **memptr, uptr align,
 }
 
 void *user_aligned_alloc(ThreadState *thr, uptr pc, uptr align, uptr sz) {
-  if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(align, sz))) {
-    errno = errno_EINVAL;
-    if (AllocatorMayReturnNull())
-      return nullptr;
-    GET_STACK_TRACE_FATAL(thr, pc);
-    ReportInvalidAlignedAllocAlignment(sz, align, &stack);
-  }
-  return SetErrnoOnNull(user_alloc_internal(thr, pc, sz, align));
+  return user_memalign(thr, pc, align, sz);
 }
 
 void *user_valloc(ThreadState *thr, uptr pc, uptr sz) {
-- 
2.32.0



More information about the llvm-commits mailing list