Skip to content
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.

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

Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (0x402, 0x400)) with preload #51620

Closed
hjl-tools opened this issue Oct 24, 2021 · 16 comments
Labels
bugzilla Issues migrated from bugzilla compiler-rt:asan Address sanitizer release:reviewed

Comments

@hjl-tools
Copy link
Contributor

Bugzilla Link 52278
Resolution FIXED
Resolved on Nov 20, 2021 06:16
Version unspecified
OS Linux
Blocks #51489
CC @delcypher,@nickdesaulniers,@Lekensteyn,@tstellar,@vitalybuka

Extended Description

With glibc 2.34 on Linux/x86-64, LLVM 13.0.0 rc1 gave me:

[hjl@gnu-skx-1 gcc]$ cat x.c
#include <assert.h>

struct A {
char a[3];
int b[3];
};

volatile int ten = 10;

attribute((noinline)) void foo(int index, int len) {
volatile struct A str[len] attribute((aligned(32)));
assert(!((long) str & 31L));
str[index].a[0] = '1'; // BOOM
}

int main(int argc, char **argv) {
foo(ten, ten);
return 0;
}
[hjl@gnu-skx-1 gcc]$ clang -O0 -fsanitize=address x.c -shared-libasan -m32
[hjl@gnu-skx-1 gcc]$ LD_PRELOAD=/export/users/hjl/build/gnu/tools-build/gcc-debug/build-x86_64-linux/x86_64-pc-linux-gnu/32/libsanitizer/libclang_rt.asan-i386.so ./a.out
AddressSanitizer: CHECK failed: asan_malloc_linux.cpp:46 "((allocated_for_dlsym)) < ((kDlsymAllocPoolSize))" (0x402, 0x400) (tid=3485465)

[hjl@gnu-skx-1 gcc]$

depending on the directory length where libclang_rt.asan-i386.so is placed.

@hjl-tools
Copy link
Contributor Author

A simpler reproducer:

[hjl@gnu-skx-1 gcc]$ LD_PRELOAD=/tmp/export-users-hjl-build-gnu-tools-build-gcc-debug-build-x86_64-linux-x86_64-pc-linux-gnu/libclang_rt.asan-i386.so ./a.out
AddressSanitizer: CHECK failed: asan_malloc_linux.cpp:46 "((allocated_for_dlsym)) < ((kDlsymAllocPoolSize))" (0x405, 0x400) (tid=3485517)

[hjl@gnu-skx-1 gcc]$

@hjl-tools
Copy link
Contributor Author

In glibc 2.34, dlsym does

  1. malloc 1
  2. malloc 2
  3. free pointer from malloc 1
  4. free pointer from malloc 2

so that
static void DeallocateFromLocalPool(const void *ptr) {
// Hack: since glibc 2.27 dlsym no longer uses stack-allocated memory to store // error messages and instead uses malloc followed by free. To avoid pool
// exhaustion due to long object filenames, handle that special case here.
uptr prev_offset = allocated_for_dlsym - last_dlsym_alloc_size_in_words;
void prev_mem = (void)&alloc_memory_for_dlsym[prev_offset];
if (prev_mem == ptr) {
REAL(memset)(prev_mem, 0, last_dlsym_alloc_size_in_words * kWordSize);
allocated_for_dlsym = prev_offset;
last_dlsym_alloc_size_in_words = 0;
}
}

has a memory leak.

@hjl-tools
Copy link
Contributor Author

A patch

@vitalybuka
Copy link
Collaborator

Reproducible by bots on Ubuntu 21.10 https://lab.llvm.org/staging/#/builders/19/builds/654/steps/8/logs/stdio

@vitalybuka
Copy link
Collaborator

Fixed with cb0e14c

But if we want to cherry-pick we need 4 patches.

1da33a5 [NFC][sanitizer] Move GET_MALLOC_STACK_TRACE closer to the use
64d4420 [NFC][lsan] Simplify root_regions initialization
651797f [NFC][asan][memprov] Remove dlsym hack from posix_memalign
cb0e14c [sanitizer] Switch dlsym hack to internal_allocator

@vitalybuka
Copy link
Collaborator

And

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_dlsym.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_dlsym.h
index 92b1373ef84d..8ae6f7cce75b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_dlsym.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_dlsym.h
@@ -25,7 +25,7 @@ struct DlSymAllocator {
return !SANITIZER_FUCHSIA && UNLIKELY(Details::UseImpl());
}

  • static bool PointerIsMine(const void *ptr) {
  • static bool PointerIsMine(void *ptr) {
    // Fuchsia doesn't use dlsym-based interceptors.
    return !SANITIZER_FUCHSIA &&
    UNLIKELY(internal_allocator()->FromPrimary(ptr));

@vitalybuka
Copy link
Collaborator

This is not a regression in LLVM, it's changes in glibc.

I am not sure if it safe to cherry-pick all these patches into release today. Maybe after a couple of weeks?

If someone can test, maybe this trivial change is safe and good enough?
kDlsymAllocPoolSize = 1024
to
kDlsymAllocPoolSize = 2048

@hjl-tools
Copy link
Contributor Author

hjl-tools commented Nov 20, 2021

This is not a regression in LLVM, it's changes in glibc.

I am not sure if it safe to cherry-pick all these patches into release
today. Maybe after a couple of weeks?

If someone can test, maybe this trivial change is safe and good enough?
kDlsymAllocPoolSize = 1024
to
kDlsymAllocPoolSize = 2048

I tested this on release/13.x branch on Fedora 35 with glibc 2.34:

diff --git a/compiler-rt/lib/asan/asan_malloc_linux.cpp b/compiler-rt/lib/asan/asan_malloc_linux.cpp
index c6bec8551bc5..b4552428f425 100644
--- a/compiler-rt/lib/asan/asan_malloc_linux.cpp
+++ b/compiler-rt/lib/asan/asan_malloc_linux.cpp
@@ -30,7 +30,7 @@ using namespace __asan;
 
 static uptr allocated_for_dlsym;
 static uptr last_dlsym_alloc_size_in_words;
-static const uptr kDlsymAllocPoolSize = 1024;
+static const uptr kDlsymAllocPoolSize = 2048;
 static uptr alloc_memory_for_dlsym[kDlsymAllocPoolSize];
 
 static inline bool IsInDlsymAllocPool(const void *ptr) {

It fixes the problem.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
@mgorny
Copy link
Member

mgorny commented Dec 13, 2021

Reopening for 13.x backport.

The change from 1024 to 2048 fixes most of the test failures for me (on Gentoo amd64) but not all of them. I'm trying 4096 now.

@mgorny mgorny reopened this Dec 13, 2021
@mgorny mgorny added this to the LLVM 13.0.1 release milestone Dec 13, 2021
@mgorny
Copy link
Member

mgorny commented Dec 13, 2021

The smallest power of 2 that I've been able to get AddressSanitizer-i386-linux :: TestCases/Linux/long-object-path.cpp to pass is 8192.

@tstellar
Copy link
Collaborator

The smallest power of 2 that I've been able to get AddressSanitizer-i386-linux :: TestCases/Linux/long-object-path.cpp to pass is 8192.

@vitalybuka Is it OK to bump the size this high on the release/13.x branch?

@vitalybuka
Copy link
Collaborator

Yes

@tstellar
Copy link
Collaborator

@mgorny Can you prepare a branch with this change and push it to a github fork?

@mgorny
Copy link
Member

mgorny commented Dec 16, 2021

@tstellar
Copy link
Collaborator

/branch mgorny/llvm-project/13-kdlsymalloc

tstellar pushed a commit that referenced this issue Dec 17, 2021
Increase kDlsymAllocPoolSize on the release branch as discussed on bug
51620, as an alternative to backporting
cb0e14c and its dependencies.
The minimum size is 8192, as needed for the following test to pass:

  AddressSanitizer-i386-linux :: TestCases/Linux/long-object-path.cpp

Fixes #51620
@tstellar
Copy link
Collaborator

Merged: d96358a

cyruscyliu added a commit to HexHive/Tango that referenced this issue Jan 3, 2024
…le check

Ref: llvm/llvm-project#51620
Note: currently, clang-13 supports 8192 bytes which is still not enough
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla compiler-rt:asan Address sanitizer release:reviewed
Projects
None yet
Development

No branches or pull requests

4 participants