Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 28 Dec 2020 14:17:22 +0100
From: Charlotte Delenk <darkkirb@...kkirb.de>
To: musl@...ts.openwall.com
Subject: [PATCH] Add support for LLVM's Control Flow Integrity (V2)

Control Flow Integrity is a sanitization option found in clang which
attempts to prevent exploits and bugs that divert the control flow to an
unintended path. For more information about it, refer to clang's
documentation[1].

While there are many different schemes currently implemented, the only
one that is enabled for C code is the cfi-icall scheme, which attempts
to prevent indirect calls to function with the wrong type. In most of
musl's code this works without issues, however there are a few cases
where it does not work, or at least won't work without breaking a
considerable amount of applications.

This patch force-disables CFI for the following functions:

libc_start_main_stage2: Requries main() to take all 3 arguments and
return an int. Standard C defines a 0 and 2 argument variant which are
used by most applications.

libc_start_init: This function calls static initializers with the type
void(*)(void). The actual function signature for static initializers are
undefined and it is not the same type as the one used by clang for c++
static initializers.

libc_exit_fini: Same as libc_start_init, just with static destructors.

_dlstart_c: The compiler can't determine the type of the __dls2
function. I am not sure why exactly, because it can do that with the
ctors, dtors and main. Might be because of the inline assembly.

__dls2: The compiler can't determine the type of the __dls2b function
that is called indirectly

Additionally the patch changes the following functions:

__dls2: LTO (a requirement for CFI) will errorneously determine that
this symbol is unused. We have to tell the compiler that the symbol is
in fact used.

__stack_chk_guard: Same as __dls2
__dls2b: Same as __dls2
__dls3: Same as __dls2

Despite making changes to the dynamic loader portion, this patch
currently does not allow to use CFI in dynamically linked applications,
as it would either require a runtime library or break every function
that uses a non-libc function pointer.

I have checked all of the places with indirect function calls using the
output of Fangrui's clang tidy patch and only found the aforementioned
functions.

How to test: In addition to the -fsanitize=cfi flag, you also need to pass
-flto=thin and -fvisibility=default (or hidden in a static build). The
application has to be compiled and linked with the same flags as well.
You might need to set the environment variables AR=llvm-ar and RANLIB=
llvm-ranlib for musl or the software you are compiling.

[1]: https://clang.llvm.org/docs/ControlFlowIntegrity.html
Special thanks to Fangrui Song <i@...kray.me>

---
  ldso/dlstart.c              |  5 +++++
  ldso/dynlink.c              | 15 ++++++++++++---
  src/env/__libc_start_main.c |  9 +++++++++
  src/env/__stack_chk_fail.c  |  3 +++
  src/exit/exit.c             |  4 ++++
  5 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/ldso/dlstart.c b/ldso/dlstart.c
index 20d50f2c..f32177f4 100644
--- a/ldso/dlstart.c
+++ b/ldso/dlstart.c
@@ -18,6 +18,11 @@
      *(fp) = static_func_ptr; } while(0)
  #endif

+
+#ifdef __clang__
+/* function is incompatible with CFI */
+__attribute__((no_sanitize("cfi")))
+#endif
  hidden void _dlstart_c(size_t *sp, size_t *dynv)
  {
      size_t i, aux[AUX_CNT], dyn[DYN_CNT];
diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index 6b868c84..f9c77c66 100644
--- a/ldso/dynlink.c
+++ b/ldso/dynlink.c
@@ -1637,7 +1637,12 @@ static void install_new_tls(void)
   * symbols. Its job is to perform symbolic relocations on the dynamic
   * linker itself, but some of the relocations performed may need to be
   * replaced later due to copy relocations in the main program. */
-
+#ifdef __clang__
+/* workaround for compiling ldso with LTO with clang. This forces the
+ * linker to emit this function, even though it is "seemingly" unused
+ */
+__attribute__((used))
+#endif
  hidden void __dls2(unsigned char *base, size_t *sp)
  {
      size_t *auxv;
@@ -1702,7 +1707,9 @@ hidden void __dls2(unsigned char *base, size_t *sp)
   * This is done as a separate stage, with symbolic lookup as a barrier,
   * so that loads of the thread pointer and &errno can be pure/const and
   * thereby hoistable. */
-
+#ifdef __clang__
+__attribute__((used))
+#endif
  void __dls2b(size_t *sp, size_t *auxv)
  {
      /* Setup early thread pointer in builtin_tls for ldso/libc itself to
@@ -1725,7 +1732,9 @@ void __dls2b(size_t *sp, size_t *auxv)
   * fully functional. Its job is to load (if not already loaded) and
   * process dependencies and relocations for the main application and
   * transfer control to its entry point. */
-
+#ifdef __clang__
+__attribute__((used))
+#endif
  void __dls3(size_t *sp, size_t *auxv)
  {
      static struct dso app, vdso;
diff --git a/src/env/__libc_start_main.c b/src/env/__libc_start_main.c
index 8fbe5262..9eaeda17 100644
--- a/src/env/__libc_start_main.c
+++ b/src/env/__libc_start_main.c
@@ -56,6 +56,12 @@ void __init_libc(char **envp, char *pn)
      libc.secure = 1;
  }

+#ifdef __clang__
+/* Disable CFI sanitization as the constructors don't necessarily
+ * have the correct void(void) type (didn't work with c++ static
+ * constructors */
+__attribute__((no_sanitize("cfi")))
+#endif
  static void libc_start_init(void)
  {
      _init();
@@ -85,6 +91,9 @@ int __libc_start_main(int (*main)(int,char **,char 
**), int argc, char **argv)
      return stage2(main, argc, argv);
  }

+#ifdef __clang__
+__attribute__((no_sanitize("cfi")))
+#endif
  static int libc_start_main_stage2(int (*main)(int,char **,char **), 
int argc, char **argv)
  {
      char **envp = argv+argc+1;
diff --git a/src/env/__stack_chk_fail.c b/src/env/__stack_chk_fail.c
index bf5a280a..adbac9b7 100644
--- a/src/env/__stack_chk_fail.c
+++ b/src/env/__stack_chk_fail.c
@@ -2,6 +2,9 @@
  #include <stdint.h>
  #include "pthread_impl.h"

+#ifdef __clang__
+__attribute__((used))
+#endif
  uintptr_t __stack_chk_guard;

  void __init_ssp(void *entropy)
diff --git a/src/exit/exit.c b/src/exit/exit.c
index a6869b37..3be952e6 100644
--- a/src/exit/exit.c
+++ b/src/exit/exit.c
@@ -14,6 +14,10 @@ weak_alias(dummy, _fini);

  extern weak hidden void (*const __fini_array_start)(void), (*const 
__fini_array_end)(void);

+#ifdef __clang__
+/* static destructor might not be of the correct type, just like the 
initializer */
+__attribute__((no_sanitize("cfi")))
+#endif
  static void libc_exit_fini(void)
  {
      uintptr_t a = (uintptr_t)&__fini_array_end;
-- 
2.29.2

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.