lkml.org 
[lkml]   [2023]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RFC/RFT] CFI: Add support for gcc CFI in aarch64
    Hi all,

    I am happy to introduce to you that I have contacted a colleague Likun Wang, who
    is willing to continue completing this patch.
    This patch has been delayed for a long time. I hope that gcc will
    support this feature
    in the near future.

    Thanks.
    Dan.

    On Mon, 19 Dec 2022 at 14:18, Dan Li <ashimida.1990@gmail.com> wrote:
    >
    > Based on Sami's patch[1], this patch makes the corresponding kernel
    > configuration of CFI available when compiling the kernel with the gcc[2].
    >
    > The code after enabling cfi is as follows:
    >
    > int (*p)(void);
    > int func (int)
    > {
    > p();
    > }
    >
    > __cfi_func:
    > .4byte 0x439d3502
    > func:
    > ......
    > adrp x0, p
    > add x0, x0, :lo12:p
    > mov w1, 23592
    > movk w1, 0x4601, lsl 16
    > cmp w0, w1
    > beq .L2
    > ......
    > bl cfi_check_failed
    > .L2:
    > blr x19
    > ret
    >
    > In the compiler part[4], there are some differences from Sami's
    > implementation[3], mainly including:
    >
    > 1. When a typeid mismatch is detected, the cfi_check_failed function
    > will be called instead of the brk instruction. This function needs
    > to be implemented by the compiler user.
    > If there are user mode programs or other systems that want to use
    > this feature, it may be more convenient to use a callback (so this
    > compilation option is set to -fsanitize=cfi instead of kcfi).
    >
    > 2. A reserved typeid (such as 0x0U on the aarch64 platform) is always
    > inserted in front of functions that should not be called indirectly.
    > Functions that can be called indirectly will not use this hash value,
    > which prevents instructions/data before the function from being used
    > as a typeid by an attacker.
    >
    > 3. Some bits are ignored in the typeid to avoid conflicts between the
    > typeid and the instruction set of a specific platform, thereby
    > preventing an attacker from bypassing the CFI check by using the
    > instruction as a typeid, such as on the aarch64 platform:
    > * If the following instruction sequence exists:
    > 400620: a9be7bfd stp x29, x30, [sp, #-32]!
    > 400624: 910003fd mov x29, sp
    > 400628: f9000bf3 str x19, [sp, #16]
    > * If the expected typeid of the indirect call is exactly 0x910003fd,
    > the attacker can jump to the next instruction position of any
    > "mov x29,sp" instruction (such as 0x400628 here).
    >
    > 4. Insert a symbol __cfi_<function> before each function's typeid,
    > which may be helpful for fine-grained KASLR implementations (or not?).
    >
    > 5. The current implementation of gcc only supports the aarch64 platform.
    >
    > This produces the following oops on CFI failure (generated using lkdtm):
    >
    > /kselftest_install/lkdtm # ./CFI_FORWARD_PROTO.sh
    > [ 74.856516] lkdtm: Performing direct entry CFI_FORWARD_PROTO
    > [ 74.856878] lkdtm: Calling matched prototype ...
    > [ 74.857011] lkdtm: Calling mismatched prototype ...
    > [ 74.857133] CFI failure at lkdtm_indirect_call+0x30/0x50 (target: lkdtm_increment_int+0x0/0x1c; expected type: 0xc59c68f1)
    > [ 74.858185] Kernel panic - not syncing: Oops - CFI
    > [ 74.859240] CPU: 0 PID: 129 Comm: cat Not tainted 6.0.0-rc4-00024-g32bf7f14f497-dirty #150
    > [ 74.859481] Hardware name: linux,dummy-virt (DT)
    > [ 74.859795] Call trace:
    > [ 74.859959] dump_backtrace.part.0+0xcc/0xe0
    > [ 74.860212] show_stack+0x18/0x5c
    > [ 74.860327] dump_stack_lvl+0x64/0x84
    > [ 74.860398] dump_stack+0x18/0x38
    > [ 74.860443] panic+0x170/0x36c
    > [ 74.860496] cfi_check_failed+0x38/0x44
    > [ 74.860564] lkdtm_indirect_call+0x30/0x50
    > [ 74.860614] lkdtm_CFI_FORWARD_PROTO+0x3c/0x6c
    > [ 74.860701] lkdtm_do_action+0x44/0x58
    > [ 74.860764] direct_entry+0x148/0x160
    > [ 74.860814] full_proxy_write+0x74/0xe0
    > [ 74.860874] vfs_write+0xd8/0x2d0
    > [ 74.860942] ksys_write+0x70/0x110
    > [ 74.861000] __arm64_sys_write+0x1c/0x30
    > [ 74.861067] invoke_syscall+0x5c/0x140
    > [ 74.861117] el0_svc_common.constprop.0+0x44/0xf0
    > [ 74.861190] do_el0_svc+0x2c/0xc0
    > [ 74.861233] el0_svc+0x20/0x60
    > [ 74.861287] el0t_64_sync_handler+0xf4/0x124
    > [ 74.861340] el0t_64_sync+0x160/0x164
    > [ 74.861782] SMP: stopping secondary CPUs
    > [ 74.862336] Kernel Offset: disabled
    > [ 74.862439] CPU features: 0x0000,00075024,699418af
    > [ 74.862799] Memory Limit: none
    > [ 74.863373] ---[ end Kernel panic - not syncing: Oops - CFI ]---
    >
    > The gcc-related patches[4] are based on tag: releases/gcc-12.2.0.
    >
    > Any suggestion please let me know :).
    >
    > Thanks, Dan.
    >
    > [1] https://lore.kernel.org/all/20220908215504.3686827-1-samitolvanen@google.com/
    > [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107048
    > [3] https://reviews.llvm.org/D119296
    > [4] https://lore.kernel.org/linux-hardening/20221219055431.22596-1-ashimida.1990@gmail.com/
    >
    > Signed-off-by: Dan Li <ashimida.1990@gmail.com>
    > ---
    > Makefile | 6 ++++++
    > arch/Kconfig | 24 +++++++++++++++++++++++-
    > arch/arm64/Kconfig | 1 +
    > include/linux/cfi_types.h | 15 +++++++++++----
    > include/linux/compiler-gcc.h | 4 ++++
    > kernel/Makefile | 1 +
    > kernel/cfi.c | 23 +++++++++++++++++++++++
    > scripts/kallsyms.c | 4 +++-
    > 8 files changed, 72 insertions(+), 6 deletions(-)
    >
    > diff --git a/Makefile b/Makefile
    > index 43e08c9f95e9..7c74dac57aa4 100644
    > --- a/Makefile
    > +++ b/Makefile
    > @@ -926,6 +926,12 @@ KBUILD_CFLAGS += $(CC_FLAGS_CFI)
    > export CC_FLAGS_CFI
    > endif
    >
    > +ifdef CONFIG_CFI_GCC
    > +CC_FLAGS_CFI := -fsanitize=cfi
    > +KBUILD_CFLAGS += $(CC_FLAGS_CFI)
    > +export CC_FLAGS_CFI
    > +endif
    > +
    > ifdef CONFIG_DEBUG_FORCE_FUNCTION_ALIGN_64B
    > KBUILD_CFLAGS += -falign-functions=64
    > endif
    > diff --git a/arch/Kconfig b/arch/Kconfig
    > index 1c1eca0c0019..8b43a9fd3b54 100644
    > --- a/arch/Kconfig
    > +++ b/arch/Kconfig
    > @@ -756,9 +756,31 @@ config CFI_CLANG
    >
    > https://clang.llvm.org/docs/ControlFlowIntegrity.html
    >
    > +config ARCH_SUPPORTS_CFI_GCC
    > + bool
    > + help
    > + An architecture should select this option if it can support GCC's
    > + Control-Flow Integrity (CFI) checking.
    > +
    > +config CFI_GCC
    > + bool "Use GCC's Control Flow Integrity (CFI)"
    > + depends on ARCH_SUPPORTS_CFI_GCC
    > + depends on $(cc-option,-fsanitize=cfi)
    > + help
    > + This option enables GCC’s forward-edge Control Flow Integrity
    > + (CFI) checking, where the compiler injects a runtime check to each
    > + indirect function call to ensure the target is a valid function with
    > + the correct static type. This restricts possible call targets and
    > + makes it more difficult for an attacker to exploit bugs that allow
    > + the modification of stored function pointers. More information can be
    > + found from the compiler's documentation:
    > +
    > + - Clang: https://clang.llvm.org/docs/ControlFlowIntegrity.html
    > + - GCC: https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#Instrumentation-Options
    > +
    > config CFI_PERMISSIVE
    > bool "Use CFI in permissive mode"
    > - depends on CFI_CLANG
    > + depends on CFI_CLANG || CFI_GCC
    > help
    > When selected, Control Flow Integrity (CFI) violations result in a
    > warning instead of a kernel panic. This option should only be used
    > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
    > index 9fb9fff08c94..60fdfb01cecb 100644
    > --- a/arch/arm64/Kconfig
    > +++ b/arch/arm64/Kconfig
    > @@ -89,6 +89,7 @@ config ARM64
    > select ARCH_SUPPORTS_LTO_CLANG if CPU_LITTLE_ENDIAN
    > select ARCH_SUPPORTS_LTO_CLANG_THIN
    > select ARCH_SUPPORTS_CFI_CLANG
    > + select ARCH_SUPPORTS_CFI_GCC
    > select ARCH_SUPPORTS_ATOMIC_RMW
    > select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
    > select ARCH_SUPPORTS_NUMA_BALANCING
    > diff --git a/include/linux/cfi_types.h b/include/linux/cfi_types.h
    > index 6b8713675765..1c3b7ea6a79f 100644
    > --- a/include/linux/cfi_types.h
    > +++ b/include/linux/cfi_types.h
    > @@ -8,18 +8,25 @@
    > #ifdef __ASSEMBLY__
    > #include <linux/linkage.h>
    >
    > -#ifdef CONFIG_CFI_CLANG
    > +#if defined(CONFIG_CFI_CLANG) || defined(CONFIG_CFI_GCC)
    > /*
    > - * Use the __kcfi_typeid_<function> type identifier symbol to
    > + * Use the __[k]cfi_typeid_<function> type identifier symbol to
    > * annotate indirectly called assembly functions. The compiler emits
    > * these symbols for all address-taken function declarations in C
    > * code.
    > */
    > #ifndef __CFI_TYPE
    > +
    > +#ifdef CONFIG_CFI_GCC
    > +#define __CFI_TYPE(name) \
    > + .4byte __cfi_typeid_##name
    > +#else
    > #define __CFI_TYPE(name) \
    > .4byte __kcfi_typeid_##name
    > #endif
    >
    > +#endif
    > +
    > #define SYM_TYPED_ENTRY(name, linkage, align...) \
    > linkage(name) ASM_NL \
    > align ASM_NL \
    > @@ -29,12 +36,12 @@
    > #define SYM_TYPED_START(name, linkage, align...) \
    > SYM_TYPED_ENTRY(name, linkage, align)
    >
    > -#else /* CONFIG_CFI_CLANG */
    > +#else /* defined(CONFIG_CFI_CLANG) || defined(CONFIG_CFI_GCC) */
    >
    > #define SYM_TYPED_START(name, linkage, align...) \
    > SYM_START(name, linkage, align)
    >
    > -#endif /* CONFIG_CFI_CLANG */
    > +#endif /* defined(CONFIG_CFI_CLANG) || defined(CONFIG_CFI_GCC) */
    >
    > #ifndef SYM_TYPED_FUNC_START
    > #define SYM_TYPED_FUNC_START(name) \
    > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
    > index 9b157b71036f..aec1ce327b1a 100644
    > --- a/include/linux/compiler-gcc.h
    > +++ b/include/linux/compiler-gcc.h
    > @@ -82,6 +82,10 @@
    > #define __noscs __attribute__((__no_sanitize__("shadow-call-stack")))
    > #endif
    >
    > +#ifdef CONFIG_CFI_GCC
    > +#define __nocfi __attribute__((no_sanitize("cfi")))
    > +#endif
    > +
    > #if __has_attribute(__no_sanitize_address__)
    > #define __no_sanitize_address __attribute__((no_sanitize_address))
    > #else
    > diff --git a/kernel/Makefile b/kernel/Makefile
    > index 318789c728d3..923d3e060852 100644
    > --- a/kernel/Makefile
    > +++ b/kernel/Makefile
    > @@ -114,6 +114,7 @@ obj-$(CONFIG_SHADOW_CALL_STACK) += scs.o
    > obj-$(CONFIG_HAVE_STATIC_CALL) += static_call.o
    > obj-$(CONFIG_HAVE_STATIC_CALL_INLINE) += static_call_inline.o
    > obj-$(CONFIG_CFI_CLANG) += cfi.o
    > +obj-$(CONFIG_CFI_GCC) += cfi.o
    >
    > obj-$(CONFIG_PERF_EVENTS) += events/
    >
    > diff --git a/kernel/cfi.c b/kernel/cfi.c
    > index 08caad776717..9bff35736756 100644
    > --- a/kernel/cfi.c
    > +++ b/kernel/cfi.c
    > @@ -25,6 +25,7 @@ enum bug_trap_type report_cfi_failure(struct pt_regs *regs, unsigned long addr,
    > return BUG_TRAP_TYPE_BUG;
    > }
    >
    > +#ifdef CONFIG_CFI_CLANG
    > #ifdef CONFIG_ARCH_USES_CFI_TRAPS
    > static inline unsigned long trap_address(s32 *p)
    > {
    > @@ -99,3 +100,25 @@ bool is_cfi_trap(unsigned long addr)
    > return is_module_cfi_trap(addr);
    > }
    > #endif /* CONFIG_ARCH_USES_CFI_TRAPS */
    > +#endif /* CONFIG_CFI_CLANG */
    > +
    > +
    > +#ifdef CONFIG_CFI_GCC
    > +void cfi_check_failed(u32 caller_hash, u32 callee_hash, void *callee_addr)
    > +{
    > + unsigned long pc, target;
    > +
    > + pc = (unsigned long)__builtin_return_address(0);
    > + target = (unsigned long)callee_addr;
    > +
    > + switch (report_cfi_failure(NULL, pc, &target, caller_hash)) {
    > + case BUG_TRAP_TYPE_WARN:
    > + break;
    > +
    > + default:
    > + panic("Oops - CFI");
    > + }
    > +}
    > +EXPORT_SYMBOL(cfi_check_failed);
    > +
    > +#endif /* CONFIG_CFI_GCC */
    > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
    > index ccdf0c897f31..ed8db513b918 100644
    > --- a/scripts/kallsyms.c
    > +++ b/scripts/kallsyms.c
    > @@ -119,7 +119,9 @@ static bool is_ignored_symbol(const char *name, char type)
    > "__ThumbV7PILongThunk_",
    > "__LA25Thunk_", /* mips lld */
    > "__microLA25Thunk_",
    > - "__kcfi_typeid_", /* CFI type identifiers */
    > + "__kcfi_typeid_", /* CFI type identifiers in Clang */
    > + "__cfi_", /* CFI type identifiers in GCC */
    > + "__pi___cfi", /* CFI type identifiers in GCC */
    > NULL
    > };
    >
    > --
    > 2.17.1
    >

    \
     
     \ /
      Last update: 2023-12-13 09:30    [W:5.433 / U:0.228 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site