From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 32474 invoked from network); 28 Dec 2020 13:17:43 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 28 Dec 2020 13:17:43 -0000 Received: (qmail 9491 invoked by uid 550); 28 Dec 2020 13:17:41 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 9470 invoked from network); 28 Dec 2020 13:17:40 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=darkkirb.de; s=mail; t=1609161447; bh=he7b0AhesAP/RdDaqjPoIk7Rr7D51ul1iy0qDsAx5ac=; h=Subject:To:References:From:Date:In-Reply-To; b=glQHVyxz451sVDk/f26YFGEGALD12Fcn3tOZyhBa7dC9P+uhW4OkcdwOmilK9MsT0 tWyks67JZElJ2+74XCoI07SUQT45JYEiA/JI7wDVyBWzPfCuu0gqeCRAOnGHh2QCQX 7GfviDfxWt05d7lY+wRRtQoX0R7GaDvAnuKhIH5o= To: musl@lists.openwall.com References: <6106be97-2c82-75c0-ad88-2e49b17c68ee@darkkirb.de> <20201227230554.ddflrnbmyvjccj4n@gmail.com> <20201228005632.rezxjuehsgmr5ira@gmail.com> <0326d35a-4c1d-3d25-1917-166e5b580062@darkkirb.de> From: Charlotte Delenk Message-ID: <865734a3-e4c6-eb06-5855-db74c00fb071@darkkirb.de> Date: Mon, 28 Dec 2020 14:17:22 +0100 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:78.0) Gecko/20100101 Thunderbird/78.4.3 MIME-Version: 1.0 In-Reply-To: <0326d35a-4c1d-3d25-1917-166e5b580062@darkkirb.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: [musl] [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 ---  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  #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