Link shared sanitizer runtime with libresolv for dn_expand

[aaronpuchert]

Since the interceptor was added in 1a729bc, a process using sanitizers might need libresolv for the real version of __dn_expand. The linker flag for the static runtime was added in 6dce56b. But the dynamic runtime didn't get the dependency. This can cause crashes even when libresolv is not used directly, because it is also used by DNS lookups via e.g. getaddrinfo when the DNS reply uses compression.

We observed the issue with the address sanitizer, but a quick look into the shared runtime libraries revealed that the memprof and tsan runtimes also export dn_expand and should thus be prone to the same issue.

It should be mentioned that glibc plans to move libresolv into libc, which was partially completed in version 2.34 with the move of dn_comp and dn_expand. However, that is not an issue here: once the move is complete, there will only be a static libresolv.a stub left and so building against a version after the move will no longer produce a runtime dependency to libresolv.so.

Fixes #59007.

[vitalybuka]

What about other sanitizers?

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

@llvm/pr-subscribers-pgo

Author: Aaron Puchert (aaronpuchert)

Changes

Since the interceptor was added in 1a729bc, a process using sanitizers might need libresolv for the real version of __dn_expand. The linker flag for the static runtime was added in 6dce56b. But the dynamic runtime didn't get the dependency. This can cause crashes even when libresolv is not used directly, because it is also used by DNS lookups via e.g. getaddrinfo when the DNS reply uses compression.

We observed the issue with the address sanitizer, but a quick look into the shared runtime libraries revealed that the memprof and tsan runtimes also export dn_expand and should thus be prone to the same issue.

It should be mentioned that glibc plans to move libresolv into libc, which was partially completed in version 2.34 with the move of dn_comp and dn_expand. However, that is not an issue here: once the move is complete, there will only be a static libresolv.a stub left and so building against a version after the move will no longer produce a runtime dependency to libresolv.so.

Fixes #59007.

---

Full diff: https://github.com/llvm/llvm-project/pull/116415.diff

4 Files Affected:

• (modified) compiler-rt/cmake/config-ix.cmake (+1)
• (modified) compiler-rt/lib/asan/CMakeLists.txt (+1)
• (modified) compiler-rt/lib/memprof/CMakeLists.txt (+1)
• (modified) compiler-rt/lib/tsan/rtl/CMakeLists.txt (+1)

diff --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index 6d52eecc9a91fe..74f7fb4ac4880f 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -189,6 +189,7 @@ check_include_files("sys/auxv.h"    COMPILER_RT_HAS_AUXV)
 # Libraries.
 check_library_exists(dl dlopen "" COMPILER_RT_HAS_LIBDL)
 check_library_exists(rt shm_open "" COMPILER_RT_HAS_LIBRT)
+check_library_exists(resolv dn_expand "" COMPILER_RT_HAS_LIBRESOLV)
 check_library_exists(m pow "" COMPILER_RT_HAS_LIBM)
 check_library_exists(pthread pthread_create "" COMPILER_RT_HAS_LIBPTHREAD)
 check_library_exists(execinfo backtrace "" COMPILER_RT_HAS_LIBEXECINFO)
diff --git a/compiler-rt/lib/asan/CMakeLists.txt b/compiler-rt/lib/asan/CMakeLists.txt
index fb3d74283a61e0..7a92a1a187a816 100644
--- a/compiler-rt/lib/asan/CMakeLists.txt
+++ b/compiler-rt/lib/asan/CMakeLists.txt
@@ -153,6 +153,7 @@ append_list_if(COMPILER_RT_HAS_LIBRT rt ASAN_DYNAMIC_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBM m ASAN_DYNAMIC_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBPTHREAD pthread ASAN_DYNAMIC_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBLOG log ASAN_DYNAMIC_LIBS)
+append_list_if(COMPILER_RT_HAS_LIBRESOLV resolv ASAN_DYNAMIC_LIBS)
 append_list_if(MINGW "${MINGW_LIBRARIES}" ASAN_DYNAMIC_LIBS)
 
 # Compile ASan sources into an object library.
diff --git a/compiler-rt/lib/memprof/CMakeLists.txt b/compiler-rt/lib/memprof/CMakeLists.txt
index e6d99daca6ee7d..2d0df65f43a260 100644
--- a/compiler-rt/lib/memprof/CMakeLists.txt
+++ b/compiler-rt/lib/memprof/CMakeLists.txt
@@ -74,6 +74,7 @@ append_list_if(COMPILER_RT_HAS_LIBRT rt MEMPROF_DYNAMIC_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBM m MEMPROF_DYNAMIC_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBPTHREAD pthread MEMPROF_DYNAMIC_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBLOG log MEMPROF_DYNAMIC_LIBS)
+append_list_if(COMPILER_RT_HAS_LIBRESOLV resolv MEMPROF_DYNAMIC_LIBS)
 
 # Compile MemProf sources into an object library.
 
diff --git a/compiler-rt/lib/tsan/rtl/CMakeLists.txt b/compiler-rt/lib/tsan/rtl/CMakeLists.txt
index f40e72dbde1f98..a6e9c7960fd594 100644
--- a/compiler-rt/lib/tsan/rtl/CMakeLists.txt
+++ b/compiler-rt/lib/tsan/rtl/CMakeLists.txt
@@ -20,6 +20,7 @@ set(TSAN_DYNAMIC_LINK_LIBS
 append_list_if(COMPILER_RT_HAS_LIBDL dl TSAN_DYNAMIC_LINK_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBM m TSAN_DYNAMIC_LINK_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBPTHREAD pthread TSAN_DYNAMIC_LINK_LIBS)
+append_list_if(COMPILER_RT_HAS_LIBRESOLV resolv TSAN_DYNAMIC_LIBS)
 
 set(TSAN_SOURCES
   tsan_debugging.cpp

[aaronpuchert]

I based it on this:

$ for lib in *.so; do echo $lib; readelf --dyn-syms --wide $lib | grep dn_comp; done
libclang_rt.asan-x86_64.so
  1069: 000000000009b880  1263 FUNC    WEAK   DEFAULT   13 __interceptor_dn_comp
  1260: 000000000009b880  1263 FUNC    GLOBAL DEFAULT   13 ___interceptor_dn_comp
  1705: 0000000000082fec     5 FUNC    GLOBAL DEFAULT   13 __interceptor_trampoline_dn_comp
  2138: 0000000000082fec     5 FUNC    WEAK   DEFAULT   13 dn_comp
libclang_rt.dyndd-x86_64.so
libclang_rt.hwasan_aliases-x86_64.so
libclang_rt.hwasan-x86_64.so
libclang_rt.memprof-x86_64.so
   978: 0000000000069b10   203 FUNC    WEAK   DEFAULT   13 __interceptor_dn_comp
  1149: 0000000000069b10   203 FUNC    GLOBAL DEFAULT   13 ___interceptor_dn_comp
  1551: 0000000000061e98     5 FUNC    GLOBAL DEFAULT   13 __interceptor_trampoline_dn_comp
  1938: 0000000000061e98     5 FUNC    WEAK   DEFAULT   13 dn_comp
libclang_rt.nsan-x86_64.so
libclang_rt.scudo_standalone-x86_64.so
libclang_rt.tsan-x86_64.so
  1147: 000000000008e7f0   518 FUNC    WEAK   DEFAULT   13 __interceptor_dn_comp
  1350: 000000000008e7f0   518 FUNC    GLOBAL DEFAULT   13 ___interceptor_dn_comp
  1844: 0000000000070935     5 FUNC    GLOBAL DEFAULT   13 __interceptor_trampoline_dn_comp
  2318: 0000000000070935     5 FUNC    WEAK   DEFAULT   13 dn_comp
libclang_rt.ubsan_minimal-x86_64.so
libclang_rt.ubsan_standalone-x86_64.so

But this might be due to my build settings.

[aaronpuchert]

For HwASan, I have COMPILER_RT_HWASAN_WITH_INTERCEPTORS enabled. Still no sign of dn_comp or dn_expand.

For NSan the library should never be relevant. There is no floating-point in DNS.

The UBSan runtime doesn't intercept any functions, it only consists of error handlers.

[aaronpuchert]

One more: MSan does not seem to offer a shared runtime.

[aaronpuchert]

RTSan has a shared runtime only on Mac, i.e. if(APPLE). When I remove that if, I see no sign of dn_* in the shared library. Also no sign in rtsan_interceptors_posix.cpp.

My understanding is that at least dn_comp and dn_expand don't block. Other functions in resolv can block, but we don't intercept them at the moment.

[vitalybuka]

Maybe as alternative solution we convert this into MSAN-only interceptor and avoid linking more stuff into asan?

[aaronpuchert]

The interceptors are definitely tailored towards MSan, where they're necessary to address false positives. They don't even check for reads, just register the writes. But I'm not sure where to disable it. HWASan has hwasan_platform_interceptors.h, where lots of interceptors are disabled, but I can't find an equivalent place for ASan.

The additional link dependency might also be temporary: my impression is that glibc wants to integrate all sublibraries into the main library.

[cjappl]

My understanding is that at least dn_comp and dn_expand don't block. Other functions in resolv can block, but we don't intercept them at the moment.

I would suspect these calls (dn_comp, dn_expand and others) should be intercepted, but we haven't gotten around to them yet. Any PR welcome to intercept this or other calls, but no worries if you don't want to follow up.

"blocking" in the case of rtsan means unsafe for real-time code (contains a syscall of any type). I would guess that any query to the DNS subsystem results in a system call.

[aaronpuchert]

"blocking" in the case of rtsan means unsafe for real-time code (contains a syscall of any type). I would guess that any query to the DNS subsystem results in a system call.

My understanding is that they don't do any syscalls, they just "compress" or "expand" domain names for a DNS query or reply. Since they write into buffers allocated by the caller, I think they don't even do any allocations. But I didn't check this hypothesis.

[aaronpuchert]

@vitalybuka, I forgot to mention that we already link with -lresolv, but only in the static runtime case. This was done in 6dce56b and applies to all sanitizers, as far as I can tell. So here we're just bringing the shared runtime in line with that.

But I have no issue with disabling the interceptor for the address sanitizer, I'm just not sure where to put that yet.

[vitalybuka]

The interceptors are definitely tailored towards MSan, where they're necessary to address false positives. They don't even check for reads, just register the writes. But I'm not sure where to disable it. HWASan has hwasan_platform_interceptors.h, where lots of interceptors are disabled, but I can't find an equivalent place for ASan.

The additional link dependency might also be temporary: my impression is that glibc wants to integrate all sublibraries into the main library.

They need to be moved from sanitizer_common into msan_iterceptors

[aaronpuchert]

One reason why it might have ended up in the common interceptors is this comment in msan_interceptors.cpp:

// FIXME: move as many interceptors as possible into
// sanitizer_common/sanitizer_common_interceptors.h

I have no problem with moving in the opposite direction, but maybe this comment should be removed? Maybe interceptors should only move into the common area if they're actually interesting for multiple sanitizers?

[vitalybuka]

I have no problem with moving in the opposite direction, but maybe this comment should be removed? Maybe interceptors should only move into the common area if they're actually interesting for multiple sanitizers?

yes, this does not look relevant to me

[aaronpuchert]

Posted the move as separate pull request #119071, will close this change if that is approved.

[vitalybuka]

I assume we go with another one

[aaronpuchert]

We decided to move the interceptors to MSan instead, so this change is no longer needed.