Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201227230554.ddflrnbmyvjccj4n@gmail.com>
Date: Sun, 27 Dec 2020 15:05:54 -0800
From: Fangrui Song <i@...kray.me>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Add support for LLVM's Control Flow Integrity

On 2020-12-27, Charlotte Delenk wrote:
>Hi,
>
>I have attempted to use musl HEAD together with clang's -fsanitize=cfi,
>but currently it requires the main function to take all 3 arguments and
>return an int.
>
>After this patch is applied, clang will no longer try to add CFI
>sanitization to the libc_start_main_stage2 function, allowing programs
>to get to main().
>
>I have tested CFI sanitization for both regular indirect functions
>(qsort()) and thread creation and validly typed function pointers cause
>no runtime aborts with CFI enabled for the whole program.
>
>---
>
> src/env/__libc_start_main.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/src/env/__libc_start_main.c b/src/env/__libc_start_main.c
>index 8fbe5262..af61fb7c 100644
>--- a/src/env/__libc_start_main.c
>+++ b/src/env/__libc_start_main.c
>@@ -85,6 +85,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;
>-- 
>2.29.2
>

tl;dr This is insufficient.


Some background about CFI for other readers

Clang -fsanitize=cfi includes several checks [0]. Most are C++ specific and I
think only -fsanitize=cfi-icall (forward-edge CFI for indirect function calls
[1]) is relevant to C. CFI requires -flto={thin,full}.

[0]: https://clang.llvm.org/docs/ControlFlowIntegrity.html
[1]: https://clang.llvm.org/docs/ControlFlowIntegrityDesign.html#forward-edge-cfi-for-indirect-function-calls

For libc, we can ignore all the other CFI checks.

ldso/ has several indirect function calls which are incompatible with -fsanitize=cfi-icall

ldso/dlstart.c
In _dlstart_c, dls2((void *)base, sp); The address is obtained via GETFUNCSYM (inline asm) - Clang does not know the original function type and thus the llvm.type.test check will fail. The codegen places functions of the same signature consecutively and checks whether the indirect function is a known function in this list - an unknown function will fail.

(Another problem: Clang tracks undefined symbols in module-level inline asm but does not know undefined symbols in function-scope inline asm.
If you build this file with LTO, the LTO backend will internalize __dls2 and cause a subsequent "undefined reference" error unless you specify -u __dls2
to mark __dls2 as "visibible to a regular object file" (a workaround))

ldso/dynlink.c
In __dls2, ((stage3_func)laddr(&ldso, dls2b_def.sym->st_value))(sp, auxv);
In do_init_fini, fpaddr(p, dyn[DT_INIT])();

src/env/__libc_start_main.c
In libc_start_main_stage2,
In libc_start_init, (

Adding this line to Makefile is a simple way to disable LTO and CFI

   obj/ldso/dlstart.lo obj/ldso/dynlink.lo: CFLAGS+=-fno-lto -fno-sanitize=cfi

(LTO has to be disabled, otherwise LTO will error "inconsistent LTO Unit splitting (recompile with -fsplit-lto-unit)")

With the above, I can build musl with CFI:

   mkdir -p out/lto
   cd out/lto
   ../../configure CC=clang CFLAGS='-g -flto=thin -fsanitize=cfi-icall' LDFLAGS='-fuse-ld=lld'

---

src/env/__libc_start_main.c:libc_start_main_stage2 has a problem.
Other functions with external hooks can also have problems as well, e.g. src/exit/atexit.c:call ((void (*)(void))(uintptr_t)p)(); may fail.

These functions may reside in either libc.so or libc.a. Let's discuss both.

## libc.so

-fsanitize=cfi-icall alone fundamentally does not work due to hooks.
So the experimental -fsanitize=cfi-icall -fsanitize-cfi-cross-dso is needed:
instead of trapping immediately, Clang emits a call to __cfi_slowpath to check
whether the function is defined externally.

However, -shared -fsanitize-cfi-cross-dso does not currently have a good link story.
clang -shared does not link in libclang_rt.cfi-*.a, so __cfi_slowpath call will be external.
One can still build musl with

   ../../configure CC=clang CFLAGS='-g -flto=thin -fsanitize=cfi-icall -fsanitize-cfi-cross-dso' LDFLAGS='-fuse-ld=lld -z undefs'

(libclang_rt.cfi-* call musl-unsupported dlvsym. This can be worked around with

--- i/src/ldso/x86_64/dlsym.s
+++ w/src/ldso/x86_64/dlsym.s
@@ -5,3 +5,10 @@
  dlsym:
         mov (%rsp),%rdx
         jmp __dlsym
+
+.global dlvsym
+.hidden __dlvsym
+.type dlvsym,@function
+dlvsym:
+       mov (%rsp),%rdx
+       jmp __dlsym
)

but such a built libc.so requires the main executable to link in libclang_rt.cfi-*.a, i.e. the main executable has to
be built with -fsanitize=cfi-icall (or another CFI check) -fsanitize-cfi-cross-dso.

(There is currently an unrelated llvm.ubsantrap bug (the intrinsic somehow is not lowered)
% ~/musl/out/cross-dso/obj/musl-clang a.c -fsanitize=cfi-icall -flto -fvisibility=default -fsanitize-cfi-cross-dso
ld: error: undefined symbol: llvm.ubsantrap
>>> referenced by crt1.c:0 (../../crt/crt1.c:0)
>>>               lto.tmp:(__cfi_check_fail)
clang-12: error: linker command failed with exit code 1 (use -v to see invocation)
)

% ~/musl/out/cross-dso/obj/musl-clang a.c -fsanitize=cfi-icall -flto -fvisibility=default -fsanitize-cfi-cross-dso -Wl,--defsym=llvm.ubsantrap=0
% ./a.out
Error relocating libc.so: __cfi_slowpath: symbol not found

The diagnostic is from __dls2.  The issue is that musl expects its relocations can be resolved by itself.

## libc.a

For a -static link, there are additional failures in:

src/env/__libc_start_main.c
In libc_start_init, (*(void (**)(void))a)();
a is linker synthesized __init_array_start

src/exit/exit.c
In libc_exit_fini, (*(void (**)())(a-sizeof(void(*)())))();

After annotating the above:

% ~/musl/out/cross-dso/obj/musl-clang a.c -fsanitize=cfi-icall -flto -fvisibility=default -fsanitize-cfi-cross-dso -static -Wl,--defsym=llvm.ubsantrap=0
% ./a.out   # succeeded

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.