KCFI is a proposed forward-edge control-flow integrity scheme for
Clang, which is more suitable for kernel use than the existing CFI
scheme used by CONFIG_CFI_CLANG. KCFI doesn't require LTO, doesn't
alter function references to point to a jump table, and won't break
function address equality. The latest LLVM patch is here:
https://reviews.llvm.org/D119296
This RFC series replaces the current arm64 CFI implementation with
KCFI and adds support for x86_64.
KCFI requires assembly functions that are indirectly called from C
code to be annotated with type identifiers. As type information is
only available in C, the compiler emits expected type identifiers into
the symbol table, so they can be referenced from assembly without
having to hardcode type hashes. Patch 7 adds helper macros for
annotating functions, and patches 9 and 17 add annotations.
In case of a type mismatch, KCFI always traps. To support error
handling, the compiler generates a .kcfi_traps section for x86_64,
which contains the locations of each trap, and for arm64, encodes
the necessary register information to the ESR. Patches 10 and 20 add
arch-specific error handlers.
To test this series, you'll need to compile your own Clang toolchain
with the patches linked above. You can also find the complete source
tree here:
https://github.com/samitolvanen/llvm-project/commits/kcfi-rfc-v2
This series is also available in GitHub:
https://github.com/samitolvanen/linux/commits/kcfi-rfc-v2
Sami Tolvanen (21):
efi/libstub: Filter out CC_FLAGS_CFI
arm64/vdso: Filter out CC_FLAGS_CFI
kallsyms: Ignore __kcfi_typeid_
cfi: Remove CONFIG_CFI_CLANG_SHADOW
cfi: Drop __CFI_ADDRESSABLE
cfi: Switch to -fsanitize=kcfi
cfi: Add type helper macros
psci: Fix the function type for psci_initcall_t
arm64: Add types to indirect called assembly functions
arm64: Add CFI error handling
arm64: Drop unneeded __nocfi attributes
treewide: Drop function_nocfi
treewide: Drop WARN_ON_FUNCTION_MISMATCH
treewide: Drop __cficanonical
objtool: Don't warn about __cfi_ preambles falling through
x86/tools/relocs: Ignore __kcfi_typeid_ relocations
x86: Add types to indirectly called assembly functions
x86/purgatory: Disable CFI
x86/vdso: Disable CFI
x86: Add support for CONFIG_CFI_CLANG
init: Drop __nocfi from __init
Makefile | 13 +-
arch/Kconfig | 21 +-
arch/arm64/crypto/ghash-ce-core.S | 5 +-
arch/arm64/crypto/sm3-ce-core.S | 3 +-
arch/arm64/include/asm/brk-imm.h | 6 +
arch/arm64/include/asm/compiler.h | 16 -
arch/arm64/include/asm/ftrace.h | 2 +-
arch/arm64/include/asm/mmu_context.h | 4 +-
arch/arm64/kernel/acpi_parking_protocol.c | 2 +-
arch/arm64/kernel/alternative.c | 2 +-
arch/arm64/kernel/cpu-reset.S | 5 +-
arch/arm64/kernel/cpufeature.c | 4 +-
arch/arm64/kernel/ftrace.c | 2 +-
arch/arm64/kernel/machine_kexec.c | 2 +-
arch/arm64/kernel/psci.c | 2 +-
arch/arm64/kernel/smp_spin_table.c | 2 +-
arch/arm64/kernel/traps.c | 46 ++-
arch/arm64/kernel/vdso/Makefile | 3 +-
arch/arm64/mm/proc.S | 5 +-
arch/x86/Kconfig | 2 +
arch/x86/crypto/blowfish-x86_64-asm_64.S | 5 +-
arch/x86/entry/vdso/Makefile | 3 +-
arch/x86/include/asm/linkage.h | 12 +
arch/x86/kernel/traps.c | 60 +++-
arch/x86/lib/memcpy_64.S | 3 +-
arch/x86/purgatory/Makefile | 4 +
arch/x86/tools/relocs.c | 1 +
drivers/firmware/efi/libstub/Makefile | 2 +
drivers/firmware/psci/psci.c | 6 +-
drivers/misc/lkdtm/usercopy.c | 2 +-
include/asm-generic/bug.h | 16 -
include/asm-generic/vmlinux.lds.h | 37 +--
include/linux/cfi.h | 65 ++--
include/linux/cfi_types.h | 57 ++++
include/linux/compiler-clang.h | 6 +-
include/linux/compiler.h | 16 +-
include/linux/compiler_types.h | 4 -
include/linux/init.h | 6 +-
include/linux/module.h | 10 +-
include/linux/pci.h | 4 +-
kernel/cfi.c | 343 ++++------------------
kernel/kthread.c | 3 +-
kernel/module.c | 49 +---
kernel/workqueue.c | 2 +-
scripts/kallsyms.c | 1 +
scripts/module.lds.S | 23 +-
tools/objtool/check.c | 4 +
47 files changed, 357 insertions(+), 534 deletions(-)
create mode 100644 include/linux/cfi_types.h
--
2.36.0.550.gb090851708-goog
The __CFI_ADDRESSABLE macro is used for init_module and cleanup_module
to ensure we have the address of the CFI jump table, and with
CONFIG_X86_KERNEL_IBT to ensure LTO won't optimize away the symbols.
As __CFI_ADDRESSABLE is no longer necessary with -fsanitize=kcfi, add
a more flexible version of the __ADDRESSABLE macro and always ensure
these symbols won't be dropped.
Signed-off-by: Sami Tolvanen <[email protected]>
---
include/linux/cfi.h | 20 --------------------
include/linux/compiler.h | 6 ++++--
include/linux/module.h | 4 ++--
3 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/include/linux/cfi.h b/include/linux/cfi.h
index 4ab51c067007..2cdbc0fbd0ab 100644
--- a/include/linux/cfi.h
+++ b/include/linux/cfi.h
@@ -13,26 +13,6 @@ typedef void (*cfi_check_fn)(uint64_t id, void *ptr, void *diag);
/* Compiler-generated function in each module, and the kernel */
extern void __cfi_check(uint64_t id, void *ptr, void *diag);
-/*
- * Force the compiler to generate a CFI jump table entry for a function
- * and store the jump table address to __cfi_jt_<function>.
- */
-#define __CFI_ADDRESSABLE(fn, __attr) \
- const void *__cfi_jt_ ## fn __visible __attr = (void *)&fn
-
-#else /* !CONFIG_CFI_CLANG */
-
-#ifdef CONFIG_X86_KERNEL_IBT
-
-#define __CFI_ADDRESSABLE(fn, __attr) \
- const void *__cfi_jt_ ## fn __visible __attr = (void *)&fn
-
-#endif /* CONFIG_X86_KERNEL_IBT */
-
#endif /* CONFIG_CFI_CLANG */
-#ifndef __CFI_ADDRESSABLE
-#define __CFI_ADDRESSABLE(fn, __attr)
-#endif
-
#endif /* _LINUX_CFI_H */
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 219aa5ddbc73..9303f5fe5d89 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -221,9 +221,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
* otherwise, or eliminated entirely due to lack of references that are
* visible to the compiler.
*/
-#define __ADDRESSABLE(sym) \
- static void * __section(".discard.addressable") __used \
+#define ___ADDRESSABLE(sym, __attrs) \
+ static void * __used __attrs \
__UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)&sym;
+#define __ADDRESSABLE(sym) \
+ ___ADDRESSABLE(sym, __section(".discard.addressable"))
/**
* offset_to_ptr - convert a relative memory offset to an absolute pointer
diff --git a/include/linux/module.h b/include/linux/module.h
index 1e135fd5c076..87857275c047 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -132,7 +132,7 @@ extern void cleanup_module(void);
{ return initfn; } \
int init_module(void) __copy(initfn) \
__attribute__((alias(#initfn))); \
- __CFI_ADDRESSABLE(init_module, __initdata);
+ ___ADDRESSABLE(init_module, __initdata);
/* This is only required if you want to be unloadable. */
#define module_exit(exitfn) \
@@ -140,7 +140,7 @@ extern void cleanup_module(void);
{ return exitfn; } \
void cleanup_module(void) __copy(exitfn) \
__attribute__((alias(#exitfn))); \
- __CFI_ADDRESSABLE(cleanup_module, __exitdata);
+ ___ADDRESSABLE(cleanup_module, __exitdata);
#endif
--
2.36.0.550.gb090851708-goog
In preparation to switching to -fsanitize=kcfi, remove support for the
CFI module shadow that will no longer be needed.
Signed-off-by: Sami Tolvanen <[email protected]>
---
arch/Kconfig | 10 --
include/linux/cfi.h | 12 ---
kernel/cfi.c | 237 +-------------------------------------------
kernel/module.c | 15 ---
4 files changed, 1 insertion(+), 273 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 31c4fdc4a4ba..625db6376726 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -739,16 +739,6 @@ config CFI_CLANG
https://clang.llvm.org/docs/ControlFlowIntegrity.html
-config CFI_CLANG_SHADOW
- bool "Use CFI shadow to speed up cross-module checks"
- default y
- depends on CFI_CLANG && MODULES
- help
- If you select this option, the kernel builds a fast look-up table of
- CFI check functions in loaded modules to reduce performance overhead.
-
- If unsure, say Y.
-
config CFI_PERMISSIVE
bool "Use CFI in permissive mode"
depends on CFI_CLANG
diff --git a/include/linux/cfi.h b/include/linux/cfi.h
index c6dfc1ed0626..4ab51c067007 100644
--- a/include/linux/cfi.h
+++ b/include/linux/cfi.h
@@ -20,18 +20,6 @@ extern void __cfi_check(uint64_t id, void *ptr, void *diag);
#define __CFI_ADDRESSABLE(fn, __attr) \
const void *__cfi_jt_ ## fn __visible __attr = (void *)&fn
-#ifdef CONFIG_CFI_CLANG_SHADOW
-
-extern void cfi_module_add(struct module *mod, unsigned long base_addr);
-extern void cfi_module_remove(struct module *mod, unsigned long base_addr);
-
-#else
-
-static inline void cfi_module_add(struct module *mod, unsigned long base_addr) {}
-static inline void cfi_module_remove(struct module *mod, unsigned long base_addr) {}
-
-#endif /* CONFIG_CFI_CLANG_SHADOW */
-
#else /* !CONFIG_CFI_CLANG */
#ifdef CONFIG_X86_KERNEL_IBT
diff --git a/kernel/cfi.c b/kernel/cfi.c
index 9594cfd1cf2c..2cc0d01ea980 100644
--- a/kernel/cfi.c
+++ b/kernel/cfi.c
@@ -32,237 +32,6 @@ static inline void handle_cfi_failure(void *ptr)
}
#ifdef CONFIG_MODULES
-#ifdef CONFIG_CFI_CLANG_SHADOW
-/*
- * Index type. A 16-bit index can address at most (2^16)-2 pages (taking
- * into account SHADOW_INVALID), i.e. ~256M with 4k pages.
- */
-typedef u16 shadow_t;
-#define SHADOW_INVALID ((shadow_t)~0UL)
-
-struct cfi_shadow {
- /* Page index for the beginning of the shadow */
- unsigned long base;
- /* An array of __cfi_check locations (as indices to the shadow) */
- shadow_t shadow[1];
-} __packed;
-
-/*
- * The shadow covers ~128M from the beginning of the module region. If
- * the region is larger, we fall back to __module_address for the rest.
- */
-#define __SHADOW_RANGE (_UL(SZ_128M) >> PAGE_SHIFT)
-
-/* The in-memory size of struct cfi_shadow, always at least one page */
-#define __SHADOW_PAGES ((__SHADOW_RANGE * sizeof(shadow_t)) >> PAGE_SHIFT)
-#define SHADOW_PAGES max(1UL, __SHADOW_PAGES)
-#define SHADOW_SIZE (SHADOW_PAGES << PAGE_SHIFT)
-
-/* The actual size of the shadow array, minus metadata */
-#define SHADOW_ARR_SIZE (SHADOW_SIZE - offsetof(struct cfi_shadow, shadow))
-#define SHADOW_ARR_SLOTS (SHADOW_ARR_SIZE / sizeof(shadow_t))
-
-static DEFINE_MUTEX(shadow_update_lock);
-static struct cfi_shadow __rcu *cfi_shadow __read_mostly;
-
-/* Returns the index in the shadow for the given address */
-static inline int ptr_to_shadow(const struct cfi_shadow *s, unsigned long ptr)
-{
- unsigned long index;
- unsigned long page = ptr >> PAGE_SHIFT;
-
- if (unlikely(page < s->base))
- return -1; /* Outside of module area */
-
- index = page - s->base;
-
- if (index >= SHADOW_ARR_SLOTS)
- return -1; /* Cannot be addressed with shadow */
-
- return (int)index;
-}
-
-/* Returns the page address for an index in the shadow */
-static inline unsigned long shadow_to_ptr(const struct cfi_shadow *s,
- int index)
-{
- if (unlikely(index < 0 || index >= SHADOW_ARR_SLOTS))
- return 0;
-
- return (s->base + index) << PAGE_SHIFT;
-}
-
-/* Returns the __cfi_check function address for the given shadow location */
-static inline unsigned long shadow_to_check_fn(const struct cfi_shadow *s,
- int index)
-{
- if (unlikely(index < 0 || index >= SHADOW_ARR_SLOTS))
- return 0;
-
- if (unlikely(s->shadow[index] == SHADOW_INVALID))
- return 0;
-
- /* __cfi_check is always page aligned */
- return (s->base + s->shadow[index]) << PAGE_SHIFT;
-}
-
-static void prepare_next_shadow(const struct cfi_shadow __rcu *prev,
- struct cfi_shadow *next)
-{
- int i, index, check;
-
- /* Mark everything invalid */
- memset(next->shadow, 0xFF, SHADOW_ARR_SIZE);
-
- if (!prev)
- return; /* No previous shadow */
-
- /* If the base address didn't change, an update is not needed */
- if (prev->base == next->base) {
- memcpy(next->shadow, prev->shadow, SHADOW_ARR_SIZE);
- return;
- }
-
- /* Convert the previous shadow to the new address range */
- for (i = 0; i < SHADOW_ARR_SLOTS; ++i) {
- if (prev->shadow[i] == SHADOW_INVALID)
- continue;
-
- index = ptr_to_shadow(next, shadow_to_ptr(prev, i));
- if (index < 0)
- continue;
-
- check = ptr_to_shadow(next,
- shadow_to_check_fn(prev, prev->shadow[i]));
- if (check < 0)
- continue;
-
- next->shadow[index] = (shadow_t)check;
- }
-}
-
-static void add_module_to_shadow(struct cfi_shadow *s, struct module *mod,
- unsigned long min_addr, unsigned long max_addr)
-{
- int check_index;
- unsigned long check = (unsigned long)mod->cfi_check;
- unsigned long ptr;
-
- if (unlikely(!PAGE_ALIGNED(check))) {
- pr_warn("cfi: not using shadow for module %s\n", mod->name);
- return;
- }
-
- check_index = ptr_to_shadow(s, check);
- if (check_index < 0)
- return; /* Module not addressable with shadow */
-
- /* For each page, store the check function index in the shadow */
- for (ptr = min_addr; ptr <= max_addr; ptr += PAGE_SIZE) {
- int index = ptr_to_shadow(s, ptr);
-
- if (index >= 0) {
- /* Each page must only contain one module */
- WARN_ON_ONCE(s->shadow[index] != SHADOW_INVALID);
- s->shadow[index] = (shadow_t)check_index;
- }
- }
-}
-
-static void remove_module_from_shadow(struct cfi_shadow *s, struct module *mod,
- unsigned long min_addr, unsigned long max_addr)
-{
- unsigned long ptr;
-
- for (ptr = min_addr; ptr <= max_addr; ptr += PAGE_SIZE) {
- int index = ptr_to_shadow(s, ptr);
-
- if (index >= 0)
- s->shadow[index] = SHADOW_INVALID;
- }
-}
-
-typedef void (*update_shadow_fn)(struct cfi_shadow *, struct module *,
- unsigned long min_addr, unsigned long max_addr);
-
-static void update_shadow(struct module *mod, unsigned long base_addr,
- update_shadow_fn fn)
-{
- struct cfi_shadow *prev;
- struct cfi_shadow *next;
- unsigned long min_addr, max_addr;
-
- next = vmalloc(SHADOW_SIZE);
-
- mutex_lock(&shadow_update_lock);
- prev = rcu_dereference_protected(cfi_shadow,
- mutex_is_locked(&shadow_update_lock));
-
- if (next) {
- next->base = base_addr >> PAGE_SHIFT;
- prepare_next_shadow(prev, next);
-
- min_addr = (unsigned long)mod->core_layout.base;
- max_addr = min_addr + mod->core_layout.text_size;
- fn(next, mod, min_addr & PAGE_MASK, max_addr & PAGE_MASK);
-
- set_memory_ro((unsigned long)next, SHADOW_PAGES);
- }
-
- rcu_assign_pointer(cfi_shadow, next);
- mutex_unlock(&shadow_update_lock);
- synchronize_rcu();
-
- if (prev) {
- set_memory_rw((unsigned long)prev, SHADOW_PAGES);
- vfree(prev);
- }
-}
-
-void cfi_module_add(struct module *mod, unsigned long base_addr)
-{
- update_shadow(mod, base_addr, add_module_to_shadow);
-}
-
-void cfi_module_remove(struct module *mod, unsigned long base_addr)
-{
- update_shadow(mod, base_addr, remove_module_from_shadow);
-}
-
-static inline cfi_check_fn ptr_to_check_fn(const struct cfi_shadow __rcu *s,
- unsigned long ptr)
-{
- int index;
-
- if (unlikely(!s))
- return NULL; /* No shadow available */
-
- index = ptr_to_shadow(s, ptr);
- if (index < 0)
- return NULL; /* Cannot be addressed with shadow */
-
- return (cfi_check_fn)shadow_to_check_fn(s, index);
-}
-
-static inline cfi_check_fn find_shadow_check_fn(unsigned long ptr)
-{
- cfi_check_fn fn;
-
- rcu_read_lock_sched_notrace();
- fn = ptr_to_check_fn(rcu_dereference_sched(cfi_shadow), ptr);
- rcu_read_unlock_sched_notrace();
-
- return fn;
-}
-
-#else /* !CONFIG_CFI_CLANG_SHADOW */
-
-static inline cfi_check_fn find_shadow_check_fn(unsigned long ptr)
-{
- return NULL;
-}
-
-#endif /* CONFIG_CFI_CLANG_SHADOW */
static inline cfi_check_fn find_module_check_fn(unsigned long ptr)
{
@@ -291,11 +60,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
* up if necessary.
*/
RCU_NONIDLE({
- if (IS_ENABLED(CONFIG_CFI_CLANG_SHADOW))
- fn = find_shadow_check_fn(ptr);
-
- if (!fn)
- fn = find_module_check_fn(ptr);
+ fn = find_module_check_fn(ptr);
});
return fn;
diff --git a/kernel/module.c b/kernel/module.c
index 6cea788fd965..296fe02323e9 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2151,8 +2151,6 @@ void __weak module_arch_freeing_init(struct module *mod)
{
}
-static void cfi_cleanup(struct module *mod);
-
/* Free a module, remove from lists, etc. */
static void free_module(struct module *mod)
{
@@ -2194,9 +2192,6 @@ static void free_module(struct module *mod)
synchronize_rcu();
mutex_unlock(&module_mutex);
- /* Clean up CFI for the module. */
- cfi_cleanup(mod);
-
/* This may be empty, but that's OK */
module_arch_freeing_init(mod);
module_memfree(mod->init_layout.base);
@@ -4141,7 +4136,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
synchronize_rcu();
kfree(mod->args);
free_arch_cleanup:
- cfi_cleanup(mod);
module_arch_cleanup(mod);
free_modinfo:
free_modinfo(mod);
@@ -4530,15 +4524,6 @@ static void cfi_init(struct module *mod)
if (exit)
mod->exit = *exit;
#endif
-
- cfi_module_add(mod, module_addr_min);
-#endif
-}
-
-static void cfi_cleanup(struct module *mod)
-{
-#ifdef CONFIG_CFI_CLANG
- cfi_module_remove(mod, module_addr_min);
#endif
}
--
2.36.0.550.gb090851708-goog
Ignore __kcfi_typeid_ symbols. These are compiler-generated constants
that contain CFI type identifiers.
Signed-off-by: Sami Tolvanen <[email protected]>
---
arch/x86/tools/relocs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index e2c5b296120d..2925074b9a58 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -56,6 +56,7 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
"^(xen_irq_disable_direct_reloc$|"
"xen_save_fl_direct_reloc$|"
"VDSO|"
+ "__kcfi_typeid_|"
"__crc_)",
/*
--
2.36.0.550.gb090851708-goog
With -fsanitize=kcfi, CONFIG_CFI_CLANG no longer has issues
with address space confusion in functions that switch to linear
mapping. Now that the indirectly called assembly functions have
type annotations, drop the __nocfi attributes.
Suggested-by: Mark Rutland <[email protected]>
Signed-off-by: Sami Tolvanen <[email protected]>
---
arch/arm64/include/asm/mmu_context.h | 2 +-
arch/arm64/kernel/alternative.c | 2 +-
arch/arm64/kernel/cpufeature.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 6770667b34a3..ca0140d0b8cf 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -143,7 +143,7 @@ static inline void cpu_install_ttbr0(phys_addr_t ttbr0, unsigned long t0sz)
* Atomically replaces the active TTBR1_EL1 PGD with a new VA-compatible PGD,
* avoiding the possibility of conflicting TLB entries being allocated.
*/
-static inline void __nocfi cpu_replace_ttbr1(pgd_t *pgdp)
+static inline void cpu_replace_ttbr1(pgd_t *pgdp)
{
typedef void (ttbr_replace_func)(phys_addr_t);
extern ttbr_replace_func idmap_cpu_replace_ttbr1;
diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index 7bbf5104b7b7..e98466bab633 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -133,7 +133,7 @@ static void clean_dcache_range_nopatch(u64 start, u64 end)
} while (cur += d_size, cur < end);
}
-static void __nocfi __apply_alternatives(struct alt_region *region, bool is_module,
+static void __apply_alternatives(struct alt_region *region, bool is_module,
unsigned long *feature_mask)
{
struct alt_instr *alt;
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index d72c4b4d389c..af78dcacf9fe 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1596,7 +1596,7 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
}
#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
-static void __nocfi
+static void
kpti_install_ng_mappings(const struct arm64_cpu_capabilities *__unused)
{
typedef void (kpti_remap_fn)(int, int, phys_addr_t);
--
2.36.0.550.gb090851708-goog
With -fsanitize=kcfi, CFI always traps. Add arm64 support for handling
CFI failures. The registers containing the target address and the
expected type are encoded in the first ten bits of the ESR as follows:
- 0-4: n, where the register Xn contains the target address
- 5-9: m, where the register Wm contains the type hash
Suggested-by: Mark Rutland <[email protected]>
Signed-off-by: Sami Tolvanen <[email protected]>
---
arch/arm64/include/asm/brk-imm.h | 6 +++++
arch/arm64/kernel/traps.c | 46 +++++++++++++++++++++++++++++---
2 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h
index ec7720dbe2c8..6e000113e508 100644
--- a/arch/arm64/include/asm/brk-imm.h
+++ b/arch/arm64/include/asm/brk-imm.h
@@ -17,6 +17,7 @@
* 0x401: for compile time BRK instruction
* 0x800: kernel-mode BUG() and WARN() traps
* 0x9xx: tag-based KASAN trap (allowed values 0x900 - 0x9ff)
+ * 0x8xxx: Control-Flow Integrity traps
*/
#define KPROBES_BRK_IMM 0x004
#define UPROBES_BRK_IMM 0x005
@@ -28,4 +29,9 @@
#define KASAN_BRK_IMM 0x900
#define KASAN_BRK_MASK 0x0ff
+#define CFI_BRK_IMM_TARGET GENMASK(4, 0)
+#define CFI_BRK_IMM_TYPE GENMASK(9, 5)
+#define CFI_BRK_IMM_BASE 0x8000
+#define CFI_BRK_IMM_MASK (CFI_BRK_IMM_TARGET | CFI_BRK_IMM_TYPE)
+
#endif
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 0529fd57567e..17b083b683f4 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -26,6 +26,7 @@
#include <linux/syscalls.h>
#include <linux/mm_types.h>
#include <linux/kasan.h>
+#include <linux/cfi.h>
#include <asm/atomic.h>
#include <asm/bug.h>
@@ -990,6 +991,37 @@ static struct break_hook bug_break_hook = {
.imm = BUG_BRK_IMM,
};
+#ifdef CONFIG_CFI_CLANG
+static int cfi_handler(struct pt_regs *regs, unsigned int esr)
+{
+ unsigned long target, type;
+
+ target = pt_regs_read_reg(regs, FIELD_GET(CFI_BRK_IMM_TARGET, esr));
+ type = pt_regs_read_reg(regs, FIELD_GET(CFI_BRK_IMM_TYPE, esr));
+
+ switch (report_cfi_failure(regs, regs->pc, target, type)) {
+ case BUG_TRAP_TYPE_BUG:
+ die("Oops - CFI", regs, 0);
+ break;
+
+ case BUG_TRAP_TYPE_WARN:
+ break;
+
+ default:
+ return DBG_HOOK_ERROR;
+ }
+
+ arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
+ return DBG_HOOK_HANDLED;
+}
+
+static struct break_hook cfi_break_hook = {
+ .fn = cfi_handler,
+ .imm = CFI_BRK_IMM_BASE,
+ .mask = CFI_BRK_IMM_MASK,
+};
+#endif /* CONFIG_CFI_CLANG */
+
static int reserved_fault_handler(struct pt_regs *regs, unsigned int esr)
{
pr_err("%s generated an invalid instruction at %pS!\n",
@@ -1051,6 +1083,9 @@ static struct break_hook kasan_break_hook = {
};
#endif
+
+#define esr_comment(esr) ((esr) & ESR_ELx_BRK64_ISS_COMMENT_MASK)
+
/*
* Initial handler for AArch64 BRK exceptions
* This handler only used until debug_traps_init().
@@ -1058,10 +1093,12 @@ static struct break_hook kasan_break_hook = {
int __init early_brk64(unsigned long addr, unsigned int esr,
struct pt_regs *regs)
{
+#ifdef CONFIG_CFI_CLANG
+ if ((esr_comment(esr) & ~CFI_BRK_IMM_MASK) == CFI_BRK_IMM_BASE)
+ return cfi_handler(regs, esr) != DBG_HOOK_HANDLED;
+#endif
#ifdef CONFIG_KASAN_SW_TAGS
- unsigned int comment = esr & ESR_ELx_BRK64_ISS_COMMENT_MASK;
-
- if ((comment & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
+ if ((esr_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
return kasan_handler(regs, esr) != DBG_HOOK_HANDLED;
#endif
return bug_handler(regs, esr) != DBG_HOOK_HANDLED;
@@ -1070,6 +1107,9 @@ int __init early_brk64(unsigned long addr, unsigned int esr,
void __init trap_init(void)
{
register_kernel_break_hook(&bug_break_hook);
+#ifdef CONFIG_CFI_CLANG
+ register_kernel_break_hook(&cfi_break_hook);
+#endif
register_kernel_break_hook(&fault_break_hook);
#ifdef CONFIG_KASAN_SW_TAGS
register_kernel_break_hook(&kasan_break_hook);
--
2.36.0.550.gb090851708-goog
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 <[email protected]>
---
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
CONFIG_CFI_CLANG doesn't use a jump table anymore and therefore,
won't change function references to point elsewhere. Remove the
__cficanonical attribute and all uses of it.
Signed-off-by: Sami Tolvanen <[email protected]>
---
include/linux/compiler_types.h | 4 ----
include/linux/init.h | 4 ++--
include/linux/pci.h | 4 ++--
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 1c2c33ae1b37..bdd2526af46a 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -263,10 +263,6 @@ struct ftrace_likely_data {
# define __nocfi
#endif
-#ifndef __cficanonical
-# define __cficanonical
-#endif
-
/*
* Any place that could be marked with the "alloc_size" attribute is also
* a place to be marked with the "malloc" attribute. Do this as part of the
diff --git a/include/linux/init.h b/include/linux/init.h
index baf0b29a7010..76058c9e0399 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -220,8 +220,8 @@ extern bool initcall_debug;
__initcall_name(initstub, __iid, id)
#define __define_initcall_stub(__stub, fn) \
- int __init __cficanonical __stub(void); \
- int __init __cficanonical __stub(void) \
+ int __init __stub(void); \
+ int __init __stub(void) \
{ \
return fn(); \
} \
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 60adf42460ab..3cc50c4e3c64 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2021,8 +2021,8 @@ enum pci_fixup_pass {
#ifdef CONFIG_LTO_CLANG
#define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \
class_shift, hook, stub) \
- void __cficanonical stub(struct pci_dev *dev); \
- void __cficanonical stub(struct pci_dev *dev) \
+ void stub(struct pci_dev *dev); \
+ void stub(struct pci_dev *dev) \
{ \
hook(dev); \
} \
--
2.36.0.550.gb090851708-goog
Disable CONFIG_CFI_CLANG for the stand-alone purgatory.ro.
Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Tested-by: Nick Desaulniers <[email protected]>
Tested-by: Sedat Dilek <[email protected]>
---
arch/x86/purgatory/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index ae53d54d7959..b3fa947fa38b 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -55,6 +55,10 @@ ifdef CONFIG_RETPOLINE
PURGATORY_CFLAGS_REMOVE += $(RETPOLINE_CFLAGS)
endif
+ifdef CONFIG_CFI_CLANG
+PURGATORY_CFLAGS_REMOVE += $(CC_FLAGS_CFI)
+endif
+
CFLAGS_REMOVE_purgatory.o += $(PURGATORY_CFLAGS_REMOVE)
CFLAGS_purgatory.o += $(PURGATORY_CFLAGS)
--
2.36.0.550.gb090851708-goog
The __cfi_ preambles contain valid instructions, which embed KCFI
type information in the following format:
__cfi_function:
int3
int3
mov <id>, %eax
int3
int3
function:
...
While the preambles are STT_FUNC and contain valid instructions,
they are not executed and always fall through. Skip the warning for
them.
Signed-off-by: Sami Tolvanen <[email protected]>
---
tools/objtool/check.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index ca5b74603008..88f005ae6dcc 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3311,6 +3311,10 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
next_insn = next_insn_to_validate(file, insn);
if (func && insn->func && func != insn->func->pfunc) {
+ /* Ignore KCFI type preambles, which always fall through */
+ if (!strncmp(func->name, "__cfi_", 6))
+ return 0;
+
WARN("%s() falls through to next function %s()",
func->name, insn->func->name);
return 1;
--
2.36.0.550.gb090851708-goog
Explicitly filter out CC_FLAGS_CFI in preparation for the flags being
removed from CC_FLAGS_LTO.
Signed-off-by: Sami Tolvanen <[email protected]>
---
drivers/firmware/efi/libstub/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index d0537573501e..234fb2910622 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -39,6 +39,8 @@ KBUILD_CFLAGS := $(cflags-y) -Os -DDISABLE_BRANCH_PROFILING \
# remove SCS flags from all objects in this directory
KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_SCS), $(KBUILD_CFLAGS))
+# disable CFI
+KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_CFI), $(KBUILD_CFLAGS))
# disable LTO
KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO), $(KBUILD_CFLAGS))
--
2.36.0.550.gb090851708-goog
On Fri, May 13, 2022 at 01:21:43PM -0700, Sami Tolvanen wrote:
> The __CFI_ADDRESSABLE macro is used for init_module and cleanup_module
> to ensure we have the address of the CFI jump table, and with
> CONFIG_X86_KERNEL_IBT to ensure LTO won't optimize away the symbols.
> As __CFI_ADDRESSABLE is no longer necessary with -fsanitize=kcfi, add
> a more flexible version of the __ADDRESSABLE macro and always ensure
> these symbols won't be dropped.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
These macros were so fun to build, though! ;)
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:44PM -0700, Sami Tolvanen wrote:
> 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 <[email protected]>
Yes please. And just to note it somewhere: landing the KCFI
implementation on Clang depends on this series being accepted (i.e. if
the arm64 and x86 maintainers are happy with this series, then that'll
unblock landing it in Clang (no reason to land something that won't get
used.)
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:39PM -0700, Sami Tolvanen wrote:
> Explicitly filter out CC_FLAGS_CFI in preparation for the flags being
> removed from CC_FLAGS_LTO.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
Is it worth merging all the "existing" CC_FLAGS_LTO/CFI splits into a
single patch? Either way:
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:42PM -0700, Sami Tolvanen wrote:
> In preparation to switching to -fsanitize=kcfi, remove support for the
> CFI module shadow that will no longer be needed.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
I like that most of this series is dropping code. :P
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:48PM -0700, Sami Tolvanen wrote:
> With -fsanitize=kcfi, CFI always traps. Add arm64 support for handling
> CFI failures. The registers containing the target address and the
> expected type are encoded in the first ten bits of the ESR as follows:
>
> - 0-4: n, where the register Xn contains the target address
> - 5-9: m, where the register Wm contains the type hash
>
> Suggested-by: Mark Rutland <[email protected]>
> Signed-off-by: Sami Tolvanen <[email protected]>
It might be nice just to include an example exception Oops in this
commit log.
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:52PM -0700, Sami Tolvanen wrote:
> CONFIG_CFI_CLANG doesn't use a jump table anymore and therefore,
> won't change function references to point elsewhere. Remove the
> __cficanonical attribute and all uses of it.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
> ---
> include/linux/compiler_types.h | 4 ----
> include/linux/init.h | 4 ++--
> include/linux/pci.h | 4 ++--
> 3 files changed, 4 insertions(+), 8 deletions(-)
I think this is missing removing it from include/linux/compiler-clang.h ?
With that done (or explained why not):
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:54PM -0700, Sami Tolvanen wrote:
> Ignore __kcfi_typeid_ symbols. These are compiler-generated constants
> that contain CFI type identifiers.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:49PM -0700, Sami Tolvanen wrote:
> With -fsanitize=kcfi, CONFIG_CFI_CLANG no longer has issues
> with address space confusion in functions that switch to linear
> mapping. Now that the indirectly called assembly functions have
> type annotations, drop the __nocfi attributes.
>
> Suggested-by: Mark Rutland <[email protected]>
> Signed-off-by: Sami Tolvanen <[email protected]>
It looks like there are still other cases that continue to require
__nocfi, yes? It looks like after this series, it's still BPF?
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:53PM -0700, Sami Tolvanen wrote:
> The __cfi_ preambles contain valid instructions, which embed KCFI
> type information in the following format:
>
> __cfi_function:
> int3
> int3
> mov <id>, %eax
> int3
> int3
> function:
> ...
>
> While the preambles are STT_FUNC and contain valid instructions,
> they are not executed and always fall through. Skip the warning for
> them.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:56PM -0700, Sami Tolvanen wrote:
> Disable CONFIG_CFI_CLANG for the stand-alone purgatory.ro.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Fri, May 13, 2022 at 01:21:44PM -0700, Sami Tolvanen wrote:
> 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 <[email protected]>
Looks good on arm64 with LKDTM, too:
[ 96.904483] lkdtm: Performing direct entry CFI_FORWARD_PROTO
[ 96.904718] lkdtm: Calling matched prototype ...
[ 96.904829] lkdtm: Calling mismatched prototype ...
[ 96.905250] CFI failure at lkdtm_CFI_FORWARD_PROTO+0x54/0x94 [lkdtm] (target: lkdtm_increment_int+0x0/0x20 [lkdtm]; expected type: 0x7e0c52a5)
Tested-by: Kees Cook <[email protected]>
-Kees
--
Kees Cook
On Sat, May 14, 2022 at 2:51 PM Kees Cook <[email protected]> wrote:
>
> On Fri, May 13, 2022 at 01:21:48PM -0700, Sami Tolvanen wrote:
> > With -fsanitize=kcfi, CFI always traps. Add arm64 support for handling
> > CFI failures. The registers containing the target address and the
> > expected type are encoded in the first ten bits of the ESR as follows:
> >
> > - 0-4: n, where the register Xn contains the target address
> > - 5-9: m, where the register Wm contains the type hash
> >
> > Suggested-by: Mark Rutland <[email protected]>
> > Signed-off-by: Sami Tolvanen <[email protected]>
>
> It might be nice just to include an example exception Oops in this
> commit log.
Agreed, I'll add an example.
Sami
On Sat, May 14, 2022 at 2:42 PM Kees Cook <[email protected]> wrote:
>
> On Fri, May 13, 2022 at 01:21:39PM -0700, Sami Tolvanen wrote:
> > Explicitly filter out CC_FLAGS_CFI in preparation for the flags being
> > removed from CC_FLAGS_LTO.
> >
> > Signed-off-by: Sami Tolvanen <[email protected]>
>
> Is it worth merging all the "existing" CC_FLAGS_LTO/CFI splits into a
> single patch? Either way:
Sure, I can merge them in the next version.
Sami
On Mon, May 16, 2022 at 10:31 AM Sedat Dilek <[email protected]> wrote:
>> Anything Like LLVM cmake Options to be condidered and Set?
>
>
> I activate Clang and LLD ad projects - OK - enough?
Clang and LLD should be sufficient.
>> This series is also available in GitHub:
>>
>> https://github.com/samitolvanen/linux/commits/kcfi-rfc-v2
>>
>>
>>
>
>> Can you please add a history of what changed?
Oops, I forgot to include that.
Changes in v2:
- Changed the compiler patch to encode arm64 target and type details
in the ESR, and updated the kernel error handling patch accordingly.
- Changed the compiler patch to embed the x86_64 type hash in a valid
instruction to avoid special casing objtool instruction decoding, and
added a __cfi_ symbol for the preamble. Changed the kernel error
handling and manual type annotations to match.
- Dropped the .kcfi_types section as that’s no longer needed by
objtool, and changed the objtool patch to simply ignore the __cfi_
preambles falling through.
- Dropped the .kcfi_traps section on arm64 as it’s no longer needed,
and moved the trap look-up code behind CONFIG_ARCH_USES_CFI_TRAPS,
which is selected only for x86_64.
- Dropped __nocfi attributes from arm64 code where CFI was disabled
due to address space confusion issues, and added type annotations to
relevant assembly functions.
- Dropped __nocfi from __init.
> Nathan has a i915 cfi patch in His personal kernel.org Git.
> Is this relevant to kcfi?
It fixes a type mismatch, so in that sense it's relevant.
> To distinguish between clang-cfi we should use different kbuild variables for kcfi.
The plan is to replace the current CFI implementation. Does reusing
the kbuild variable names cause a problem?
Sami
On Sat, May 14, 2022 at 2:56 PM Kees Cook <[email protected]> wrote:
>
> On Fri, May 13, 2022 at 01:21:52PM -0700, Sami Tolvanen wrote:
> > CONFIG_CFI_CLANG doesn't use a jump table anymore and therefore,
> > won't change function references to point elsewhere. Remove the
> > __cficanonical attribute and all uses of it.
> >
> > Signed-off-by: Sami Tolvanen <[email protected]>
> > ---
> > include/linux/compiler_types.h | 4 ----
> > include/linux/init.h | 4 ++--
> > include/linux/pci.h | 4 ++--
> > 3 files changed, 4 insertions(+), 8 deletions(-)
>
> I think this is missing removing it from include/linux/compiler-clang.h ?
That was removed in the earlier patch that switched the CFI implementation.
> With that done (or explained why not):
I'll add a note about it to the commit message.
Sami
On Fri, May 13, 2022 at 01:21:38PM -0700, Sami Tolvanen wrote:
> KCFI is a proposed forward-edge control-flow integrity scheme for
> Clang, which is more suitable for kernel use than the existing CFI
> scheme used by CONFIG_CFI_CLANG. KCFI doesn't require LTO, doesn't
> alter function references to point to a jump table, and won't break
> function address equality. The latest LLVM patch is here:
>
> https://reviews.llvm.org/D119296
>
> This RFC series replaces the current arm64 CFI implementation with
> KCFI and adds support for x86_64.
You have some weird behaviour vs weak functions (I so hate those)...
100: 0000000000000980 9 FUNC LOCAL DEFAULT 2 __cfi_free_initmem
233: 0000000000000989 35 FUNC WEAK DEFAULT 2 free_initmem
With the result that on the final link:
179: 00000000000009b0 9 FUNC LOCAL DEFAULT 1 __cfi_free_initmem
8689: 00000000000007f0 9 FUNC LOCAL DEFAULT 65 __cfi_free_initmem
173283: 00000000000007f9 198 FUNC GLOBAL DEFAULT 65 free_initmem
This is getting me objtool issues (I'll fix them) but perhaps it's
something you can do something about as well.
On Sat, May 14, 2022 at 2:54 PM Kees Cook <[email protected]> wrote:
>
> On Fri, May 13, 2022 at 01:21:49PM -0700, Sami Tolvanen wrote:
> > With -fsanitize=kcfi, CONFIG_CFI_CLANG no longer has issues
> > with address space confusion in functions that switch to linear
> > mapping. Now that the indirectly called assembly functions have
> > type annotations, drop the __nocfi attributes.
> >
> > Suggested-by: Mark Rutland <[email protected]>
> > Signed-off-by: Sami Tolvanen <[email protected]>
>
> It looks like there are still other cases that continue to require
> __nocfi, yes? It looks like after this series, it's still BPF?
Yes, BPF is the only remaining user of __nocfi after this series.
Sami
On Mon, May 16, 2022 at 7:57 PM Sami Tolvanen <[email protected]> wrote:
>
> On Mon, May 16, 2022 at 10:31 AM Sedat Dilek <[email protected]> wrote:
> >> Anything Like LLVM cmake Options to be condidered and Set?
> >
> >
> > I activate Clang and LLD ad projects - OK - enough?
>
> Clang and LLD should be sufficient.
>
Before I give this a try...
Some questions about the LLVM toolchain requirements...
I use tc-build to generate a selfmade LLVM toolchain for X86
(stage1-only + BPF).
$ python3 ./build-llvm.py --no-update --build-type Release -p
clang;lld -t X86;BPF --clang-vendor dileks -B
/home/dileks/src/llvm-toolchain/build -I /opt/llvm-toolchain
--check-targets clang lld --build-stage1-only --install-stage1-only -D
LLVM_PARALLEL_LINK_JOBS=1 --show-build-commands
Link: https://github.com/ClangBuiltLinux/tc-build.git
tc-build uses a GOOD_REVISION for regular kernel-builds, this is
currently 3b2e605e33bd.
$ git describe
llvmorg-15-init-5163-g3b2e605e33bd
$ git show -1 42a879eb2f22
commit 42a879eb2f22af6d1b85983c12fac68b2e9a5e03
Author: Nathan Chancellor <[email protected]>
Date: Mon Mar 21 09:19:26 2022 -0700
build-llvm.py: Update known good revision
Qualified with https://github.com/nathanchance/llvm-kernel-testing.
Signed-off-by: Nathan Chancellor <[email protected]>
diff --git a/build-llvm.py b/build-llvm.py
index c3aade1092ec..54e5d4729a1a 100755
--- a/build-llvm.py
+++ b/build-llvm.py
@@ -16,7 +16,7 @@ import urllib.request as request
from urllib.error import URLError
# This is a known good revision of LLVM for building the kernel
-GOOD_REVISION = '08f70adedb775ce6d41a1f8ad75c4bac225efb5b'
+GOOD_REVISION = '3b2e605e33bd9017ff2eff1493add07822f9d58b'
class Directories:
So, your LLVM git is approx. 5.200 commits further.
$ git log --oneline
tc-build/GOOD_REVISION-20210321..for-15/kcfi-rfc-v2-samitolvanen | wc
-l
5190
Is your tags/kcfi-rfc-v2 the recommended LLVM toolchain for testing kcfi-rfc-v2?
What I want to ask is if your commit well tested for x86 (and arm64)
means build and boot on bare metal?
From Nathan I know if he says I have run kernel-tests - it works.
"Qualified with https://github.com/nathanchance/llvm-kernel-testing."
Just for the records:
You definitely need a pre-LLVM-15 toolchain + KCFI sanitizer patch?
LLVM-14?
> >> This series is also available in GitHub:
> >>
> >> https://github.com/samitolvanen/linux/commits/kcfi-rfc-v2
> >>
> >>
> >>
> >
> >> Can you please add a history of what changed?
>
> Oops, I forgot to include that.
>
Thanks.
Please fold this ChangeLog into kcfi-rfc-v3.
> Changes in v2:
> - Changed the compiler patch to encode arm64 target and type details
> in the ESR, and updated the kernel error handling patch accordingly.
> - Changed the compiler patch to embed the x86_64 type hash in a valid
> instruction to avoid special casing objtool instruction decoding, and
> added a __cfi_ symbol for the preamble. Changed the kernel error
> handling and manual type annotations to match.
> - Dropped the .kcfi_types section as that’s no longer needed by
> objtool, and changed the objtool patch to simply ignore the __cfi_
> preambles falling through.
> - Dropped the .kcfi_traps section on arm64 as it’s no longer needed,
> and moved the trap look-up code behind CONFIG_ARCH_USES_CFI_TRAPS,
> which is selected only for x86_64.
> - Dropped __nocfi attributes from arm64 code where CFI was disabled
> due to address space confusion issues, and added type annotations to
> relevant assembly functions.
> - Dropped __nocfi from __init.
>
> > Nathan has a i915 cfi patch in His personal kernel.org Git.
> > Is this relevant to kcfi?
>
> It fixes a type mismatch, so in that sense it's relevant.
>
Here the link to patch "drm/i915: Fix CFI violation with show_dynamic_id()":
https://git.kernel.org/pub/scm/linux/kernel/git/nathan/linux.git/commit/?h=submitted/i915-cfi-fix&id=53735be6dc53453fcfbac658e847b54360e73871
> > To distinguish between clang-cfi we should use different kbuild variables for kcfi.
>
> The plan is to replace the current CFI implementation. Does reusing
> the kbuild variable names cause a problem?
>
No hurry.
Afaics someone in the long list of comments to single patches was
thinking of when GCC has "KCFI" support implemented...
You say no need to build your kernel with LTO...
That sounds good.
Currently, I build my kernels with Clang-14 and CONFIG_LTO_CLANG_THIN=y.
Does something speak against using CONFIG_LTO_CLANG_THIN=y with KCFI support?
Build-time?
Disc-usage?
Thanks for answering my questions.
- Sedat -
On Tue, May 17, 2022 at 09:33:47AM +0200, Sedat Dilek wrote:
> Is your tags/kcfi-rfc-v2 the recommended LLVM toolchain for testing kcfi-rfc-v2?
>
> What I want to ask is if your commit well tested for x86 (and arm64)
> means build and boot on bare metal?
I have run kCFI (v1, I haven't had time to test v2) on x86_64 hardware,
both AMD and Intel. I have only found two failures so far: the i915
issue that I mention below and a failure in the KVM subsystem, which I
can see by just running QEMU:
[ 124.344654] CFI failure at kvm_mmu_notifier_invalidate_range_end+0xca/0x177 [kvm] (target: kvm_null_fn+0x0/0x7 [kvm]; expected type: 0x1e2c5b9c)
[ 124.344691] WARNING: CPU: 5 PID: 2767 at kvm_mmu_notifier_invalidate_range_end+0xca/0x177 [kvm]
[ 124.344708] Modules linked in: ...
[ 124.344737] CPU: 5 PID: 2767 Comm: qemu-system-x86 Tainted: P 5.18.0-rc6-debug-00033-g1d5284aff7cd #1 8c4966c7fb24f3345076747feebac5fb340a4797
[ 124.344738] Hardware name: ASUS System Product Name/PRIME Z590M-PLUS, BIOS 1203 10/27/2021
[ 124.344738] RIP: 0010:kvm_mmu_notifier_invalidate_range_end+0xca/0x177 [kvm]
[ 124.344756] Code: 48 89 da e8 88 16 86 dd 48 85 c0 89 6c 24 0c 74 30 4d 85 f6 74 36 48 c7 c5 19 c8 b4 c0 4c 8b 34 24 81 7d fa 9c 5b 2c 1e 74 02 <0f> 0b 48 89 c7 4c 89 fe 48 89 da e8 b6 16 86 dd 48 85 c0 75 e2 eb
[ 124.344757] RSP: 0018:ffffc033039d78e8 EFLAGS: 00010282
[ 124.344758] RAX: ffffa0a141ed9450 RBX: 00007fb897e9efff RCX: ffffa0a143c9d850
[ 124.344758] RDX: 00007fb897e9efff RSI: 00007fb897e9e000 RDI: ffffc03304499cf8
[ 124.344759] RBP: ffffffffc0b4c819 R08: 0000000000000000 R09: 0000000000000000
[ 124.344759] R10: 0000000000000000 R11: ffffffffc0b4c639 R12: ffffc03304499000
[ 124.344760] R13: ffffc033044a30a0 R14: ffffc033044a3120 R15: 00007fb897e9e000
[ 124.344760] FS: 00007fbca4bc3640(0000) GS:ffffa0a87f540000(0000) knlGS:0000000000000000
[ 124.344761] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 124.344761] CR2: 0000000000000000 CR3: 0000000119f74006 CR4: 0000000000772ee0
[ 124.344762] PKRU: 55555554
[ 124.344762] Call Trace:
[ 124.344762] <TASK>
[ 124.344763] __mmu_notifier_invalidate_range_end+0xa1/0xd7
[ 124.344764] wp_page_copy+0x592/0x920
[ 124.344766] __handle_mm_fault+0x820/0x8f0
[ 124.344767] handle_mm_fault+0xe0/0x227
[ 124.344768] __get_user_pages+0x17a/0x430
[ 124.344769] get_user_pages_unlocked+0xd9/0x327
[ 124.344770] hva_to_pfn+0xfa/0x3f7 [kvm 950e483805e01f967a3e331506278d766494a4cc]
[ 124.344788] kvm_faultin_pfn+0xc3/0x2f0 [kvm 950e483805e01f967a3e331506278d766494a4cc]
[ 124.344806] ? fast_page_fault+0x400/0x4c0 [kvm 950e483805e01f967a3e331506278d766494a4cc]
[ 124.344823] direct_page_fault+0x130/0x350 [kvm 950e483805e01f967a3e331506278d766494a4cc]
[ 124.344841] kvm_mmu_page_fault+0x176/0x2f7 [kvm 950e483805e01f967a3e331506278d766494a4cc]
[ 124.344858] vmx_handle_exit+0xe/0x37 [kvm_intel 647f514f18fc365045dc57db46d2bb4930b746eb]
[ 124.344863] vcpu_enter_guest+0xbff/0x1030 [kvm 950e483805e01f967a3e331506278d766494a4cc]
[ 124.344881] vcpu_run+0x65/0x2f0 [kvm 950e483805e01f967a3e331506278d766494a4cc]
[ 124.344898] kvm_arch_vcpu_ioctl_run+0x15f/0x3f7 [kvm 950e483805e01f967a3e331506278d766494a4cc]
[ 124.344916] kvm_vcpu_ioctl+0x547/0x627 [kvm 950e483805e01f967a3e331506278d766494a4cc]
[ 124.344933] ? syscall_exit_to_user_mode+0x24/0x47
[ 124.344934] ? do_syscall_64+0x7a/0x97
[ 124.344935] ? __fget_files+0xa1/0xc0
[ 124.344936] __se_sys_ioctl+0x7c/0xc0
[ 124.344937] do_syscall_64+0x6c/0x97
[ 124.344938] ? do_syscall_64+0x7a/0x97
[ 124.344939] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 124.344940] RIP: 0033:0x7fbca75f7b1f
[ 124.344940] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 18 48 8b 44 24 18 64 48 2b 04 25 28 00 00
[ 124.344941] RSP: 002b:00007fbca4bc2550 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 124.344942] RAX: ffffffffffffffda RBX: 000000000000ae80 RCX: 00007fbca75f7b1f
[ 124.344942] RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 000000000000000e
[ 124.344943] RBP: 000056351ba661d0 R08: 000056351a74dc48 R09: 0000000000000004
[ 124.344943] R10: 004818cf4f129e11 R11: 0000000000000246 R12: 0000000000000000
[ 124.344943] R13: 0000000000000000 R14: 00007fbca814f004 R15: 0000000000000000
[ 124.344944] </TASK>
[ 124.344945] ---[ end trace 0000000000000000 ]---
I don't do any bare metal testing for the tc-build known good revision
bumps, just QEMU tests with boot-utils.
> Just for the records:
> You definitely need a pre-LLVM-15 toolchain + KCFI sanitizer patch?
> LLVM-14?
The feature won't land in LLVM 14, so there is little point to
backporting and testing it on LLVM 14. This is a work in progress
feature so it has to target the main branch. I have fairly good coverage
of tip of tree locally and we have solid coverage through CI so we'll
know if something major happens, it should be pretty safe to test Sami's
series.
> > > Nathan has a i915 cfi patch in His personal kernel.org Git.
> > > Is this relevant to kcfi?
> >
> > It fixes a type mismatch, so in that sense it's relevant.
> >
>
> Here the link to patch "drm/i915: Fix CFI violation with show_dynamic_id()":
>
> https://git.kernel.org/pub/scm/linux/kernel/git/nathan/linux.git/commit/?h=submitted/i915-cfi-fix&id=53735be6dc53453fcfbac658e847b54360e73871
This is now in the i915 tree so that branch is going to disappear:
https://cgit.freedesktop.org/drm/drm-intel/commit/?id=18fb42db05a0b93ab5dd5eab5315e50eaa3ca620
> You say no need to build your kernel with LTO...
> That sounds good.
> Currently, I build my kernels with Clang-14 and CONFIG_LTO_CLANG_THIN=y.
> Does something speak against using CONFIG_LTO_CLANG_THIN=y with KCFI support?
> Build-time?
> Disc-usage?
I wouldn't expect the build time or disk usage to significantly increase
with kCFI. I ran a quick benchmark with Arch Linux's configuration with
ThinLTO then ThinLTO + kCFI on an AMD EPYC 7513:
Benchmark 1: ThinLTO
Time (abs ≡): 166.036 s [User: 4687.951 s, System: 1636.767 s]
Benchmark 2: ThinLTO + kCFI
Time (abs ≡): 168.739 s [User: 4682.020 s, System: 1638.109 s]
Summary
'ThinLTO' ran
1.02 times faster than 'ThinLTO + kCFI'
Cheers,
Nathan
On Tue, May 17, 2022 at 1:58 AM Peter Zijlstra <[email protected]> wrote:
>
> On Fri, May 13, 2022 at 01:21:38PM -0700, Sami Tolvanen wrote:
> > KCFI is a proposed forward-edge control-flow integrity scheme for
> > Clang, which is more suitable for kernel use than the existing CFI
> > scheme used by CONFIG_CFI_CLANG. KCFI doesn't require LTO, doesn't
> > alter function references to point to a jump table, and won't break
> > function address equality. The latest LLVM patch is here:
> >
> > https://reviews.llvm.org/D119296
> >
> > This RFC series replaces the current arm64 CFI implementation with
> > KCFI and adds support for x86_64.
>
> You have some weird behaviour vs weak functions (I so hate those)...
>
> 100: 0000000000000980 9 FUNC LOCAL DEFAULT 2 __cfi_free_initmem
> 233: 0000000000000989 35 FUNC WEAK DEFAULT 2 free_initmem
>
> With the result that on the final link:
>
> 179: 00000000000009b0 9 FUNC LOCAL DEFAULT 1 __cfi_free_initmem
> 8689: 00000000000007f0 9 FUNC LOCAL DEFAULT 65 __cfi_free_initmem
> 173283: 00000000000007f9 198 FUNC GLOBAL DEFAULT 65 free_initmem
>
> This is getting me objtool issues (I'll fix them) but perhaps it's
> something you can do something about as well.
Good catch, I'll fix this.
Sami
On Tue, May 17, 2022 at 8:49 PM Nathan Chancellor <[email protected]> wrote:
>
> On Tue, May 17, 2022 at 09:33:47AM +0200, Sedat Dilek wrote:
> > Is your tags/kcfi-rfc-v2 the recommended LLVM toolchain for testing kcfi-rfc-v2?
> >
> > What I want to ask is if your commit well tested for x86 (and arm64)
> > means build and boot on bare metal?
>
> I have run kCFI (v1, I haven't had time to test v2) on x86_64 hardware,
> both AMD and Intel. I have only found two failures so far: the i915
> issue that I mention below and a failure in the KVM subsystem, which I
> can see by just running QEMU:
>
Is there a fix around this issue?
> [ 124.344654] CFI failure at kvm_mmu_notifier_invalidate_range_end+0xca/0x177 [kvm] (target: kvm_null_fn+0x0/0x7 [kvm]; expected type: 0x1e2c5b9c)
> [ 124.344691] WARNING: CPU: 5 PID: 2767 at kvm_mmu_notifier_invalidate_range_end+0xca/0x177 [kvm]
> [ 124.344708] Modules linked in: ...
> [ 124.344737] CPU: 5 PID: 2767 Comm: qemu-system-x86 Tainted: P 5.18.0-rc6-debug-00033-g1d5284aff7cd #1 8c4966c7fb24f3345076747feebac5fb340a4797
> [ 124.344738] Hardware name: ASUS System Product Name/PRIME Z590M-PLUS, BIOS 1203 10/27/2021
> [ 124.344738] RIP: 0010:kvm_mmu_notifier_invalidate_range_end+0xca/0x177 [kvm]
> [ 124.344756] Code: 48 89 da e8 88 16 86 dd 48 85 c0 89 6c 24 0c 74 30 4d 85 f6 74 36 48 c7 c5 19 c8 b4 c0 4c 8b 34 24 81 7d fa 9c 5b 2c 1e 74 02 <0f> 0b 48 89 c7 4c 89 fe 48 89 da e8 b6 16 86 dd 48 85 c0 75 e2 eb
> [ 124.344757] RSP: 0018:ffffc033039d78e8 EFLAGS: 00010282
> [ 124.344758] RAX: ffffa0a141ed9450 RBX: 00007fb897e9efff RCX: ffffa0a143c9d850
> [ 124.344758] RDX: 00007fb897e9efff RSI: 00007fb897e9e000 RDI: ffffc03304499cf8
> [ 124.344759] RBP: ffffffffc0b4c819 R08: 0000000000000000 R09: 0000000000000000
> [ 124.344759] R10: 0000000000000000 R11: ffffffffc0b4c639 R12: ffffc03304499000
> [ 124.344760] R13: ffffc033044a30a0 R14: ffffc033044a3120 R15: 00007fb897e9e000
> [ 124.344760] FS: 00007fbca4bc3640(0000) GS:ffffa0a87f540000(0000) knlGS:0000000000000000
> [ 124.344761] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 124.344761] CR2: 0000000000000000 CR3: 0000000119f74006 CR4: 0000000000772ee0
> [ 124.344762] PKRU: 55555554
> [ 124.344762] Call Trace:
> [ 124.344762] <TASK>
> [ 124.344763] __mmu_notifier_invalidate_range_end+0xa1/0xd7
> [ 124.344764] wp_page_copy+0x592/0x920
> [ 124.344766] __handle_mm_fault+0x820/0x8f0
> [ 124.344767] handle_mm_fault+0xe0/0x227
> [ 124.344768] __get_user_pages+0x17a/0x430
> [ 124.344769] get_user_pages_unlocked+0xd9/0x327
> [ 124.344770] hva_to_pfn+0xfa/0x3f7 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> [ 124.344788] kvm_faultin_pfn+0xc3/0x2f0 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> [ 124.344806] ? fast_page_fault+0x400/0x4c0 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> [ 124.344823] direct_page_fault+0x130/0x350 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> [ 124.344841] kvm_mmu_page_fault+0x176/0x2f7 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> [ 124.344858] vmx_handle_exit+0xe/0x37 [kvm_intel 647f514f18fc365045dc57db46d2bb4930b746eb]
> [ 124.344863] vcpu_enter_guest+0xbff/0x1030 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> [ 124.344881] vcpu_run+0x65/0x2f0 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> [ 124.344898] kvm_arch_vcpu_ioctl_run+0x15f/0x3f7 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> [ 124.344916] kvm_vcpu_ioctl+0x547/0x627 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> [ 124.344933] ? syscall_exit_to_user_mode+0x24/0x47
> [ 124.344934] ? do_syscall_64+0x7a/0x97
> [ 124.344935] ? __fget_files+0xa1/0xc0
> [ 124.344936] __se_sys_ioctl+0x7c/0xc0
> [ 124.344937] do_syscall_64+0x6c/0x97
> [ 124.344938] ? do_syscall_64+0x7a/0x97
> [ 124.344939] entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 124.344940] RIP: 0033:0x7fbca75f7b1f
> [ 124.344940] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 18 48 8b 44 24 18 64 48 2b 04 25 28 00 00
> [ 124.344941] RSP: 002b:00007fbca4bc2550 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [ 124.344942] RAX: ffffffffffffffda RBX: 000000000000ae80 RCX: 00007fbca75f7b1f
> [ 124.344942] RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 000000000000000e
> [ 124.344943] RBP: 000056351ba661d0 R08: 000056351a74dc48 R09: 0000000000000004
> [ 124.344943] R10: 004818cf4f129e11 R11: 0000000000000246 R12: 0000000000000000
> [ 124.344943] R13: 0000000000000000 R14: 00007fbca814f004 R15: 0000000000000000
> [ 124.344944] </TASK>
> [ 124.344945] ---[ end trace 0000000000000000 ]---
>
> I don't do any bare metal testing for the tc-build known good revision
> bumps, just QEMU tests with boot-utils.
>
> > Just for the records:
> > You definitely need a pre-LLVM-15 toolchain + KCFI sanitizer patch?
> > LLVM-14?
>
> The feature won't land in LLVM 14, so there is little point to
> backporting and testing it on LLVM 14. This is a work in progress
> feature so it has to target the main branch. I have fairly good coverage
> of tip of tree locally and we have solid coverage through CI so we'll
> know if something major happens, it should be pretty safe to test Sami's
> series.
>
OK, no backport.
> > > > Nathan has a i915 cfi patch in His personal kernel.org Git.
> > > > Is this relevant to kcfi?
> > >
> > > It fixes a type mismatch, so in that sense it's relevant.
> > >
> >
> > Here the link to patch "drm/i915: Fix CFI violation with show_dynamic_id()":
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/nathan/linux.git/commit/?h=submitted/i915-cfi-fix&id=53735be6dc53453fcfbac658e847b54360e73871
>
> This is now in the i915 tree so that branch is going to disappear:
>
> https://cgit.freedesktop.org/drm/drm-intel/commit/?id=18fb42db05a0b93ab5dd5eab5315e50eaa3ca620
>
Good to know.
> > You say no need to build your kernel with LTO...
> > That sounds good.
> > Currently, I build my kernels with Clang-14 and CONFIG_LTO_CLANG_THIN=y.
> > Does something speak against using CONFIG_LTO_CLANG_THIN=y with KCFI support?
> > Build-time?
> > Disc-usage?
>
> I wouldn't expect the build time or disk usage to significantly increase
> with kCFI. I ran a quick benchmark with Arch Linux's configuration with
> ThinLTO then ThinLTO + kCFI on an AMD EPYC 7513:
>
> Benchmark 1: ThinLTO
> Time (abs ≡): 166.036 s [User: 4687.951 s, System: 1636.767 s]
>
> Benchmark 2: ThinLTO + kCFI
> Time (abs ≡): 168.739 s [User: 4682.020 s, System: 1638.109 s]
>
> Summary
> 'ThinLTO' ran
> 1.02 times faster than 'ThinLTO + kCFI'
>
Thanks for your feedback.
-Sedat-
On Thu, May 19, 2022 at 11:01:40AM +0200, Sedat Dilek wrote:
> On Tue, May 17, 2022 at 8:49 PM Nathan Chancellor <[email protected]> wrote:
> > I have run kCFI (v1, I haven't had time to test v2) on x86_64 hardware,
> > both AMD and Intel. I have only found two failures so far: the i915
> > issue that I mention below and a failure in the KVM subsystem, which I
> > can see by just running QEMU:
> >
>
> Is there a fix around this issue?
No, I mentioned it offhand to Sami in IRC but I never followed up. This
failure appears to be introduced by commit f922bd9bf33b ("KVM: Move MMU
notifier's mmu_lock acquisition into common helper").
Sami, do you want an issue opened around this somewhere?
> > [ 124.344654] CFI failure at kvm_mmu_notifier_invalidate_range_end+0xca/0x177 [kvm] (target: kvm_null_fn+0x0/0x7 [kvm]; expected type: 0x1e2c5b9c)
> > [ 124.344691] WARNING: CPU: 5 PID: 2767 at kvm_mmu_notifier_invalidate_range_end+0xca/0x177 [kvm]
> > [ 124.344708] Modules linked in: ...
> > [ 124.344737] CPU: 5 PID: 2767 Comm: qemu-system-x86 Tainted: P 5.18.0-rc6-debug-00033-g1d5284aff7cd #1 8c4966c7fb24f3345076747feebac5fb340a4797
> > [ 124.344738] Hardware name: ASUS System Product Name/PRIME Z590M-PLUS, BIOS 1203 10/27/2021
> > [ 124.344738] RIP: 0010:kvm_mmu_notifier_invalidate_range_end+0xca/0x177 [kvm]
> > [ 124.344756] Code: 48 89 da e8 88 16 86 dd 48 85 c0 89 6c 24 0c 74 30 4d 85 f6 74 36 48 c7 c5 19 c8 b4 c0 4c 8b 34 24 81 7d fa 9c 5b 2c 1e 74 02 <0f> 0b 48 89 c7 4c 89 fe 48 89 da e8 b6 16 86 dd 48 85 c0 75 e2 eb
> > [ 124.344757] RSP: 0018:ffffc033039d78e8 EFLAGS: 00010282
> > [ 124.344758] RAX: ffffa0a141ed9450 RBX: 00007fb897e9efff RCX: ffffa0a143c9d850
> > [ 124.344758] RDX: 00007fb897e9efff RSI: 00007fb897e9e000 RDI: ffffc03304499cf8
> > [ 124.344759] RBP: ffffffffc0b4c819 R08: 0000000000000000 R09: 0000000000000000
> > [ 124.344759] R10: 0000000000000000 R11: ffffffffc0b4c639 R12: ffffc03304499000
> > [ 124.344760] R13: ffffc033044a30a0 R14: ffffc033044a3120 R15: 00007fb897e9e000
> > [ 124.344760] FS: 00007fbca4bc3640(0000) GS:ffffa0a87f540000(0000) knlGS:0000000000000000
> > [ 124.344761] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 124.344761] CR2: 0000000000000000 CR3: 0000000119f74006 CR4: 0000000000772ee0
> > [ 124.344762] PKRU: 55555554
> > [ 124.344762] Call Trace:
> > [ 124.344762] <TASK>
> > [ 124.344763] __mmu_notifier_invalidate_range_end+0xa1/0xd7
> > [ 124.344764] wp_page_copy+0x592/0x920
> > [ 124.344766] __handle_mm_fault+0x820/0x8f0
> > [ 124.344767] handle_mm_fault+0xe0/0x227
> > [ 124.344768] __get_user_pages+0x17a/0x430
> > [ 124.344769] get_user_pages_unlocked+0xd9/0x327
> > [ 124.344770] hva_to_pfn+0xfa/0x3f7 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> > [ 124.344788] kvm_faultin_pfn+0xc3/0x2f0 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> > [ 124.344806] ? fast_page_fault+0x400/0x4c0 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> > [ 124.344823] direct_page_fault+0x130/0x350 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> > [ 124.344841] kvm_mmu_page_fault+0x176/0x2f7 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> > [ 124.344858] vmx_handle_exit+0xe/0x37 [kvm_intel 647f514f18fc365045dc57db46d2bb4930b746eb]
> > [ 124.344863] vcpu_enter_guest+0xbff/0x1030 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> > [ 124.344881] vcpu_run+0x65/0x2f0 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> > [ 124.344898] kvm_arch_vcpu_ioctl_run+0x15f/0x3f7 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> > [ 124.344916] kvm_vcpu_ioctl+0x547/0x627 [kvm 950e483805e01f967a3e331506278d766494a4cc]
> > [ 124.344933] ? syscall_exit_to_user_mode+0x24/0x47
> > [ 124.344934] ? do_syscall_64+0x7a/0x97
> > [ 124.344935] ? __fget_files+0xa1/0xc0
> > [ 124.344936] __se_sys_ioctl+0x7c/0xc0
> > [ 124.344937] do_syscall_64+0x6c/0x97
> > [ 124.344938] ? do_syscall_64+0x7a/0x97
> > [ 124.344939] entry_SYSCALL_64_after_hwframe+0x44/0xae
> > [ 124.344940] RIP: 0033:0x7fbca75f7b1f
> > [ 124.344940] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 18 48 8b 44 24 18 64 48 2b 04 25 28 00 00
> > [ 124.344941] RSP: 002b:00007fbca4bc2550 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > [ 124.344942] RAX: ffffffffffffffda RBX: 000000000000ae80 RCX: 00007fbca75f7b1f
> > [ 124.344942] RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 000000000000000e
> > [ 124.344943] RBP: 000056351ba661d0 R08: 000056351a74dc48 R09: 0000000000000004
> > [ 124.344943] R10: 004818cf4f129e11 R11: 0000000000000246 R12: 0000000000000000
> > [ 124.344943] R13: 0000000000000000 R14: 00007fbca814f004 R15: 0000000000000000
> > [ 124.344944] </TASK>
> > [ 124.344945] ---[ end trace 0000000000000000 ]---
Cheers,
Nathan
On Thu, May 19, 2022 at 1:26 PM Nathan Chancellor <[email protected]> wrote:
>
> On Thu, May 19, 2022 at 11:01:40AM +0200, Sedat Dilek wrote:
> > On Tue, May 17, 2022 at 8:49 PM Nathan Chancellor <[email protected]> wrote:
> > > I have run kCFI (v1, I haven't had time to test v2) on x86_64 hardware,
> > > both AMD and Intel. I have only found two failures so far: the i915
> > > issue that I mention below and a failure in the KVM subsystem, which I
> > > can see by just running QEMU:
> > >
> >
> > Is there a fix around this issue?
>
> No, I mentioned it offhand to Sami in IRC but I never followed up. This
> failure appears to be introduced by commit f922bd9bf33b ("KVM: Move MMU
> notifier's mmu_lock acquisition into common helper").
>
> Sami, do you want an issue opened around this somewhere?
Yes, please file a bug. The ClangBuiltLinux Github is probably the best place.
Sami