llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-driver
Author: edcsnt
<details>
<summary>Changes</summary>
cat >a.c <<EOF && clang -fsanitize=safe-stack -static a.c
&& ./a.out
void *f(void *p)
{
return NULL;
}
int main(void)
{
pthread_t t;
(void)pthread_create(&t, NULL, f, NULL);
return 0;
}
EOF
causes an invalid memory reference at 0x0 that generates SIGSEGV, as lldb -b -o
r -Q ./a.out will attest.
On the first call to pthread_create, the SafeStack interceptor calls
dlsym(RTLD_NEXT) to obtain the address of pthread_create. In a statically
linked executable file, there is no next executable object file to search in. A
null pointer is dereferenced when the interceptor attempts to call
REAL(pthread_create).
Declare a weak symbol for the implementation of pthread_create and fall back to
it if dlsym returns a null pointer. The Clang driver instructs the link editor
to extract the defining archive member if linking statically.
illumos and Solaris do not provide libc.a as [1] and [2] document.
The SafeStack runtime obtains the addresses of libc symbols for NetBSD almost
exclusively via dlsym [3]. Even if refactored to avoid dlsym, [4] itself
discourages static linking.
[1] https://www.illumos.org/issues/5247#note-7
[2] http://www.linker-aliens.org/blogs/rie/entry/static_linking_where_did_it/
[3]
https://github.com/llvm/llvm-project/blob/e3cbd9984a78422c3799629eb6b5f7f7818c1a11/compiler-rt/lib/safestack/safestack_platform.h#L66-L86
[4] https://man.netbsd.org/pthread.3#DESCRIPTION
---
Full diff: https://github.com/llvm/llvm-project/pull/190126.diff
3 Files Affected:
- (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+9)
- (modified) compiler-rt/lib/safestack/safestack.cpp (+45)
- (added) compiler-rt/test/safestack/pthread-static.c (+20)
``````````diff
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 286657a320087..57e147ab8446e 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1707,6 +1707,15 @@ collectSanitizerRuntimes(const ToolChain &TC, const
ArgList &Args,
if (SanArgs.needsSafeStackRt()) {
NonWholeStaticRuntimes.push_back("safestack");
RequiredSymbols.push_back("__safestack_init");
+ if (Args.hasArg(options::OPT_static) ||
+ Args.hasArg(options::OPT_static_pie)) {
+ if (TC.getTriple().isGNUEnvironment())
+ RequiredSymbols.push_back("__pthread_create_2_1");
+ else if (TC.getTriple().isOSLinux())
+ RequiredSymbols.push_back("__pthread_create");
+ else if (TC.getTriple().isOSFreeBSD())
+ RequiredSymbols.push_back("_pthread_create");
+ }
}
if (!(SanArgs.needsSharedRt() && SanArgs.needsUbsanRt())) {
if (SanArgs.needsCfiCrossDsoRt())
diff --git a/compiler-rt/lib/safestack/safestack.cpp
b/compiler-rt/lib/safestack/safestack.cpp
index f01a642987646..05cee3289bff1 100644
--- a/compiler-rt/lib/safestack/safestack.cpp
+++ b/compiler-rt/lib/safestack/safestack.cpp
@@ -62,6 +62,26 @@ __attribute__((visibility(
"default"))) __thread void *__safestack_unsafe_stack_ptr = nullptr;
}
+// These symbols may have no definition within the component being linked,
+// either because the archive library is not passed to the link editor or
+// because the shared object does not include them in its dynamic linking
+// symbol table. Give them an STB_WEAK binding so that, if left unresolved,
+// they have a zero value.
+#if SANITIZER_GLIBC
+extern "C"
+ __attribute__((weak)) int __pthread_create_2_1(pthread_t*,
+ const pthread_attr_t*,
+ void* (*)(void*), void*);
+#elif SANITIZER_LINUX
+extern "C" __attribute__((weak)) int __pthread_create(pthread_t*,
+ const pthread_attr_t*,
+ void* (*)(void*), void*);
+#elif SANITIZER_FREEBSD
+extern "C"
+ __attribute__((weak)) int _pthread_create(pthread_t*, const
pthread_attr_t*,
+ void* (*)(void*), void*);
+#endif
+
namespace {
// TODO: The runtime library does not currently protect the safe stack beyond
@@ -288,6 +308,24 @@ void EnsureInterceptorsInitialized() {
// Initialize pthread interceptors for thread allocation
INTERCEPT_FUNCTION(pthread_create);
+#if SANITIZER_GLIBC
+ // REAL(pthread_create) is null after INTERCEPT_FUNCTION(pthread_create) in
+ // statically linked executable files. Fall back to the weak symbol we
+ // declare.
+ //
+ // diet libc and uClibc-ng are not supported because interception requires a
+ // distinct symbol to call after replacing pthread_create; neither provides
+ // such a symbol.
+ if (!REAL(pthread_create))
+ REAL(pthread_create) = __pthread_create_2_1;
+#elif SANITIZER_LINUX
+ if (!REAL(pthread_create))
+ REAL(pthread_create) = __pthread_create;
+#elif SANITIZER_FREEBSD
+ if (!REAL(pthread_create))
+ REAL(pthread_create) = _pthread_create;
+#endif
+
interceptors_inited = true;
}
@@ -320,8 +358,15 @@ void __safestack_init() {
// On other platforms we use the constructor attribute to arrange to run our
// initialization early.
extern "C" {
+# if !SANITIZER_MUSL
__attribute__((section(".preinit_array"),
used)) void (*__safestack_preinit)(void) = __safestack_init;
+# else
+// musl does not execute .preinit_array functions.
+__attribute__((constructor(0))) void __safestack_init_fallback() {
+ __safestack_init();
+}
+# endif
}
#endif
diff --git a/compiler-rt/test/safestack/pthread-static.c
b/compiler-rt/test/safestack/pthread-static.c
new file mode 100644
index 0000000000000..fde4b174aaff2
--- /dev/null
+++ b/compiler-rt/test/safestack/pthread-static.c
@@ -0,0 +1,20 @@
+// RUN: %clang_safestack %s -pthread -static -o %t
+// RUN: %run %t
+
+// REQUIRES: linux || freebsd
+
+// Smoke test for pthread_create in a statically linked executable.
+
+#include <pthread.h>
+
+void *f(void *p)
+{
+ return NULL;
+}
+
+int main(void)
+{
+ pthread_t t;
+ (void)pthread_create(&t, NULL, f, NULL);
+ return 0;
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/190126
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
