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 6 adds helper macros for
annotating functions, and patches 8 and 18 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 9 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-v3
This series is also available in GitHub:
https://github.com/samitolvanen/linux/commits/kcfi-rfc-v3
---
Changes in v3:
- Merged the patches that split CC_FLAGS_CFI from CC_FLAGS_LTO.
- Dropped the psci_initcall_t patch as Mark volunteered to send a
patch for this. Note that this patch is still needed to boot a
CFI kernel on certain arm64 systems:
https://lore.kernel.org/lkml/YoNhKaTT3EDukxXY@FVFF77S0Q05N/
- Added a patch to remove the now unnecessary workarounds with
CFI+ThinLTO in kallsyms.
- Added an lkdtm patch to ensure the test actually generates an
indirect call.
- Changed report_cfi_failure to clearly indicate if we failed to
decode target address.
- Switched to relative offsets for .kcfi_traps.
- On x86_64, moved CFI error handling from traps.c to cfi.c, and
as we only call memcpy indirectly w/ CONFIG_MODULES, ensured that
the compiler emits __kcfi_typeid_memcpy also without modules.
- On x86_64, added a check for the cmpl REX prefix to handle the
case where the compiler might not use r8-r15 registers for the
call target.
- On the compiler side, ensured that on x86_64 calls are emitted
immediately after the CFI check, fixed the __cfi_ preamble
linkage, and changed the compiler to emit relative offsets in
.kcfi_traps.
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.
Sami Tolvanen (20):
treewide: Filter out CC_FLAGS_CFI
scripts/kallsyms: Ignore __kcfi_typeid_
cfi: Remove CONFIG_CFI_CLANG_SHADOW
cfi: Drop __CFI_ADDRESSABLE
cfi: Switch to -fsanitize=kcfi
cfi: Add type helper macros
lkdtm: Emit an indirect call for CFI tests
arm64: Add types to indirect called assembly functions
arm64: Add CFI error handling
arm64: Drop unneeded __nocfi attributes
init: Drop __nocfi from __init
treewide: Drop function_nocfi
treewide: Drop WARN_ON_FUNCTION_MISMATCH
treewide: Drop __cficanonical
objtool: Disable CFI warnings
kallsyms: Drop CONFIG_CFI_CLANG workarounds
x86/tools/relocs: Ignore __kcfi_typeid_ relocations
x86: Add types to indirectly called assembly functions
x86/purgatory: Disable CFI
x86: Add support for CONFIG_CFI_CLANG
Makefile | 13 +-
arch/Kconfig | 18 +-
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/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 | 47 ++-
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/cfi.h | 22 ++
arch/x86/include/asm/linkage.h | 12 +
arch/x86/kernel/Makefile | 2 +
arch/x86/kernel/cfi.c | 83 ++++++
arch/x86/kernel/traps.c | 4 +-
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 | 4 +-
drivers/misc/lkdtm/cfi.c | 15 +-
drivers/misc/lkdtm/usercopy.c | 2 +-
include/asm-generic/bug.h | 16 -
include/asm-generic/vmlinux.lds.h | 37 +--
include/linux/cfi.h | 59 ++--
include/linux/cfi_types.h | 57 ++++
include/linux/compiler-clang.h | 14 +-
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 | 342 ++++------------------
kernel/kallsyms.c | 17 --
kernel/kthread.c | 3 +-
kernel/module/main.c | 49 +---
kernel/workqueue.c | 2 +-
scripts/kallsyms.c | 1 +
scripts/module.lds.S | 23 +-
tools/objtool/check.c | 7 +-
51 files changed, 420 insertions(+), 538 deletions(-)
create mode 100644 arch/x86/include/asm/cfi.h
create mode 100644 arch/x86/kernel/cfi.c
create mode 100644 include/linux/cfi_types.h
--
2.36.1.476.g0c4daa206d-goog
With -fsanitize=kcfi, we no longer need function_nocfi() as
the compiler won't change function references to point to a
jump table. Remove all implementations and uses of the macro.
Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
arch/arm64/include/asm/ftrace.h | 2 +-
arch/arm64/include/asm/mmu_context.h | 2 +-
arch/arm64/kernel/acpi_parking_protocol.c | 2 +-
arch/arm64/kernel/cpufeature.c | 2 +-
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 +-
drivers/firmware/psci/psci.c | 4 ++--
drivers/misc/lkdtm/usercopy.c | 2 +-
include/linux/compiler.h | 10 ----------
11 files changed, 11 insertions(+), 21 deletions(-)
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index dbc45a4157fa..329dbbd4d50b 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -26,7 +26,7 @@
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
#define ARCH_SUPPORTS_FTRACE_OPS 1
#else
-#define MCOUNT_ADDR ((unsigned long)function_nocfi(_mcount))
+#define MCOUNT_ADDR ((unsigned long)_mcount)
#endif
/* The BL at the callsite's adjusted rec->ip */
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index ca0140d0b8cf..8fa4cfbdda90 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -164,7 +164,7 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp)
ttbr1 |= TTBR_CNP_BIT;
}
- replace_phys = (void *)__pa_symbol(function_nocfi(idmap_cpu_replace_ttbr1));
+ replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1);
cpu_install_idmap();
replace_phys(ttbr1);
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
index bfeeb5319abf..b1990e38aed0 100644
--- a/arch/arm64/kernel/acpi_parking_protocol.c
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -99,7 +99,7 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
* that read this address need to convert this address to the
* Boot-Loader's endianness before jumping.
*/
- writeq_relaxed(__pa_symbol(function_nocfi(secondary_entry)),
+ writeq_relaxed(__pa_symbol(secondary_entry),
&mailbox->entry_point);
writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index f6f9694d0448..6cd476f0d19c 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1668,7 +1668,7 @@ kpti_install_ng_mappings(const struct arm64_cpu_capabilities *__unused)
if (arm64_use_ng_mappings)
return;
- remap_fn = (void *)__pa_symbol(function_nocfi(idmap_kpti_install_ng_mappings));
+ remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
cpu_install_idmap();
remap_fn(cpu, num_online_cpus(), __pa_symbol(swapper_pg_dir));
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index f447c4a36f69..74c8ab01dd8c 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -56,7 +56,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
unsigned long pc;
u32 new;
- pc = (unsigned long)function_nocfi(ftrace_call);
+ pc = (unsigned long)ftrace_call;
new = aarch64_insn_gen_branch_imm(pc, (unsigned long)func,
AARCH64_INSN_BRANCH_LINK);
diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
index 19c2d487cb08..ce3d40120f72 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -204,7 +204,7 @@ void machine_kexec(struct kimage *kimage)
typeof(cpu_soft_restart) *restart;
cpu_install_idmap();
- restart = (void *)__pa_symbol(function_nocfi(cpu_soft_restart));
+ restart = (void *)__pa_symbol(cpu_soft_restart);
restart(is_hyp_nvhe(), kimage->start, kimage->arch.dtb_mem,
0, 0);
} else {
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index ab7f4c476104..29a8e444db83 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -38,7 +38,7 @@ static int __init cpu_psci_cpu_prepare(unsigned int cpu)
static int cpu_psci_cpu_boot(unsigned int cpu)
{
- phys_addr_t pa_secondary_entry = __pa_symbol(function_nocfi(secondary_entry));
+ phys_addr_t pa_secondary_entry = __pa_symbol(secondary_entry);
int err = psci_ops.cpu_on(cpu_logical_map(cpu), pa_secondary_entry);
if (err)
pr_err("failed to boot CPU%d (%d)\n", cpu, err);
diff --git a/arch/arm64/kernel/smp_spin_table.c b/arch/arm64/kernel/smp_spin_table.c
index 7e1624ecab3c..49029eace3ad 100644
--- a/arch/arm64/kernel/smp_spin_table.c
+++ b/arch/arm64/kernel/smp_spin_table.c
@@ -66,7 +66,7 @@ static int smp_spin_table_cpu_init(unsigned int cpu)
static int smp_spin_table_cpu_prepare(unsigned int cpu)
{
__le64 __iomem *release_addr;
- phys_addr_t pa_holding_pen = __pa_symbol(function_nocfi(secondary_holding_pen));
+ phys_addr_t pa_holding_pen = __pa_symbol(secondary_holding_pen);
if (!cpu_release_addr[cpu])
return -ENODEV;
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index cfb448eabdaa..aa3133cafced 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -334,7 +334,7 @@ static int __init psci_features(u32 psci_func_id)
static int psci_suspend_finisher(unsigned long state)
{
u32 power_state = state;
- phys_addr_t pa_cpu_resume = __pa_symbol(function_nocfi(cpu_resume));
+ phys_addr_t pa_cpu_resume = __pa_symbol(cpu_resume);
return psci_ops.cpu_suspend(power_state, pa_cpu_resume);
}
@@ -359,7 +359,7 @@ int psci_cpu_suspend_enter(u32 state)
static int psci_system_suspend(unsigned long unused)
{
- phys_addr_t pa_cpu_resume = __pa_symbol(function_nocfi(cpu_resume));
+ phys_addr_t pa_cpu_resume = __pa_symbol(cpu_resume);
return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
pa_cpu_resume, 0, 0);
diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c
index 6215ec995cd3..67db57249a34 100644
--- a/drivers/misc/lkdtm/usercopy.c
+++ b/drivers/misc/lkdtm/usercopy.c
@@ -330,7 +330,7 @@ static void lkdtm_USERCOPY_KERNEL(void)
pr_info("attempting bad copy_to_user from kernel text: %px\n",
vm_mmap);
- if (copy_to_user((void __user *)user_addr, function_nocfi(vm_mmap),
+ if (copy_to_user((void __user *)user_addr, vm_mmap,
unconst + PAGE_SIZE)) {
pr_warn("copy_to_user failed, but lacked Oops\n");
goto free_user;
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 63be1c23d676..76f5e41ea725 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -203,16 +203,6 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
__v; \
})
-/*
- * With CONFIG_CFI_CLANG, the compiler replaces function addresses in
- * instrumented C code with jump table addresses. Architectures that
- * support CFI can define this macro to return the actual function address
- * when needed.
- */
-#ifndef function_nocfi
-#define function_nocfi(x) (x)
-#endif
-
#endif /* __KERNEL__ */
/*
--
2.36.1.476.g0c4daa206d-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.
Note that the Clang definition of the attribute was removed earlier,
just clean up the no-op definition and users.
Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[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 d08dfcb0ac68..2957edd29252 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 88f2964097f5..a0a90cd73ebe 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 81a57b498f22..c36c52933c8c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2029,8 +2029,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.1.476.g0c4daa206d-goog
In preparation for removing CC_FLAGS_CFI from CC_FLAGS_LTO, explicitly
filter out CC_FLAGS_CFI in all the makefiles where we currently filter
out CC_FLAGS_LTO.
Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
arch/arm64/kernel/vdso/Makefile | 3 ++-
arch/x86/entry/vdso/Makefile | 3 ++-
drivers/firmware/efi/libstub/Makefile | 2 ++
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index f6e25d7c346a..c7c123ed29cc 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -34,7 +34,8 @@ ccflags-y += -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
# kernel with CONFIG_WERROR enabled.
CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os $(CC_FLAGS_SCS) \
$(RANDSTRUCT_CFLAGS) $(GCC_PLUGINS_CFLAGS) \
- $(CC_FLAGS_LTO) -Wmissing-prototypes -Wmissing-declarations
+ $(CC_FLAGS_LTO) $(CC_FLAGS_CFI) \
+ -Wmissing-prototypes -Wmissing-declarations
KASAN_SANITIZE := n
KCSAN_SANITIZE := n
UBSAN_SANITIZE := n
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index c2a8b76ae0bc..0148df4f0425 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -91,7 +91,7 @@ ifneq ($(RETPOLINE_VDSO_CFLAGS),)
endif
endif
-$(vobjs): KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO) $(RANDSTRUCT_CFLAGS) $(GCC_PLUGINS_CFLAGS) $(RETPOLINE_CFLAGS),$(KBUILD_CFLAGS)) $(CFL)
+$(vobjs): KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO) $(CC_FLAGS_CFI) $(RANDSTRUCT_CFLAGS) $(GCC_PLUGINS_CFLAGS) $(RETPOLINE_CFLAGS),$(KBUILD_CFLAGS)) $(CFL)
#
# vDSO code runs in userspace and -pg doesn't help with profiling anyway.
@@ -152,6 +152,7 @@ KBUILD_CFLAGS_32 := $(filter-out $(RANDSTRUCT_CFLAGS),$(KBUILD_CFLAGS_32))
KBUILD_CFLAGS_32 := $(filter-out $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS_32))
KBUILD_CFLAGS_32 := $(filter-out $(RETPOLINE_CFLAGS),$(KBUILD_CFLAGS_32))
KBUILD_CFLAGS_32 := $(filter-out $(CC_FLAGS_LTO),$(KBUILD_CFLAGS_32))
+KBUILD_CFLAGS_32 := $(filter-out $(CC_FLAGS_CFI),$(KBUILD_CFLAGS_32))
KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=0 -fpic
KBUILD_CFLAGS_32 += -fno-stack-protector
KBUILD_CFLAGS_32 += $(call cc-option, -foptimize-sibling-calls)
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.1.476.g0c4daa206d-goog
With CONFIG_CFI_CLANG, assembly functions indirectly called from C code
must be annotated with type identifiers to pass CFI checking. Add types
to indirectly called functions.
Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
arch/x86/crypto/blowfish-x86_64-asm_64.S | 5 +++--
arch/x86/lib/memcpy_64.S | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/crypto/blowfish-x86_64-asm_64.S b/arch/x86/crypto/blowfish-x86_64-asm_64.S
index 802d71582689..4a43e072d2d1 100644
--- a/arch/x86/crypto/blowfish-x86_64-asm_64.S
+++ b/arch/x86/crypto/blowfish-x86_64-asm_64.S
@@ -6,6 +6,7 @@
*/
#include <linux/linkage.h>
+#include <linux/cfi_types.h>
.file "blowfish-x86_64-asm.S"
.text
@@ -141,7 +142,7 @@ SYM_FUNC_START(__blowfish_enc_blk)
RET;
SYM_FUNC_END(__blowfish_enc_blk)
-SYM_FUNC_START(blowfish_dec_blk)
+SYM_TYPED_FUNC_START(blowfish_dec_blk)
/* input:
* %rdi: ctx
* %rsi: dst
@@ -332,7 +333,7 @@ SYM_FUNC_START(__blowfish_enc_blk_4way)
RET;
SYM_FUNC_END(__blowfish_enc_blk_4way)
-SYM_FUNC_START(blowfish_dec_blk_4way)
+SYM_TYPED_FUNC_START(blowfish_dec_blk_4way)
/* input:
* %rdi: ctx
* %rsi: dst
diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
index d0d7b9bc6cad..e5d9b299577f 100644
--- a/arch/x86/lib/memcpy_64.S
+++ b/arch/x86/lib/memcpy_64.S
@@ -2,6 +2,7 @@
/* Copyright 2002 Andi Kleen */
#include <linux/linkage.h>
+#include <linux/cfi_types.h>
#include <asm/errno.h>
#include <asm/cpufeatures.h>
#include <asm/alternative.h>
@@ -27,7 +28,7 @@
* Output:
* rax original destination
*/
-SYM_FUNC_START(__memcpy)
+__SYM_TYPED_FUNC_START(__memcpy, memcpy)
ALTERNATIVE_2 "jmp memcpy_orig", "", X86_FEATURE_REP_GOOD, \
"jmp memcpy_erms", X86_FEATURE_ERMS
--
2.36.1.476.g0c4daa206d-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
This produces the following oops on CFI failure (generated using lkdtm):
[ 21.885179] CFI failure at lkdtm_indirect_call+0x2c/0x44 [lkdtm]
(target: lkdtm_increment_int+0x0/0x1c [lkdtm]; expected type: 0x7e0c52a)
[ 21.886593] Internal error: Oops - CFI: 0 [#1] PREEMPT SMP
[ 21.891060] Modules linked in: lkdtm
[ 21.893363] CPU: 0 PID: 151 Comm: sh Not tainted
5.19.0-rc1-00021-g852f4e48dbab #1
[ 21.895560] Hardware name: linux,dummy-virt (DT)
[ 21.896543] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 21.897583] pc : lkdtm_indirect_call+0x2c/0x44 [lkdtm]
[ 21.898551] lr : lkdtm_CFI_FORWARD_PROTO+0x3c/0x6c [lkdtm]
[ 21.899520] sp : ffff8000083a3c50
[ 21.900191] x29: ffff8000083a3c50 x28: ffff0000027e0ec0 x27: 0000000000000000
[ 21.902453] x26: 0000000000000000 x25: ffffc2aa3d07e7b0 x24: 0000000000000002
[ 21.903736] x23: ffffc2aa3d079088 x22: ffffc2aa3d07e7b0 x21: ffff000003379000
[ 21.905062] x20: ffff8000083a3dc0 x19: 0000000000000012 x18: 0000000000000000
[ 21.906371] x17: 000000007e0c52a5 x16: 000000003ad55aca x15: ffffc2aa60d92138
[ 21.907662] x14: ffffffffffffffff x13: 2e2e2e2065707974 x12: 0000000000000018
[ 21.909775] x11: ffffc2aa62322b88 x10: ffffc2aa62322aa0 x9 : c7e305fb5195d200
[ 21.911898] x8 : ffffc2aa3d077e20 x7 : 6d20676e696c6c61 x6 : 43203a6d74646b6c
[ 21.913108] x5 : ffffc2aa6266c9df x4 : ffffc2aa6266c9e1 x3 : ffff8000083a3968
[ 21.914358] x2 : 80000000fffff122 x1 : 00000000fffff122 x0 : ffffc2aa3d07e8f8
[ 21.915827] Call trace:
[ 21.916375] lkdtm_indirect_call+0x2c/0x44 [lkdtm]
[ 21.918060] lkdtm_CFI_FORWARD_PROTO+0x3c/0x6c [lkdtm]
[ 21.919030] lkdtm_do_action+0x34/0x4c [lkdtm]
[ 21.919920] direct_entry+0x170/0x1ac [lkdtm]
[ 21.920772] full_proxy_write+0x84/0x104
[ 21.921759] vfs_write+0x188/0x3d8
[ 21.922387] ksys_write+0x78/0xe8
[ 21.922986] __arm64_sys_write+0x1c/0x2c
[ 21.923696] invoke_syscall+0x58/0x134
[ 21.924554] el0_svc_common+0xb4/0xf4
[ 21.925603] do_el0_svc+0x2c/0xb4
[ 21.926563] el0_svc+0x2c/0x7c
[ 21.927147] el0t_64_sync_handler+0x84/0xf0
[ 21.927985] el0t_64_sync+0x18c/0x190
[ 21.929133] Code: 728a54b1 72afc191 6b11021f 54000040 (d4304500)
[ 21.930690] ---[ end trace 0000000000000000 ]---
[ 21.930971] Kernel panic - not syncing: Oops - CFI: Fatal exception
Suggested-by: Mark Rutland <[email protected]>
Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
arch/arm64/include/asm/brk-imm.h | 6 ++++
arch/arm64/kernel/traps.c | 47 ++++++++++++++++++++++++++++++--
2 files changed, 50 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 9ac7a81b79be..7547d3abf0f5 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>
@@ -991,6 +992,38 @@ static struct break_hook bug_break_hook = {
.imm = BUG_BRK_IMM,
};
+#ifdef CONFIG_CFI_CLANG
+static int cfi_handler(struct pt_regs *regs, unsigned long esr)
+{
+ unsigned long target;
+ u32 type;
+
+ target = pt_regs_read_reg(regs, FIELD_GET(CFI_BRK_IMM_TARGET, esr));
+ type = (u32)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 long esr)
{
pr_err("%s generated an invalid instruction at %pS!\n",
@@ -1052,6 +1085,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().
@@ -1059,10 +1095,12 @@ static struct break_hook kasan_break_hook = {
int __init early_brk64(unsigned long addr, unsigned long 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 long 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;
@@ -1071,6 +1109,9 @@ int __init early_brk64(unsigned long addr, unsigned long 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.1.476.g0c4daa206d-goog
CONFIG_CFI_CLANG no longer breaks cross-module function address
equality, which makes WARN_ON_FUNCTION_MISMATCH unnecessary. Remove
the definition and switch back to WARN_ON_ONCE.
Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
include/asm-generic/bug.h | 16 ----------------
kernel/kthread.c | 3 +--
kernel/workqueue.c | 2 +-
3 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index ba1f860af38b..4050b191e1a9 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -220,22 +220,6 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
# define WARN_ON_SMP(x) ({0;})
#endif
-/*
- * WARN_ON_FUNCTION_MISMATCH() warns if a value doesn't match a
- * function address, and can be useful for catching issues with
- * callback functions, for example.
- *
- * With CONFIG_CFI_CLANG, the warning is disabled because the
- * compiler replaces function addresses taken in C code with
- * local jump table addresses, which breaks cross-module function
- * address equality.
- */
-#if defined(CONFIG_CFI_CLANG) && defined(CONFIG_MODULES)
-# define WARN_ON_FUNCTION_MISMATCH(x, fn) ({ 0; })
-#else
-# define WARN_ON_FUNCTION_MISMATCH(x, fn) WARN_ON_ONCE((x) != (fn))
-#endif
-
#endif /* __ASSEMBLY__ */
#endif
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 544fd4097406..19d446f4e3a9 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -1050,8 +1050,7 @@ static void __kthread_queue_delayed_work(struct kthread_worker *worker,
struct timer_list *timer = &dwork->timer;
struct kthread_work *work = &dwork->work;
- WARN_ON_FUNCTION_MISMATCH(timer->function,
- kthread_delayed_work_timer_fn);
+ WARN_ON_ONCE(timer->function != kthread_delayed_work_timer_fn);
/*
* If @delay is 0, queue @dwork->work immediately. This is for
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 4056f2a3f9d5..2fed7bb018a7 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1651,7 +1651,7 @@ static void __queue_delayed_work(int cpu, struct workqueue_struct *wq,
struct work_struct *work = &dwork->work;
WARN_ON_ONCE(!wq);
- WARN_ON_FUNCTION_MISMATCH(timer->function, delayed_work_timer_fn);
+ WARN_ON_ONCE(timer->function != delayed_work_timer_fn);
WARN_ON_ONCE(timer_pending(timer));
WARN_ON_ONCE(!list_empty(&work->entry));
--
2.36.1.476.g0c4daa206d-goog
The compiler generates __kcfi_typeid_ symbols for annotating assembly
functions with type information. These are constants that can be
referenced in assembly code and are resolved by the linker. Ignore
them in kallsyms.
Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
scripts/kallsyms.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index f18e6dfc68c5..ccdf0c897f31 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -119,6 +119,7 @@ static bool is_ignored_symbol(const char *name, char type)
"__ThumbV7PILongThunk_",
"__LA25Thunk_", /* mips lld */
"__microLA25Thunk_",
+ "__kcfi_typeid_", /* CFI type identifiers */
NULL
};
--
2.36.1.476.g0c4daa206d-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]>
Reviewed-by: Kees Cook <[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 01ce94b58b42..63be1c23d676 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 abd9fa916b7d..efecd65a976d 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.1.476.g0c4daa206d-goog
The __cfi_ preambles contain int3 padding and a mov instruction that
embeds the KCFI type identifier in the following format:
; type preamble
__cfi_function:
int3
int3
mov <id>, %eax
int3
int3
function:
...
While the preamble symbols are STT_FUNC and contain valid
instructions, they are never executed and always fall through. Skip
the warning for them.
.kcfi_traps sections point to CFI traps in text sections. Also skip
the warning about them referencing !ENDBR instructions.
Signed-off-by: Sami Tolvanen <[email protected]>
---
tools/objtool/check.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 864bb9dd3584..337b92c3b755 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3207,6 +3207,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;
@@ -3827,7 +3831,8 @@ static int validate_ibt(struct objtool_file *file)
!strcmp(sec->name, "__ex_table") ||
!strcmp(sec->name, "__jump_table") ||
!strcmp(sec->name, "__mcount_loc") ||
- !strcmp(sec->name, "__tracepoints"))
+ !strcmp(sec->name, "__tracepoints") ||
+ !strcmp(sec->name, ".kcfi_traps"))
continue;
list_for_each_entry(reloc, &sec->reloc->reloc_list, list)
--
2.36.1.476.g0c4daa206d-goog
Disable CONFIG_CFI_CLANG for the stand-alone purgatory.ro.
Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Reviewed-by: Kees Cook <[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.1.476.g0c4daa206d-goog
With CONFIG_CFI_CLANG, the compiler injects a type preamble immediately
before each function and a check to validate the target function type
before indirect calls:
; type preamble
__cfi_function:
int3
int3
mov <id>, %eax
int3
int3
function:
...
; indirect call check
cmpl <id>, -6(%r11)
je .Ltmp1
ud2
.Ltmp1:
call __x86_indirect_thunk_r11
Define the __CFI_TYPE helper macro for manually annotating indirectly
called assembly function with the identical premable, add error handling
code for the ud2 traps emitted for indirect call checks, and allow
CONFIG_CFI_CLANG to be selected on x86_64.
This produces the following oops on CFI failure (generated using lkdtm):
[ 15.896503] CFI failure at lkdtm_indirect_call+0x14/0x20 [lkdtm]
(target: lkdtm_increment_int+0x0/0x7 [lkdtm]; expected type: 0x7e0c52a5)
[ 15.898565] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[ 15.898798] CPU: 2 PID: 133 Comm: sh Not tainted
5.19.0-rc1-00020-g524d4b861d15 #1
[ 15.898967] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[ 15.899004] RIP: 0010:lkdtm_indirect_call+0x14/0x20 [lkdtm]
[ 15.899004] Code: c7 c2 76 02 40 c0 48 c7 c1 95 00 40 c0 e9 3b cb 3e
e8 0f 1f 40 00 49 89 fb 48 c7 c7 70 64 40 c0 41 81 7b fa a5 52 0c 7f
[ 15.899004] RSP: 0018:ffff9d928029fdc0 EFLAGS: 00000283
[ 15.899004] RAX: 0000000000000027 RBX: ffffffffc0406320 RCX: 024cef129f458500
[ 15.899004] RDX: ffffffffa9251580 RSI: ffffffffa90736c8 RDI: ffffffffc0406470
[ 15.899004] RBP: 0000000000000006 R08: ffffffffa9251670 R09: 65686374616d7369
[ 15.899004] R10: 000000002e2e2e20 R11: ffffffffc03fdc69 R12: 0000000000000000
[ 15.899004] R13: ffff8b2c022ee000 R14: 0000000000000000 R15: 0000000000000002
[ 15.899004] FS: 00007f0a6e7e86a0(0000) GS:ffff8b2c1f500000(0000)
knlGS:0000000000000000
[ 15.899004] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 15.899004] CR2: 00000000010ddfd0 CR3: 0000000001bc8000 CR4: 00000000000006e0
[ 15.899004] Call Trace:
[ 15.899004] <TASK>
[ 15.899004] lkdtm_CFI_FORWARD_PROTO+0x30/0x57 [lkdtm]
[ 15.899004] direct_entry+0x129/0x137 [lkdtm]
[ 15.899004] full_proxy_write+0x5b/0xa7
[ 15.899004] vfs_write+0x142/0x457
[ 15.899004] ? __x64_sys_wait4+0x5a/0xb7
[ 15.899004] ksys_write+0x69/0xd7
[ 15.899004] do_syscall_64+0x4f/0x97
[ 15.899004] entry_SYSCALL_64_after_hwframe+0x46/0xb0
[ 15.899004] RIP: 0033:0x7f0a6e76dfe1
[ 15.899004] Code: be 07 00 00 00 41 89 c0 e8 7e ff ff ff 44 89 c7 89
04 24 e8 91 c6 02 00 8b 04 24 48 83 c4 68 c3 48 63 ff b8 01 00 00 03
[ 15.899004] RSP: 002b:00007ffe32e133c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 15.899004] RAX: ffffffffffffffda RBX: 00007f0a6e7e8690 RCX: 00007f0a6e76dfe1
[ 15.899004] RDX: 0000000000000012 RSI: 0000000001ad5910 RDI: 0000000000000001
[ 15.899004] RBP: 0000000000000001 R08: fefefefefefefeff R09: fefefeff00abff4e
[ 15.899004] R10: 00007f0a6e7e92b0 R11: 0000000000000246 R12: 0000000001ad5910
[ 15.899004] R13: 0000000000000012 R14: 00007ffe32e13501 R15: 0000000001ad2450
[ 15.899004] </TASK>
[ 15.899004] Modules linked in: lkdtm
[ 15.899004] Dumping ftrace buffer:
[ 15.899004] (ftrace buffer empty)
[ 15.925612] ---[ end trace 0000000000000000 ]---
[ 15.925661] RIP: 0010:lkdtm_indirect_call+0x14/0x20 [lkdtm]
[ 15.925689] Code: c7 c2 76 02 40 c0 48 c7 c1 95 00 40 c0 e9 3b cb 3e
e8 0f 1f 40 00 49 89 fb 48 c7 c7 70 64 40 c0 41 81 7b fa a5 52 0c 7f
[ 15.925697] RSP: 0018:ffff9d928029fdc0 EFLAGS: 00000283
[ 15.925709] RAX: 0000000000000027 RBX: ffffffffc0406320 RCX: 024cef129f458500
[ 15.925716] RDX: ffffffffa9251580 RSI: ffffffffa90736c8 RDI: ffffffffc0406470
[ 15.925722] RBP: 0000000000000006 R08: ffffffffa9251670 R09: 65686374616d7369
[ 15.925731] R10: 000000002e2e2e20 R11: ffffffffc03fdc69 R12: 0000000000000000
[ 15.925739] R13: ffff8b2c022ee000 R14: 0000000000000000 R15: 0000000000000002
[ 15.925746] FS: 00007f0a6e7e86a0(0000) GS:ffff8b2c1f500000(0000)
knlGS:0000000000000000
[ 15.925755] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 15.925762] CR2: 00000000010ddfd0 CR3: 0000000001bc8000 CR4: 00000000000006e0
[ 15.925889] Kernel panic - not syncing: Fatal exception
Signed-off-by: Sami Tolvanen <[email protected]>
---
arch/x86/Kconfig | 2 +
arch/x86/include/asm/cfi.h | 22 +++++++++
arch/x86/include/asm/linkage.h | 12 +++++
arch/x86/kernel/Makefile | 2 +
arch/x86/kernel/cfi.c | 83 ++++++++++++++++++++++++++++++++++
arch/x86/kernel/traps.c | 4 +-
6 files changed, 124 insertions(+), 1 deletion(-)
create mode 100644 arch/x86/include/asm/cfi.h
create mode 100644 arch/x86/kernel/cfi.c
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index be0b95e51df6..8b8c12b223ae 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -108,6 +108,8 @@ config X86
select ARCH_SUPPORTS_PAGE_TABLE_CHECK if X86_64
select ARCH_SUPPORTS_NUMA_BALANCING if X86_64
select ARCH_SUPPORTS_KMAP_LOCAL_FORCE_MAP if NR_CPUS <= 4096
+ select ARCH_SUPPORTS_CFI_CLANG if X86_64
+ select ARCH_USES_CFI_TRAPS if X86_64 && CFI_CLANG
select ARCH_SUPPORTS_LTO_CLANG
select ARCH_SUPPORTS_LTO_CLANG_THIN
select ARCH_USE_BUILTIN_BSWAP
diff --git a/arch/x86/include/asm/cfi.h b/arch/x86/include/asm/cfi.h
new file mode 100644
index 000000000000..86a7b727ed89
--- /dev/null
+++ b/arch/x86/include/asm/cfi.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_CFI_H
+#define _ASM_X86_CFI_H
+
+/*
+ * Clang Control Flow Integrity (CFI) support.
+ *
+ * Copyright (C) 2022 Google LLC
+ */
+
+#include <linux/cfi.h>
+
+#ifdef CONFIG_CFI_CLANG
+enum bug_trap_type handle_cfi_failure(struct pt_regs *regs);
+#else
+static inline enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
+{
+ return BUG_TRAP_TYPE_NONE;
+}
+#endif /*CONFIG_CFI_CLANG */
+
+#endif /* _ASM_X86_CFI_H */
diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
index 85865f1645bd..0ee4a0af3974 100644
--- a/arch/x86/include/asm/linkage.h
+++ b/arch/x86/include/asm/linkage.h
@@ -25,6 +25,18 @@
#define RET ret
#endif
+#ifdef CONFIG_CFI_CLANG
+#define __CFI_TYPE(name) \
+ .fill 7, 1, 0xCC ASM_NL \
+ SYM_START(__cfi_##name, SYM_L_LOCAL, SYM_A_NONE) \
+ int3 ASM_NL \
+ int3 ASM_NL \
+ mov __kcfi_typeid_##name, %eax ASM_NL \
+ int3 ASM_NL \
+ int3 ASM_NL \
+ SYM_FUNC_END(__cfi_##name)
+#endif
+
#else /* __ASSEMBLY__ */
#ifdef CONFIG_SLS
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 03364dc40d8d..2a3dc7e3e6ca 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -145,6 +145,8 @@ obj-$(CONFIG_UNWINDER_GUESS) += unwind_guess.o
obj-$(CONFIG_AMD_MEM_ENCRYPT) += sev.o
+obj-$(CONFIG_CFI_CLANG) += cfi.o
+
###
# 64 bit specific files
ifeq ($(CONFIG_X86_64),y)
diff --git a/arch/x86/kernel/cfi.c b/arch/x86/kernel/cfi.c
new file mode 100644
index 000000000000..7954342b2b3a
--- /dev/null
+++ b/arch/x86/kernel/cfi.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Clang Control Flow Integrity (CFI) support.
+ *
+ * Copyright (C) 2022 Google LLC
+ */
+#include <asm/cfi.h>
+#include <asm/insn.h>
+#include <asm/insn-eval.h>
+
+/*
+ * Returns the target address and the expected type when regs->ip points
+ * to a compiler-generated CFI trap.
+ */
+static bool decode_cfi_insn(struct pt_regs *regs, unsigned long *target,
+ u32 *type)
+{
+ char buffer[MAX_INSN_SIZE];
+ struct insn insn;
+ int offset = 0;
+
+ *target = *type = 0;
+
+ /*
+ * The compiler generates the following instruction sequence
+ * for indirect call checks:
+ *
+ * cmpl <id>, -6(%reg) ; 7-8 bytes
+ * je .Ltmp1 ; 2 bytes
+ * ud2 ; <- addr
+ * .Ltmp1:
+ *
+ * Both the type and the target address can be decoded from the
+ * cmpl instruction.
+ */
+ if (copy_from_kernel_nofault(buffer, (void *)regs->ip - 10, MAX_INSN_SIZE))
+ return false;
+ /*
+ * The compiler may not use r8-r15 without retpolines. Skip the
+ * first byte if it's not the expected REX prefix.
+ */
+ if (buffer[0] != 0x41)
+ ++offset;
+ if (insn_decode_kernel(&insn, &buffer[offset]))
+ return false;
+ if (insn.opcode.value != 0x81 || X86_MODRM_REG(insn.modrm.value) != 7)
+ return false;
+
+ *type = insn.immediate.value;
+
+ /* Read the target address from the register. */
+ offset = insn_get_modrm_rm_off(&insn, regs);
+ if (offset < 0)
+ return false;
+
+ *target = *(unsigned long *)((void *)regs + offset);
+
+ return true;
+}
+
+/*
+ * Checks if a ud2 trap is because of a CFI failure, and handles the trap
+ * if needed. Returns a bug_trap_type value similarly to report_bug.
+ */
+enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
+{
+ unsigned long target;
+ u32 type;
+
+ if (!is_cfi_trap(regs->ip))
+ return BUG_TRAP_TYPE_NONE;
+
+ if (!decode_cfi_insn(regs, &target, &type))
+ return report_cfi_failure_noaddr(regs, regs->ip);
+
+ return report_cfi_failure(regs, regs->ip, &target, type);
+}
+
+/*
+ * Ensure that __kcfi_typeid_ symbols are emitted for functions that may
+ * not be indirectly called with all configurations.
+ */
+__ADDRESSABLE(memcpy)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index d62b2cb85cea..178015a820f0 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -63,6 +63,7 @@
#include <asm/insn-eval.h>
#include <asm/vdso.h>
#include <asm/tdx.h>
+#include <asm/cfi.h>
#ifdef CONFIG_X86_64
#include <asm/x86_init.h>
@@ -313,7 +314,8 @@ static noinstr bool handle_bug(struct pt_regs *regs)
*/
if (regs->flags & X86_EFLAGS_IF)
raw_local_irq_enable();
- if (report_bug(regs->ip, regs) == BUG_TRAP_TYPE_WARN) {
+ if (report_bug(regs->ip, regs) == BUG_TRAP_TYPE_WARN ||
+ handle_cfi_failure(regs) == BUG_TRAP_TYPE_WARN) {
regs->ip += LEN_UD2;
handled = true;
}
--
2.36.1.476.g0c4daa206d-goog
On Fri, Jun 10, 2022 at 04:35:08PM -0700, Sami Tolvanen wrote:
> The __cfi_ preambles contain int3 padding and a mov instruction that
> embeds the KCFI type identifier in the following format:
>
> ; type preamble
> __cfi_function:
> int3
> int3
> mov <id>, %eax
> int3
> int3
> function:
> ...
>
> While the preamble symbols are STT_FUNC and contain valid
> instructions, they are never executed and always fall through. Skip
> the warning for them.
>
> .kcfi_traps sections point to CFI traps in text sections. Also skip
> the warning about them referencing !ENDBR instructions.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
--
Josh
On Fri, Jun 10, 2022 at 04:34:53PM -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.
I think the "RFC" prefix for this series can be dropped. :)
It looks to me like all of Peter's concerns have been addressed. I'd say
let's get the Clang side landed, and once that's done, land this via x86
-tip?
Peter and Will does this sound right to you? It touches arm64, so if
-tip isn't okay, I could take it in one of my trees?
--
Kees Cook
On Mon, Jun 13, 2022 at 10:04:12AM -0700, Kees Cook wrote:
> On Fri, Jun 10, 2022 at 04:34:53PM -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.
>
> I think the "RFC" prefix for this series can be dropped. :)
>
> It looks to me like all of Peter's concerns have been addressed. I'd say
> let's get the Clang side landed, and once that's done, land this via x86
> -tip?
>
> Peter and Will does this sound right to you? It touches arm64, so if
> -tip isn't okay, I could take it in one of my trees?
Sorry, I was a bit pre-occupied with this retbleed crud, I'll try and
have a look at things shortly.
On Mon, Jun 13, 2022 at 10:04:12AM -0700, Kees Cook wrote:
> On Fri, Jun 10, 2022 at 04:34:53PM -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.
>
> I think the "RFC" prefix for this series can be dropped. :)
>
> It looks to me like all of Peter's concerns have been addressed. I'd say
> let's get the Clang side landed, and once that's done, land this via x86
> -tip?
>
> Peter and Will does this sound right to you? It touches arm64, so if
> -tip isn't okay, I could take it in one of my trees?
The arm64 bits look fine to me. Please just check if it conflicts horribly
with -next so that we have a chance to figure out a shared branch if
necessary.
Will
On Fri, Jun 10, 2022 at 04:35:13PM -0700, Sami Tolvanen wrote:
> +#ifdef CONFIG_CFI_CLANG
> +#define __CFI_TYPE(name) \
> + .fill 7, 1, 0xCC ASM_NL \
> + SYM_START(__cfi_##name, SYM_L_LOCAL, SYM_A_NONE) \
> + int3 ASM_NL \
> + int3 ASM_NL \
> + mov __kcfi_typeid_##name, %eax ASM_NL \
> + int3 ASM_NL \
> + int3 ASM_NL \
> + SYM_FUNC_END(__cfi_##name)
> +#endif
Like said on IRC yesterday, this doesn't generate the right mov
encoding.
.byte 0xb8 ; .long __kcfi_typeid_##name ; \
works. Your LLVM tree already has the ZExt patch you gave me yesterday
to fix up the linker fallout from this change.
On Sat, Jul 23, 2022 at 4:21 AM Peter Zijlstra <[email protected]> wrote:
>
> On Fri, Jun 10, 2022 at 04:35:13PM -0700, Sami Tolvanen wrote:
>
> > +#ifdef CONFIG_CFI_CLANG
> > +#define __CFI_TYPE(name) \
> > + .fill 7, 1, 0xCC ASM_NL \
> > + SYM_START(__cfi_##name, SYM_L_LOCAL, SYM_A_NONE) \
> > + int3 ASM_NL \
> > + int3 ASM_NL \
> > + mov __kcfi_typeid_##name, %eax ASM_NL \
> > + int3 ASM_NL \
> > + int3 ASM_NL \
> > + SYM_FUNC_END(__cfi_##name)
> > +#endif
>
> Like said on IRC yesterday, this doesn't generate the right mov
> encoding.
>
> .byte 0xb8 ; .long __kcfi_typeid_##name ; \
>
> works. Your LLVM tree already has the ZExt patch you gave me yesterday
> to fix up the linker fallout from this change.
Indeed, I updated my kernel tree and confirmed that this fixes the
issue. Thanks for pointing it out.
Sami