Skip to content

Commit c2de592

Browse files
stelfragstelfrag
authored
fix(build): make AddressSanitizer detect pooled allocator use-after-free (#23566)
fix(aral): enhance AddressSanitizer integration for accurate memory checks
1 parent 855fbba commit c2de592

3 files changed

Lines changed: 32 additions & 5 deletions

File tree

‎.github/workflows/topology-container-tests.yml‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,15 @@ jobs:
173173
libprotobuf-dev protobuf-compiler
174174
175175
- name: Configure
176-
env:
177-
CFLAGS: -fsanitize=address -fno-omit-frame-pointer
178-
CXXFLAGS: -fsanitize=address -fno-omit-frame-pointer
179-
LDFLAGS: -fsanitize=address
180176
run: |
177+
# ENABLE_ADDRESS_SANITIZER adds the sanitizer flags AND defines
178+
# FSANITIZE_ADDRESS, which is what makes libnetdata's pooled
179+
# allocators fall back to plain malloc/free. Setting the flags by hand
180+
# (as this step used to) instruments the build but leaves the pools
181+
# intact, so ASan cannot see a use-after-free inside ARAL, the
182+
# dictionary allocators, STRING or onewayalloc.
181183
cmake -S . -B build -G Ninja \
184+
-DENABLE_ADDRESS_SANITIZER=On \
182185
-DCMAKE_BUILD_TYPE=Debug \
183186
-DENABLE_PLUGIN_GO=OFF \
184187
-DENABLE_PLUGIN_XENSTAT=OFF \

‎packaging/cmake/Modules/NetdataCompilerFlags.cmake‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,22 @@ option(ENABLE_ADDRESS_SANITIZER "Build with address sanitizer enabled" False)
9797
mark_as_advanced(ENABLE_ADDRESS_SANITIZER)
9898

9999
if(ENABLE_ADDRESS_SANITIZER)
100-
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address")
100+
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -fno-omit-frame-pointer")
101+
102+
# Instrumentation alone is not enough. Large parts of libnetdata allocate from
103+
# pools (ARAL, the dictionary allocators, STRING, onewayalloc), so a freed
104+
# object stays inside a page that is still allocated and AddressSanitizer sees
105+
# nothing. Those subsystems fall back to plain malloc/free only when
106+
# FSANITIZE_ADDRESS is defined, which nothing set until now - so an
107+
# ENABLE_ADDRESS_SANITIZER build was blind to use-after-free in every pooled
108+
# allocator. Define it here so the option means what its name implies.
109+
add_compile_definitions(FSANITIZE_ADDRESS)
101110
endif()
102111

112+
# CMAKE_CXX_FLAGS picks the above up further down this file, and CMake uses
113+
# CMAKE_CXX_FLAGS on the link line too, so no separate CXX or linker flag is
114+
# needed here.
115+
103116
if(STATIC_BUILD)
104117
add_required_compiler_flag("-static")
105118

‎src/libnetdata/aral/aral.c‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2507,6 +2507,16 @@ int aral_stress_test(size_t threads, size_t elements, size_t seconds) {
25072507
}
25082508

25092509
int aral_unittest(size_t elements) {
2510+
#if defined(FSANITIZE_ADDRESS)
2511+
// Under address sanitizer ARAL is bypassed entirely: aral_mallocz() and
2512+
// aral_freez() delegate straight to glibc, so there is no pool to stress.
2513+
// The stress loop below would still run and still report PASSED, but with
2514+
// "did 0 malloc, 0 free" - a green result that proves nothing. Skip
2515+
// explicitly instead, the same way aral_unittest_concurrency() does.
2516+
(void)elements;
2517+
fprintf(stderr, "ARAL unittest: SKIPPED (ARAL is disabled under FSANITIZE_ADDRESS)\n");
2518+
return 0;
2519+
#else
25102520
const char *cache_dir = "/tmp/";
25112521
#ifdef NETDATA_INTERNAL_CHECKS
25122522
int errors = aral_detect_acquire_to_page_lock_race();
@@ -2544,4 +2554,5 @@ int aral_unittest(size_t elements) {
25442554
fprintf(stderr, "ARAL unittest: %s (%d errors)\n", total_errors ? "FAILED" : "PASSED", total_errors);
25452555

25462556
return total_errors;
2557+
#endif // FSANITIZE_ADDRESS
25472558
}

0 commit comments

Comments
 (0)