Add support for LLVM's Control Flow Integrity

Submitted by Fangrui Song on Dec. 28, 2020, 12:56 a.m.

Details

Message ID 20201228005632.rezxjuehsgmr5ira@gmail.com
State New
Series "Add support for LLVM's Control Flow Integrity"
Headers show

Commit Message

Fangrui Song Dec. 28, 2020, 12:56 a.m.
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);
                         ^

Patch hide | download patch | download mbox

From 6b68dbf2e79df25f286bcd11805ae52093ba2b15 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i@maskray.me>
Date: Sun, 27 Dec 2020 16:54:29 -0800
Subject: [PATCH] WIP. find icall

---
 .../bugprone/BugproneTidyModule.cpp           |  2 +
 .../clang-tidy/bugprone/CMakeLists.txt        |  1 +
 .../clang-tidy/bugprone/FindICallCheck.cpp    | 43 +++++++++++++++++++
 .../clang-tidy/bugprone/FindICallCheck.h      | 30 +++++++++++++
 4 files changed, 76 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/FindICallCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/FindICallCheck.h

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 7f4d40f9701..c9a8f250766 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -19,6 +19,7 @@ 
 #include "DanglingHandleCheck.h"
 #include "DynamicStaticInitializersCheck.h"
 #include "ExceptionEscapeCheck.h"
+#include "FindICallCheck.h"
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
 #include "ForwardingReferenceOverloadCheck.h"
@@ -161,6 +162,7 @@  public:
         "bugprone-suspicious-semicolon");
     CheckFactories.registerCheck<SuspiciousStringCompareCheck>(
         "bugprone-suspicious-string-compare");
+    CheckFactories.registerCheck<FindICallCheck>("bugprone-find-icall");
     CheckFactories.registerCheck<SwappedArgumentsCheck>(
         "bugprone-swapped-arguments");
     CheckFactories.registerCheck<TerminatingContinueCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index b3684a5c101..2e52673069f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -49,6 +49,7 @@  add_clang_library(clangTidyBugproneModule
   SuspiciousMissingCommaCheck.cpp
   SuspiciousSemicolonCheck.cpp
   SuspiciousStringCompareCheck.cpp
+  FindICallCheck.cpp
   SwappedArgumentsCheck.cpp
   TerminatingContinueCheck.cpp
   ThrowKeywordMissingCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/FindICallCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/FindICallCheck.cpp
new file mode 100644
index 00000000000..81a3e87e0f6
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/FindICallCheck.cpp
@@ -0,0 +1,43 @@ 
+//===--- FindICallCheck.cpp - clang-tidy ---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "FindICallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void FindICallCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(callExpr().bind("call"), this);
+}
+
+static const Expr *ignoreNoOpCasts(const Expr *E) {
+  if (auto *Cast = dyn_cast<CastExpr>(E))
+    if (Cast->getCastKind() != CK_LValueToRValue &&
+        Cast->getCastKind() != CK_FunctionToPointerDecay)
+      return ignoreNoOpCasts(Cast->getSubExpr());
+  return E;
+}
+
+void FindICallCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");
+  if (auto *C = ignoreNoOpCasts(Call->getCallee()))
+    if (auto *Cast = dyn_cast<CastExpr>(C))
+      if (Cast->getCastKind() == CK_LValueToRValue)
+        diag(Call->getBeginLoc(), "%0") << C->getType();
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/bugprone/FindICallCheck.h b/clang-tools-extra/clang-tidy/bugprone/FindICallCheck.h
new file mode 100644
index 00000000000..8becedee82f
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/FindICallCheck.h
@@ -0,0 +1,30 @@ 
+//===--- SwappedArgumentsCheck.h - clang-tidy -------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_FINDICALLCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_FINDICALLCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+class FindICallCheck : public ClangTidyCheck {
+public:
+  FindICallCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_FINDICALLCHECK_H
-- 
2.29.2


Comments

Charlotte Delenk Dec. 28, 2020, 9:20 a.m.
Hi,

On 12/28/20 1:56 AM, Fangrui Song wrote:
>> tl;dr This is insufficient.
Yes, I'm aware that this only covers the few things that I have tested.
>> For libc, we can ignore all the other CFI checks.
>>
>> ldso/ has several indirect function calls which are incompatible with 
>> -fsanitize=cfi-icall
Use of CFI together with dynamic loading wasn't one of my priorities but 
it certainly might be for others
>>
>> 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.
I'm going to disable cfi for that one as well.
>>
>> (Another problem: Clang tracks undefined symbols in module-level 
>> inline asm but does not know undefined symbols in function-scope 
>> inline asm.
I think you can also add an __attribute__((used)) to the declaration; I 
have successfully used that hack in other low-level software that calls 
otherwise unused functions from inline asm.
> 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"))).
Thank you for writing this! I'll look through the list and see if I spot 
other issues.