[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 27 Dec 2020 16:56:32 -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, Fangrui Song wrote:
>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'
Minor correction: AR=llvm-ar RANLIB=true is needed, otherwise the
archive index of libc.a will not include LLVM IR symbols - there will be
spurious undefined reference errors.
I wrote a clang-tidy check (attached) to find all indirect function
calls. Here is the list. cfi-icall can protect them. Very few like
__libc_start_main.c:64 may need __attribute__((__no_sanitize__("cfi"))).
../ldso/dlstart.c:147:2: warning: 'stage2_func' (aka 'void (*)(unsigned char *, unsigned long *)') [bugprone-find-icall]
dls2((void *)base, sp);
^
../ldso/dynlink.c:2335:9: warning: 'int (*)(struct dl_phdr_info *, size_t, void *)' (aka 'int (*)(struct dl_phdr_info *, unsigned long, void *)') [bugprone-find-icall]
ret = (callback)(&info, sizeof (info), data);
^
../src/aio/aio.c:201:3: warning: 'void (*)(union sigval)' [bugprone-find-icall]
sev.sigev_notify_function(sev.sigev_value);
^
../src/aio/lio_listio.c:63:3: warning: 'void (*)(union sigval)' [bugprone-find-icall]
sev->sigev_notify_function(sev->sigev_value);
^
../src/dirent/scandir.c:20:15: warning: 'int (*)(const struct dirent *)' [bugprone-find-icall]
if (sel && !sel(de)) continue;
^
../src/env/__libc_start_main.c:64:3: warning: 'void (*)(void)' [bugprone-find-icall]
(*(void (**)(void))a)();
^
../src/env/__libc_start_main.c:85:9: warning: 'lsm2_fn *' (aka 'int (*)(int (*)(int, char **, char **), int, char **)') [bugprone-find-icall]
return stage2(main, argc, argv);
^
../src/env/__libc_start_main.c:94:7: warning: 'int (*)(int, char **, char **)' [bugprone-find-icall]
exit(main(argc, argv, envp));
^
../src/exit/at_quick_exit.c:20:3: warning: 'void (*)(void)' [bugprone-find-icall]
func();
^
../src/exit/atexit.c:34:3: warning: 'void (*)(void *)' [bugprone-find-icall]
func(arg);
^
../src/exit/exit.c:21:3: warning: 'void (*)()' [bugprone-find-icall]
(*(void (**)())(a-sizeof(void(*)())))();
^
../src/ldso/dl_iterate_phdr.c:43:9: warning: 'int (*)(struct dl_phdr_info *, size_t, void *)' (aka 'int (*)(struct dl_phdr_info *, unsigned long, void *)') [bugprone-find-icall]
return (callback)(&info, sizeof (info), data);
^
../src/misc/nftw.c:75:33: warning: 'int (*)(const char *, const struct stat *, int, struct FTW *)' [bugprone-find-icall]
if (!(flags & FTW_DEPTH) && (r=fn(path, &st, type, &lev)))
^
../src/misc/nftw.c:115:32: warning: 'int (*)(const char *, const struct stat *, int, struct FTW *)' [bugprone-find-icall]
if ((flags & FTW_DEPTH) && (r=fn(path, &st, type, &lev)))
^
../src/mq/mq_notify.c:28:3: warning: 'void (*)(union sigval)' [bugprone-find-icall]
func(val);
^
../src/network/dns_parse.c:29:7: warning: 'int (*)(void *, int, const void *, int, const void *)' [bugprone-find-icall]
if (callback(ctx, p[1], p+10, len, r) < 0) return -1;
^
../src/network/netlink.c:36:10: warning: 'int (*)(void *, struct nlmsghdr *)' [bugprone-find-icall]
ret = cb(ctx, h);
^
../src/process/posix_spawn.c:153:2: warning: 'int (*)(const char *, char *const *, char *const *)' [bugprone-find-icall]
exec(args->path, args->argv, args->envp);
^
../src/regex/glob.c:108:26: warning: 'int (*)(const char *, int)' [bugprone-find-icall]
if (errno!=ENOENT && (errfunc(buf, errno) || (flags & GLOB_ERR)))
^
../src/regex/glob.c:129:7: warning: 'int (*)(const char *, int)' [bugprone-find-icall]
if (errfunc(buf, errno) || (flags & GLOB_ERR))
^
../src/regex/glob.c:169:18: warning: 'int (*)(const char *, int)' [bugprone-find-icall]
if (readerr && (errfunc(buf, errno) || (flags & GLOB_ERR)))
^
../src/sched/sched_getcpu.c:18:13: warning: 'getcpu_f' (aka 'long (*)(unsigned int *, unsigned int *, void *)') [bugprone-find-icall]
return f ? f(cpu, node, unused) : -ENOSYS;
^
../src/sched/sched_getcpu.c:33:7: warning: 'getcpu_f' (aka 'long (*)(unsigned int *, unsigned int *, void *)') [bugprone-find-icall]
r = f(&cpu, 0, 0);
^
../src/search/lsearch.c:12:7: warning: 'int (*)(const void *, const void *)' [bugprone-find-icall]
if (compar(key, p[i]) == 0)
^
../src/search/lsearch.c:26:7: warning: 'int (*)(const void *, const void *)' [bugprone-find-icall]
if (compar(key, p[i]) == 0)
^
../src/search/tdelete.c:23:11: warning: 'int (*)(const void *, const void *)' [bugprone-find-icall]
int c = cmp(key, n->key);
^
../src/search/tdestroy.c:14:15: warning: 'void (*)(void *)' [bugprone-find-icall]
if (freekey) freekey((void *)r->key);
^
../src/search/tfind.c:14:11: warning: 'int (*)(const void *, const void *)' [bugprone-find-icall]
int c = cmp(key, n->key);
^
../src/search/tsearch.c:76:11: warning: 'int (*)(const void *, const void *)' [bugprone-find-icall]
int c = cmp(key, n->key);
^
../src/search/twalk.c:9:3: warning: 'void (*)(const void *, VISIT, int)' [bugprone-find-icall]
action(r, leaf, d);
^
../src/search/twalk.c:11:3: warning: 'void (*)(const void *, VISIT, int)' [bugprone-find-icall]
action(r, preorder, d);
^
../src/search/twalk.c:13:3: warning: 'void (*)(const void *, VISIT, int)' [bugprone-find-icall]
action(r, postorder, d);
^
../src/search/twalk.c:15:3: warning: 'void (*)(const void *, VISIT, int)' [bugprone-find-icall]
action(r, endorder, d);
^
../src/stdio/__fclose_ca.c:5:9: warning: 'int (*)(FILE *)' (aka 'int (*)(struct _IO_FILE *)') [bugprone-find-icall]
return f->close(f);
^
../src/stdio/__overflow.c:8:6: warning: 'size_t (*)(FILE *, const unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, const unsigned char *, unsigned long)') [bugprone-find-icall]
if (f->write(f, &c, 1)!=1) return EOF;
^
../src/stdio/__stdio_exit.c:12:27: warning: 'size_t (*)(FILE *, const unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, const unsigned char *, unsigned long)') [bugprone-find-icall]
if (f->wpos != f->wbase) f->write(f, 0, 0);
^
../src/stdio/__stdio_exit.c:13:26: warning: 'off_t (*)(FILE *, off_t, int)' (aka 'long (*)(struct _IO_FILE *, long, int)') [bugprone-find-icall]
if (f->rpos != f->rend) f->seek(f, f->rpos-f->rend, SEEK_CUR);
^
../src/stdio/__toread.c:6:27: warning: 'size_t (*)(FILE *, const unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, const unsigned char *, unsigned long)') [bugprone-find-icall]
if (f->wpos != f->wbase) f->write(f, 0, 0);
^
../src/stdio/__uflow.c:9:22: warning: 'size_t (*)(FILE *, unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, unsigned char *, unsigned long)') [bugprone-find-icall]
if (!__toread(f) && f->read(f, &c, 1)==1) return c;
^
../src/stdio/fclose.c:13:7: warning: 'int (*)(FILE *)' (aka 'int (*)(struct _IO_FILE *)') [bugprone-find-icall]
r |= f->close(f);
^
../src/stdio/fflush.c:29:3: warning: 'size_t (*)(FILE *, const unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, const unsigned char *, unsigned long)') [bugprone-find-icall]
f->write(f, 0, 0);
^
../src/stdio/fflush.c:37:26: warning: 'off_t (*)(FILE *, off_t, int)' (aka 'long (*)(struct _IO_FILE *, long, int)') [bugprone-find-icall]
if (f->rpos != f->rend) f->seek(f, f->rpos-f->rend, SEEK_CUR);
^
../src/stdio/fopencookie.c:30:9: warning: 'cookie_read_function_t *' (aka 'long (*)(void *, char *, unsigned long)') [bugprone-find-icall]
ret = fc->iofuncs.read(fc->cookie, (char *) buf, len2);
^
../src/stdio/fopencookie.c:40:8: warning: 'cookie_read_function_t *' (aka 'long (*)(void *, char *, unsigned long)') [bugprone-find-icall]
ret = fc->iofuncs.read(fc->cookie, (char *) f->rpos, f->buf_size);
^
../src/stdio/fopencookie.c:64:8: warning: 'cookie_write_function_t *' (aka 'long (*)(void *, const char *, unsigned long)') [bugprone-find-icall]
ret = fc->iofuncs.write(fc->cookie, (const char *) buf, len);
^
../src/stdio/fopencookie.c:85:8: warning: 'cookie_seek_function_t *' (aka 'int (*)(void *, long *, int)') [bugprone-find-icall]
res = fc->iofuncs.seek(fc->cookie, &off, whence);
^
../src/stdio/fopencookie.c:94:32: warning: 'cookie_close_function_t *' (aka 'int (*)(void *)') [bugprone-find-icall]
if (fc->iofuncs.close) return fc->iofuncs.close(fc->cookie);
^
../src/stdio/fread.c:27:25: warning: 'size_t (*)(FILE *, unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, unsigned char *, unsigned long)') [bugprone-find-icall]
k = __toread(f) ? 0 : f->read(f, dest, l);
^
../src/stdio/fseek.c:10:3: warning: 'size_t (*)(FILE *, const unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, const unsigned char *, unsigned long)') [bugprone-find-icall]
f->write(f, 0, 0);
^
../src/stdio/fseek.c:18:6: warning: 'off_t (*)(FILE *, off_t, int)' (aka 'long (*)(struct _IO_FILE *, long, int)') [bugprone-find-icall]
if (f->seek(f, off, whence) < 0) return -1;
^
../src/stdio/ftell.c:7:14: warning: 'off_t (*)(FILE *, off_t, int)' (aka 'long (*)(struct _IO_FILE *, long, int)') [bugprone-find-icall]
off_t pos = f->seek(f, 0,
^
../src/stdio/fwrite.c:10:36: warning: 'size_t (*)(FILE *, const unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, const unsigned char *, unsigned long)') [bugprone-find-icall]
if (l > f->wend - f->wpos) return f->write(f, s, l);
^
../src/stdio/fwrite.c:16:15: warning: 'size_t (*)(FILE *, const unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, const unsigned char *, unsigned long)') [bugprone-find-icall]
size_t n = f->write(f, s, i);
^
../src/stdio/vfprintf.c:685:3: warning: 'size_t (*)(FILE *, const unsigned char *, size_t)' (aka 'unsigned long (*)(struct _IO_FILE *, const unsigned char *, unsigned long)') [bugprone-find-icall]
f->write(f, 0, 0);
^
../src/stdlib/bsearch.c:9:10: warning: 'int (*)(const void *, const void *)' [bugprone-find-icall]
sign = cmp(key, try);
^
../src/thread/pthread_atfork.c:21:20: warning: 'void (*)(void)' [bugprone-find-icall]
if (p->prepare) p->prepare();
^
../src/thread/pthread_atfork.c:26:27: warning: 'void (*)(void)' [bugprone-find-icall]
if (!who && p->parent) p->parent();
^
../src/thread/pthread_atfork.c:27:30: warning: 'void (*)(void)' [bugprone-find-icall]
else if (who && p->child) p->child();
^
../src/thread/pthread_cleanup_push.c:19:11: warning: 'void (*)(void *)' [bugprone-find-icall]
if (run) cb->__f(cb->__x);
^
../src/thread/pthread_create.c:67:3: warning: 'void (*)(void *)' [bugprone-find-icall]
f(x);
^
../src/thread/pthread_create.c:203:17: warning: 'void *(*)(void *)' [bugprone-find-icall]
__pthread_exit(args->start_func(args->start_arg));
^
../src/thread/pthread_create.c:211:36: warning: 'int (*)(void *)' [bugprone-find-icall]
__pthread_exit((void *)(uintptr_t)start(args->start_arg));
^
../src/thread/pthread_key_create.c:82:5: warning: 'void (*)(void *)' [bugprone-find-icall]
dtor(val);
^
../src/thread/pthread_once.c:22:3: warning: 'void (*)(void)' [bugprone-find-icall]
init();
^
../src/thread/synccall.c:31:2: warning: 'void (*)(void *)' [bugprone-find-icall]
callback(context);
^
../src/thread/synccall.c:105:2: warning: 'void (*)(void *)' [bugprone-find-icall]
func(ctx);
^
../src/time/clock_gettime.c:49:13: warning: 'int (*)(clockid_t, struct timespec *)' (aka 'int (*)(int, struct timespec *)') [bugprone-find-icall]
return f ? f(clk, ts) : -ENOSYS;
^
../src/time/clock_gettime.c:64:7: warning: 'int (*)(clockid_t, struct timespec *)' (aka 'int (*)(int, struct timespec *)') [bugprone-find-icall]
r = f(clk, ts);
^
../src/time/timer_create.c:51:4: warning: 'void (*)(union sigval)' [bugprone-find-icall]
notify(val);
^
View attachment "clang-tidy-bugprone-find-icall.patch" of type "text/x-diff" (5260 bytes)
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.
