[RFC PATCH v2 06/21] cfi: Switch to -fsanitize=kcfi
Sami Tolvanen
samitolvanen at google.com
Fri May 13 13:21:44 PDT 2022
Switch from Clang's original forward-edge control-flow integrity
implementation to -fsanitize=kcfi, which is better suited for the
kernel, as it doesn't require LTO, doesn't use a jump table that
requires altering function references, and won't break cross-module
function address equality.
Signed-off-by: Sami Tolvanen <samitolvanen at google.com>
---
Makefile | 13 +--
arch/Kconfig | 11 ++-
include/asm-generic/vmlinux.lds.h | 37 ++++-----
include/linux/cfi.h | 35 +++++++--
include/linux/compiler-clang.h | 6 +-
include/linux/module.h | 6 +-
kernel/cfi.c | 126 ++++++++++++++----------------
kernel/module.c | 34 +-------
scripts/module.lds.S | 23 +-----
9 files changed, 128 insertions(+), 163 deletions(-)
diff --git a/Makefile b/Makefile
index 2284d1ca2503..8439551954f1 100644
--- a/Makefile
+++ b/Makefile
@@ -915,18 +915,7 @@ export CC_FLAGS_LTO
endif
ifdef CONFIG_CFI_CLANG
-CC_FLAGS_CFI := -fsanitize=cfi \
- -fsanitize-cfi-cross-dso \
- -fno-sanitize-cfi-canonical-jump-tables \
- -fno-sanitize-trap=cfi \
- -fno-sanitize-blacklist
-
-ifdef CONFIG_CFI_PERMISSIVE
-CC_FLAGS_CFI += -fsanitize-recover=cfi
-endif
-
-# If LTO flags are filtered out, we must also filter out CFI.
-CC_FLAGS_LTO += $(CC_FLAGS_CFI)
+CC_FLAGS_CFI := -fsanitize=kcfi
KBUILD_CFLAGS += $(CC_FLAGS_CFI)
export CC_FLAGS_CFI
endif
diff --git a/arch/Kconfig b/arch/Kconfig
index 625db6376726..f179170cb422 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -720,14 +720,13 @@ config ARCH_SUPPORTS_CFI_CLANG
An architecture should select this option if it can support Clang's
Control-Flow Integrity (CFI) checking.
+config ARCH_USES_CFI_TRAPS
+ bool
+
config CFI_CLANG
bool "Use Clang's Control Flow Integrity (CFI)"
- depends on LTO_CLANG && ARCH_SUPPORTS_CFI_CLANG
- # Clang >= 12:
- # - https://bugs.llvm.org/show_bug.cgi?id=46258
- # - https://bugs.llvm.org/show_bug.cgi?id=47479
- depends on CLANG_VERSION >= 120000
- select KALLSYMS
+ depends on ARCH_SUPPORTS_CFI_CLANG
+ depends on $(cc-option,-fsanitize=kcfi)
help
This option enables Clang’s forward-edge Control Flow Integrity
(CFI) checking, where the compiler injects a runtime check to each
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 69138e9db787..fcb3c7146a43 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -421,6 +421,22 @@
__end_ro_after_init = .;
#endif
+/*
+ * .kcfi_traps contains a list KCFI trap locations.
+ */
+#ifndef KCFI_TRAPS
+#ifdef CONFIG_ARCH_USES_CFI_TRAPS
+#define KCFI_TRAPS \
+ __kcfi_traps : AT(ADDR(__kcfi_traps) - LOAD_OFFSET) { \
+ __start___kcfi_traps = .; \
+ KEEP(*(.kcfi_traps)) \
+ __stop___kcfi_traps = .; \
+ }
+#else
+#define KCFI_TRAPS
+#endif
+#endif
+
/*
* Read only Data
*/
@@ -529,6 +545,8 @@
__stop___modver = .; \
} \
\
+ KCFI_TRAPS \
+ \
RO_EXCEPTION_TABLE \
NOTES \
BTF \
@@ -537,21 +555,6 @@
__end_rodata = .;
-/*
- * .text..L.cfi.jumptable.* contain Control-Flow Integrity (CFI)
- * jump table entries.
- */
-#ifdef CONFIG_CFI_CLANG
-#define TEXT_CFI_JT \
- . = ALIGN(PMD_SIZE); \
- __cfi_jt_start = .; \
- *(.text..L.cfi.jumptable .text..L.cfi.jumptable.*) \
- . = ALIGN(PMD_SIZE); \
- __cfi_jt_end = .;
-#else
-#define TEXT_CFI_JT
-#endif
-
/*
* Non-instrumentable text section
*/
@@ -579,7 +582,6 @@
*(.text..refcount) \
*(.ref.text) \
*(.text.asan.* .text.tsan.*) \
- TEXT_CFI_JT \
MEM_KEEP(init.text*) \
MEM_KEEP(exit.text*) \
@@ -1008,8 +1010,7 @@
* keep any .init_array.* sections.
* https://bugs.llvm.org/show_bug.cgi?id=46478
*/
-#if defined(CONFIG_GCOV_KERNEL) || defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KCSAN) || \
- defined(CONFIG_CFI_CLANG)
+#if defined(CONFIG_GCOV_KERNEL) || defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KCSAN)
# ifdef CONFIG_CONSTRUCTORS
# define SANITIZER_DISCARDS \
*(.eh_frame)
diff --git a/include/linux/cfi.h b/include/linux/cfi.h
index 2cdbc0fbd0ab..655b8b10ac3d 100644
--- a/include/linux/cfi.h
+++ b/include/linux/cfi.h
@@ -2,17 +2,42 @@
/*
* Clang Control Flow Integrity (CFI) support.
*
- * Copyright (C) 2021 Google LLC
+ * Copyright (C) 2022 Google LLC
*/
#ifndef _LINUX_CFI_H
#define _LINUX_CFI_H
+#include <linux/bug.h>
+#include <linux/module.h>
+
#ifdef CONFIG_CFI_CLANG
-typedef void (*cfi_check_fn)(uint64_t id, void *ptr, void *diag);
+enum bug_trap_type report_cfi_failure(struct pt_regs *regs, unsigned long addr,
+ unsigned long target, unsigned long type);
+#else
+static inline enum bug_trap_type report_cfi_failure(struct pt_regs *regs,
+ unsigned long addr,
+ unsigned long target,
+ unsigned long type)
+{
+ return BUG_TRAP_TYPE_NONE;
+}
+#endif /* CONFIG_CFI_CLANG */
-/* Compiler-generated function in each module, and the kernel */
-extern void __cfi_check(uint64_t id, void *ptr, void *diag);
+#ifdef CONFIG_ARCH_USES_CFI_TRAPS
+bool is_cfi_trap(unsigned long addr);
+#else
+static inline bool is_cfi_trap(unsigned long addr) { return false; }
+#endif /* CONFIG_ARCH_USES_CFI_TRAPS */
-#endif /* CONFIG_CFI_CLANG */
+#ifdef CONFIG_MODULES
+#ifdef CONFIG_ARCH_USES_CFI_TRAPS
+void module_cfi_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
+ struct module *mod);
+#else
+static inline void module_cfi_finalize(const Elf_Ehdr *hdr,
+ const Elf_Shdr *sechdrs,
+ struct module *mod) {}
+#endif /* CONFIG_ARCH_USES_CFI_TRAPS */
+#endif /* CONFIG_MODULES */
#endif /* _LINUX_CFI_H */
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index babb1347148c..42e55579d649 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -66,8 +66,10 @@
# define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
#endif
-#define __nocfi __attribute__((__no_sanitize__("cfi")))
-#define __cficanonical __attribute__((__cfi_canonical_jump_table__))
+#if __has_feature(kcfi)
+/* Disable CFI checking inside a function. */
+#define __nocfi __attribute__((__no_sanitize__("kcfi")))
+#endif
/*
* Turn individual warnings and errors on and off locally, depending
diff --git a/include/linux/module.h b/include/linux/module.h
index 87857275c047..3b485834be74 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -27,7 +27,6 @@
#include <linux/tracepoint-defs.h>
#include <linux/srcu.h>
#include <linux/static_call_types.h>
-#include <linux/cfi.h>
#include <linux/percpu.h>
#include <asm/module.h>
@@ -388,8 +387,9 @@ struct module {
const s32 *crcs;
unsigned int num_syms;
-#ifdef CONFIG_CFI_CLANG
- cfi_check_fn cfi_check;
+#ifdef CONFIG_ARCH_USES_CFI_TRAPS
+ unsigned long *kcfi_traps;
+ unsigned long *kcfi_traps_end;
#endif
/* Kernel parameters. */
diff --git a/kernel/cfi.c b/kernel/cfi.c
index 2cc0d01ea980..456d5eac082a 100644
--- a/kernel/cfi.c
+++ b/kernel/cfi.c
@@ -1,94 +1,86 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Clang Control Flow Integrity (CFI) error and slowpath handling.
+ * Clang Control Flow Integrity (CFI) error handling.
*
- * Copyright (C) 2021 Google LLC
+ * Copyright (C) 2022 Google LLC
*/
-#include <linux/hardirq.h>
-#include <linux/kallsyms.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/printk.h>
-#include <linux/ratelimit.h>
-#include <linux/rcupdate.h>
-#include <linux/vmalloc.h>
-#include <asm/cacheflush.h>
-#include <asm/set_memory.h>
-
-/* Compiler-defined handler names */
-#ifdef CONFIG_CFI_PERMISSIVE
-#define cfi_failure_handler __ubsan_handle_cfi_check_fail
-#else
-#define cfi_failure_handler __ubsan_handle_cfi_check_fail_abort
-#endif
-
-static inline void handle_cfi_failure(void *ptr)
-{
- if (IS_ENABLED(CONFIG_CFI_PERMISSIVE))
- WARN_RATELIMIT(1, "CFI failure (target: %pS):\n", ptr);
- else
- panic("CFI failure (target: %pS)\n", ptr);
-}
-
-#ifdef CONFIG_MODULES
+#include <linux/cfi.h>
-static inline cfi_check_fn find_module_check_fn(unsigned long ptr)
+enum bug_trap_type report_cfi_failure(struct pt_regs *regs, unsigned long addr,
+ unsigned long target, unsigned long type)
{
- cfi_check_fn fn = NULL;
- struct module *mod;
+ pr_err("CFI failure at %pS (target: %pS; expected type: 0x%08x)\n",
+ (void *)addr, (void *)target, (u32)type);
- rcu_read_lock_sched_notrace();
- mod = __module_address(ptr);
- if (mod)
- fn = mod->cfi_check;
- rcu_read_unlock_sched_notrace();
+ if (IS_ENABLED(CONFIG_CFI_PERMISSIVE)) {
+ __warn(NULL, 0, (void *)addr, 0, regs, NULL);
+ return BUG_TRAP_TYPE_WARN;
+ }
- return fn;
+ return BUG_TRAP_TYPE_BUG;
}
-static inline cfi_check_fn find_check_fn(unsigned long ptr)
+#ifdef CONFIG_ARCH_USES_CFI_TRAPS
+#ifdef CONFIG_MODULES
+/* Populates `kcfi_trap(_end)?` fields in `struct module`. */
+void module_cfi_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
+ struct module *mod)
{
- cfi_check_fn fn = NULL;
+ char *secstrings;
+ unsigned int i;
- if (is_kernel_text(ptr))
- return __cfi_check;
+ mod->kcfi_traps = NULL;
+ mod->kcfi_traps_end = NULL;
- /*
- * Indirect call checks can happen when RCU is not watching. Both
- * the shadow and __module_address use RCU, so we need to wake it
- * up if necessary.
- */
- RCU_NONIDLE({
- fn = find_module_check_fn(ptr);
- });
+ secstrings = (char *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
- return fn;
+ for (i = 1; i < hdr->e_shnum; i++) {
+ if (strcmp(secstrings + sechdrs[i].sh_name, "__kcfi_traps"))
+ continue;
+
+ mod->kcfi_traps = (unsigned long *)sechdrs[i].sh_addr;
+ mod->kcfi_traps_end = (unsigned long *)(sechdrs[i].sh_addr +
+ sechdrs[i].sh_size);
+ break;
+ }
}
-void __cfi_slowpath_diag(uint64_t id, void *ptr, void *diag)
+static bool is_module_cfi_trap(unsigned long addr)
{
- cfi_check_fn fn = find_check_fn((unsigned long)ptr);
+ bool found = false;
+ struct module *mod;
+ unsigned long *p;
- if (likely(fn))
- fn(id, ptr, diag);
- else /* Don't allow unchecked modules */
- handle_cfi_failure(ptr);
-}
-EXPORT_SYMBOL(__cfi_slowpath_diag);
+ rcu_read_lock_sched_notrace();
-#else /* !CONFIG_MODULES */
+ mod = __module_address(addr);
+ if (mod)
+ for (p = mod->kcfi_traps; !found && p < mod->kcfi_traps_end; ++p)
+ found = (*p == addr);
+
+ rcu_read_unlock_sched_notrace();
-void __cfi_slowpath_diag(uint64_t id, void *ptr, void *diag)
+ return found;
+}
+#else /* CONFIG_MODULES */
+static inline bool is_module_cfi_trap(unsigned long addr)
{
- handle_cfi_failure(ptr); /* No modules */
+ return false;
}
-EXPORT_SYMBOL(__cfi_slowpath_diag);
-
#endif /* CONFIG_MODULES */
-void cfi_failure_handler(void *data, void *ptr, void *vtable)
+extern unsigned long __start___kcfi_traps[];
+extern unsigned long __stop___kcfi_traps[];
+
+bool is_cfi_trap(unsigned long addr)
{
- handle_cfi_failure(ptr);
+ unsigned long *p;
+
+ for (p = __start___kcfi_traps; p < __stop___kcfi_traps; ++p)
+ if (*p == addr)
+ return true;
+
+ return is_module_cfi_trap(addr);
}
-EXPORT_SYMBOL(cfi_failure_handler);
+#endif /* CONFIG_ARCH_USES_CFI_TRAPS */
diff --git a/kernel/module.c b/kernel/module.c
index 296fe02323e9..411ae8c358e6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -57,6 +57,7 @@
#include <linux/bsearch.h>
#include <linux/dynamic_debug.h>
#include <linux/audit.h>
+#include <linux/cfi.h>
#include <uapi/linux/module.h>
#include "module-internal.h"
@@ -3871,8 +3872,9 @@ static int complete_formation(struct module *mod, struct load_info *info)
if (err < 0)
goto out;
- /* This relies on module_mutex for list integrity. */
+ /* These rely on module_mutex for list integrity. */
module_bug_finalize(info->hdr, info->sechdrs, mod);
+ module_cfi_finalize(info->hdr, info->sechdrs, mod);
module_enable_ro(mod, false);
module_enable_nx(mod);
@@ -3928,8 +3930,6 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,
return 0;
}
-static void cfi_init(struct module *mod);
-
/*
* Allocate and load the module: note that size of section 0 is always
* zero, and we rely on this for optional sections.
@@ -4059,9 +4059,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
flush_module_icache(mod);
- /* Setup CFI for the module. */
- cfi_init(mod);
-
/* Now copy in args */
mod->args = strndup_user(uargs, ~0UL >> 1);
if (IS_ERR(mod->args)) {
@@ -4502,31 +4499,6 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
#endif /* CONFIG_LIVEPATCH */
#endif /* CONFIG_KALLSYMS */
-static void cfi_init(struct module *mod)
-{
-#ifdef CONFIG_CFI_CLANG
- initcall_t *init;
- exitcall_t *exit;
-
- rcu_read_lock_sched();
- mod->cfi_check = (cfi_check_fn)
- find_kallsyms_symbol_value(mod, "__cfi_check");
- init = (initcall_t *)
- find_kallsyms_symbol_value(mod, "__cfi_jt_init_module");
- exit = (exitcall_t *)
- find_kallsyms_symbol_value(mod, "__cfi_jt_cleanup_module");
- rcu_read_unlock_sched();
-
- /* Fix init/exit functions to point to the CFI jump table */
- if (init)
- mod->init = *init;
-#ifdef CONFIG_MODULE_UNLOAD
- if (exit)
- mod->exit = *exit;
-#endif
-#endif
-}
-
/* Maximum number of characters written by module_flags() */
#define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 1d0e1e4dc3d2..0708896139cc 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -3,20 +3,10 @@
* Archs are free to supply their own linker scripts. ld will
* combine them automatically.
*/
-#ifdef CONFIG_CFI_CLANG
-# include <asm/page.h>
-# define ALIGN_CFI ALIGN(PAGE_SIZE)
-# define SANITIZER_DISCARDS *(.eh_frame)
-#else
-# define ALIGN_CFI
-# define SANITIZER_DISCARDS
-#endif
-
SECTIONS {
/DISCARD/ : {
*(.discard)
*(.discard.*)
- SANITIZER_DISCARDS
}
__ksymtab 0 : { *(SORT(___ksymtab+*)) }
@@ -31,6 +21,10 @@ SECTIONS {
__patchable_function_entries : { *(__patchable_function_entries) }
+#ifdef CONFIG_ARCH_USES_CFI_TRAPS
+ __kcfi_traps : { KEEP(*(.kcfi_traps)) }
+#endif
+
#ifdef CONFIG_LTO_CLANG
/*
* With CONFIG_LTO_CLANG, LLD always enables -fdata-sections and
@@ -51,15 +45,6 @@ SECTIONS {
*(.rodata .rodata.[0-9a-zA-Z_]*)
*(.rodata..L*)
}
-
- /*
- * With CONFIG_CFI_CLANG, we assume __cfi_check is at the beginning
- * of the .text section, and is aligned to PAGE_SIZE.
- */
- .text : ALIGN_CFI {
- *(.text.__cfi_check)
- *(.text .text.[0-9a-zA-Z_]* .text..L.cfi*)
- }
#endif
}
--
2.36.0.550.gb090851708-goog
More information about the linux-arm-kernel
mailing list