[libc-commits] [libc] 3568521 - [libc] fix issues around stack protector (#74567)
via libc-commits
libc-commits at lists.llvm.org
Tue Dec 12 12:31:55 PST 2023
Author: Schrodinger ZHU Yifan
Date: 2023-12-12T12:31:51-08:00
New Revision: 3568521e3d2d09d12bd0c476d0376de165b19178
URL: https://github.com/llvm/llvm-project/commit/3568521e3d2d09d12bd0c476d0376de165b19178
DIFF: https://github.com/llvm/llvm-project/commit/3568521e3d2d09d12bd0c476d0376de165b19178.diff
LOG: [libc] fix issues around stack protector (#74567)
If a function is declared with stack-protector, the compiler may try to
load the TLS. However, inside certain runtime functions, TLS may not be
available. This patch disables stack protectors for such routines to fix
the problem.
Closes #74487.
Added:
Modified:
libc/src/__support/OSUtil/linux/quick_exit.h
libc/startup/linux/aarch64/start.cpp
libc/startup/linux/riscv/start.cpp
libc/startup/linux/x86_64/CMakeLists.txt
libc/startup/linux/x86_64/start.cpp
Removed:
################################################################################
diff --git a/libc/src/__support/OSUtil/linux/quick_exit.h b/libc/src/__support/OSUtil/linux/quick_exit.h
index c461d5f8b8fcb6..432395584d8467 100644
--- a/libc/src/__support/OSUtil/linux/quick_exit.h
+++ b/libc/src/__support/OSUtil/linux/quick_exit.h
@@ -9,7 +9,7 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_QUICK_EXIT_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_QUICK_EXIT_H
-#include "syscall.h" // For internal syscall function.
+#include "syscall.h" // For internal syscall function.
#include "src/__support/common.h"
@@ -17,7 +17,13 @@
namespace LIBC_NAMESPACE {
-LIBC_INLINE void quick_exit(int status) {
+// mark as no_stack_protector for x86 since TLS can be torn down before calling
+// quick_exit so that the stack protector canary cannot be loaded.
+#ifdef LIBC_TARGET_ARCH_IS_X86
+__attribute__((no_stack_protector))
+#endif
+LIBC_INLINE void
+quick_exit(int status) {
for (;;) {
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit_group, status);
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit, status);
diff --git a/libc/startup/linux/aarch64/start.cpp b/libc/startup/linux/aarch64/start.cpp
index b5c426866b56d7..c3e20eb09e4b42 100644
--- a/libc/startup/linux/aarch64/start.cpp
+++ b/libc/startup/linux/aarch64/start.cpp
@@ -184,7 +184,9 @@ __attribute__((noinline)) static void do_start() {
app.tls.align = phdr->p_align;
}
- LIBC_NAMESPACE::TLSDescriptor tls;
+ // This descriptor has to be static since its cleanup function cannot
+ // capture the context.
+ static LIBC_NAMESPACE::TLSDescriptor tls;
LIBC_NAMESPACE::init_tls(tls);
if (tls.size != 0)
LIBC_NAMESPACE::set_thread_ptr(tls.tp);
@@ -192,7 +194,11 @@ __attribute__((noinline)) static void do_start() {
LIBC_NAMESPACE::self.attrib = &LIBC_NAMESPACE::main_thread_attrib;
LIBC_NAMESPACE::main_thread_attrib.atexit_callback_mgr =
LIBC_NAMESPACE::internal::get_thread_atexit_callback_mgr();
-
+ // We register the cleanup_tls function to be the last atexit callback to be
+ // invoked. It will tear down the TLS. Other callbacks may depend on TLS (such
+ // as the stack protector canary).
+ LIBC_NAMESPACE::atexit(
+ []() { LIBC_NAMESPACE::cleanup_tls(tls.tp, tls.size); });
// We want the fini array callbacks to be run after other atexit
// callbacks are run. So, we register them before running the init
// array callbacks as they can potentially register their own atexit
@@ -208,10 +214,6 @@ __attribute__((noinline)) static void do_start() {
reinterpret_cast<char **>(app.args->argv),
reinterpret_cast<char **>(env_ptr));
- // TODO: TLS cleanup should be done after all other atexit callbacks
- // are run. So, register a cleanup callback for it with atexit before
- // everything else.
- LIBC_NAMESPACE::cleanup_tls(tls.addr, tls.size);
LIBC_NAMESPACE::exit(retval);
}
diff --git a/libc/startup/linux/riscv/start.cpp b/libc/startup/linux/riscv/start.cpp
index bf04be5ad14ad1..4d37662ccea13c 100644
--- a/libc/startup/linux/riscv/start.cpp
+++ b/libc/startup/linux/riscv/start.cpp
@@ -187,7 +187,9 @@ __attribute__((noinline)) static void do_start() {
app.tls.align = phdr->p_align;
}
- LIBC_NAMESPACE::TLSDescriptor tls;
+ // This descriptor has to be static since its cleanup function cannot
+ // capture the context.
+ static LIBC_NAMESPACE::TLSDescriptor tls;
LIBC_NAMESPACE::init_tls(tls);
if (tls.size != 0)
LIBC_NAMESPACE::set_thread_ptr(tls.tp);
@@ -195,7 +197,11 @@ __attribute__((noinline)) static void do_start() {
LIBC_NAMESPACE::self.attrib = &LIBC_NAMESPACE::main_thread_attrib;
LIBC_NAMESPACE::main_thread_attrib.atexit_callback_mgr =
LIBC_NAMESPACE::internal::get_thread_atexit_callback_mgr();
-
+ // We register the cleanup_tls function to be the last atexit callback to be
+ // invoked. It will tear down the TLS. Other callbacks may depend on TLS (such
+ // as the stack protector canary).
+ LIBC_NAMESPACE::atexit(
+ []() { LIBC_NAMESPACE::cleanup_tls(tls.tp, tls.size); });
// We want the fini array callbacks to be run after other atexit
// callbacks are run. So, we register them before running the init
// array callbacks as they can potentially register their own atexit
@@ -211,10 +217,6 @@ __attribute__((noinline)) static void do_start() {
reinterpret_cast<char **>(app.args->argv),
reinterpret_cast<char **>(env_ptr));
- // TODO: TLS cleanup should be done after all other atexit callbacks
- // are run. So, register a cleanup callback for it with atexit before
- // everything else.
- LIBC_NAMESPACE::cleanup_tls(tls.addr, tls.size);
LIBC_NAMESPACE::exit(retval);
}
diff --git a/libc/startup/linux/x86_64/CMakeLists.txt b/libc/startup/linux/x86_64/CMakeLists.txt
index 076c0c3e444f56..aac5a0626a176a 100644
--- a/libc/startup/linux/x86_64/CMakeLists.txt
+++ b/libc/startup/linux/x86_64/CMakeLists.txt
@@ -15,6 +15,7 @@ add_startup_object(
libc.src.string.memory_utils.inline_memcpy
libc.src.unistd.environ
COMPILE_OPTIONS
+ -fno-stack-protector
-fno-omit-frame-pointer
-ffreestanding # To avoid compiler warnings about calling the main function.
-fno-builtin
diff --git a/libc/startup/linux/x86_64/start.cpp b/libc/startup/linux/x86_64/start.cpp
index bc1b4f0487f316..496105dfd0b43a 100644
--- a/libc/startup/linux/x86_64/start.cpp
+++ b/libc/startup/linux/x86_64/start.cpp
@@ -222,7 +222,9 @@ extern "C" void _start() {
app.tls.align = phdr->p_align;
}
- LIBC_NAMESPACE::TLSDescriptor tls;
+ // This descriptor has to be static since its cleanup function cannot
+ // capture the context.
+ static LIBC_NAMESPACE::TLSDescriptor tls;
LIBC_NAMESPACE::init_tls(tls);
if (tls.size != 0 && !LIBC_NAMESPACE::set_thread_ptr(tls.tp))
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit, 1);
@@ -230,7 +232,11 @@ extern "C" void _start() {
LIBC_NAMESPACE::self.attrib = &LIBC_NAMESPACE::main_thread_attrib;
LIBC_NAMESPACE::main_thread_attrib.atexit_callback_mgr =
LIBC_NAMESPACE::internal::get_thread_atexit_callback_mgr();
-
+ // We register the cleanup_tls function to be the last atexit callback to be
+ // invoked. It will tear down the TLS. Other callbacks may depend on TLS (such
+ // as the stack protector canary).
+ LIBC_NAMESPACE::atexit(
+ []() { LIBC_NAMESPACE::cleanup_tls(tls.tp, tls.size); });
// We want the fini array callbacks to be run after other atexit
// callbacks are run. So, we register them before running the init
// array callbacks as they can potentially register their own atexit
@@ -246,9 +252,5 @@ extern "C" void _start() {
reinterpret_cast<char **>(app.args->argv),
reinterpret_cast<char **>(env_ptr));
- // TODO: TLS cleanup should be done after all other atexit callbacks
- // are run. So, register a cleanup callback for it with atexit before
- // everything else.
- LIBC_NAMESPACE::cleanup_tls(tls.addr, tls.size);
LIBC_NAMESPACE::exit(retval);
}
More information about the libc-commits
mailing list