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/[email protected]/
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107048
[3] https://reviews.llvm.org/D119296
[4] https://lore.kernel.org/linux-hardening/[email protected]/
Signed-off-by: Dan Li <[email protected]>
---
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
On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li wrote:
> 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).
This is not going to be acceptible for x86_64.
> 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.
What, if any, are the plans for x86_64 support?
Hi Peter,
On 12/19, Peter Zijlstra wrote:
> On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li wrote:
>
> > 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).
>
> This is not going to be acceptible for x86_64.
I'm not familiar enough with the x86_64 platform, could you please
tell me why this is not acceptable? Is there a similar situation
on the arm64 platform?
> > 5. The current implementation of gcc only supports the aarch64 platform.
>
> What, if any, are the plans for x86_64 support?
I'd like to implement something similar on x86_64 later, but
currently I'm doing this in my spare time, so it might take a
little longer. :(
Thanks,
Dan
On Mon, Dec 19, 2022 at 05:32:04AM -0800, Dan Li wrote:
> Hi Peter,
>
> On 12/19, Peter Zijlstra wrote:
> > On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li wrote:
> >
> > > 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).
> >
> > This is not going to be acceptible for x86_64.
>
> I'm not familiar enough with the x86_64 platform, could you please
> tell me why this is not acceptable? Is there a similar situation
> on the arm64 platform?
Mostly because the call would be a 5 byte instruction while the trap
(UD2) is only 2 bytes.
I suspect Argh64 has a similar problem if the to be called function is
outside the immediate range (26 bits or thereabout), in which case you
end up with a multi-instruction sequence to construct the call target or
so. A trap is always a single instruction.
On 12/19, Peter Zijlstra wrote:
> On Mon, Dec 19, 2022 at 05:32:04AM -0800, Dan Li wrote:
> > Hi Peter,
> >
> > On 12/19, Peter Zijlstra wrote:
> > > On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li wrote:
> > >
> > > > 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).
> > >
> > > This is not going to be acceptible for x86_64.
> >
> > I'm not familiar enough with the x86_64 platform, could you please
> > tell me why this is not acceptable? Is there a similar situation
> > on the arm64 platform?
>
> Mostly because the call would be a 5 byte instruction while the trap
> (UD2) is only 2 bytes.
Oh ok, got it.
> I suspect Argh64 has a similar problem if the to be called function is
> outside the immediate range (26 bits or thereabout), in which case you
> end up with a multi-instruction sequence to construct the call target or
> so. A trap is always a single instruction.
>
Yes, IIRC, long jumps also typically require at least three instructions
in arm64.
Thanks,
Dan.
On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li 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.
For arm64, we specifically wanted the BRK approach (with the information endoed
into an immediate) because it gave us all the information we needed to diagnose
the failed check, while giving the compiler freedom to perform the check
however it wanted, and without needing to shuffle a bunch of registers (or
having a weird calling convention for the cfi_check_failed handler).
> 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).
I appreciate that may be nicer for userspace, but it would be far nicer for the
kernel if we could have a kcfi mode that behaves the same as LLVM, using a BRK.
That's going to be simpler for the kernel to deal with, and should result in
nicer code / smaller binary size (for the reasons given above).
Can we please have an LLVM-compatible KCFI mode, and have the -fsanitize=cfi be
a separate option from -fsanitize=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.
That sounds sensible, though it meanse we'll need to go audit all the assembly
without type annotations.
I presume that "functions that should not be called indirectly" only includes
those which are not directly visible outside the compilation unit AND whose
address is never taken / escaped from the compilation unit. Is that the case?
> 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).
Which bits exactly are ignored on arm64?
e.g. are these encoded into UDF immediates?
> 4. Insert a symbol __cfi_<function> before each function's typeid,
> which may be helpful for fine-grained KASLR implementations (or not?).
I can imagine this is useful, but I am not immediately sure.
> 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 :).
As a general thing, how does this work with -fpatchable-function-entry=M,N,
where N is non-zero?
We still need to fix that for LLVM, and it would be good to align on the same behaviour.
Thanks,
Mark.
>
> Thanks, Dan.
>
> [1] https://lore.kernel.org/all/[email protected]/
> [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107048
> [3] https://reviews.llvm.org/D119296
> [4] https://lore.kernel.org/linux-hardening/[email protected]/
>
> Signed-off-by: Dan Li <[email protected]>
> ---
> 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
>
On Mon, Dec 19, 2022 at 04:04:55PM +0100, Peter Zijlstra wrote:
> On Mon, Dec 19, 2022 at 05:32:04AM -0800, Dan Li wrote:
> > Hi Peter,
> >
> > On 12/19, Peter Zijlstra wrote:
> > > On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li wrote:
> > >
> > > > 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).
> > >
> > > This is not going to be acceptible for x86_64.
> >
> > I'm not familiar enough with the x86_64 platform, could you please
> > tell me why this is not acceptable? Is there a similar situation
> > on the arm64 platform?
>
> Mostly because the call would be a 5 byte instruction while the trap
> (UD2) is only 2 bytes.
>
> I suspect Argh64 has a similar problem if the to be called function is
> outside the immediate range (26 bits or thereabout), in which case you
> end up with a multi-instruction sequence to construct the call target or
> so.
Either that or a direct branc to a PLT.
> A trap is always a single instruction.
Indeed.
I strongly prefer the BRK for the reasons I've given in my other reply, which
include code size.
Thanks,
Mark.
On Mon, Dec 19, 2022 at 05:32:04AM -0800, Dan Li wrote:
> Hi Peter,
>
> On 12/19, Peter Zijlstra wrote:
> > On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li wrote:
> >
> > > 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).
> >
> > This is not going to be acceptible for x86_64.
>
> I'm not familiar enough with the x86_64 platform, could you please
> tell me why this is not acceptable? Is there a similar situation
> on the arm64 platform?
>
> > > 5. The current implementation of gcc only supports the aarch64 platform.
> >
> > What, if any, are the plans for x86_64 support?
>
> I'd like to implement something similar on x86_64 later, but
> currently I'm doing this in my spare time, so it might take a
> little longer. :(
Hi!
First of all, thank you thank you for working on this in GCC. This will
make a big difference for folks that don't have the option to build with
Clang to gain CFI coverage.
As for the implementation details, the core issue is really that this
type of CFI is specifically designed for the Linux kernel, and it took a
rather long time to figure out all the specifics needed (down to the
byte counts and instruction layouts). GCC's version will ultimately need
to exactly match the Clang output, or Linux is unlikely to support it.
We're already on our second CFI -- the original Clang CFI was just too
clunky for long-term use in Linux, so unless we're going to improve on
the latest Clang KCFI implementation in some way, it's better to stick
to exactly byte-for-byte identical results. The KCFI support in Linux
depends on the arm64 and x86_64 runtimes for catching the traps, and the
post-processing done (on x86_64) with objtool that prepares the kernel
for IBT use, and converts to the optional FineIBT CFI mechanism. With
all those moving parts, there needs to be a very compelling reason to
have GCC KCFI implementation differ from Clang's.
Hopefully that context helps a little. I'm excited to try out future
versions!
-Kees
--
Kees Cook
Hi Mark,
Sorry for the late reply.
On 01/03, Mark Rutland wrote:
> On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li wrote:
> > 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).
>
> I appreciate that may be nicer for userspace, but it would be far nicer for the
> kernel if we could have a kcfi mode that behaves the same as LLVM, using a BRK.
> That's going to be simpler for the kernel to deal with, and should result in
> nicer code / smaller binary size (for the reasons given above).
>
> Can we please have an LLVM-compatible KCFI mode, and have the -fsanitize=cfi be
> a separate option from -fsanitize=kcfi?
Ok, in the next version I will change to the same option as clang :)
>
> > 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.
>
> That sounds sensible, though it meanse we'll need to go audit all the assembly
> without type annotations.
>
> I presume that "functions that should not be called indirectly" only includes
> those which are not directly visible outside the compilation unit AND whose
> address is never taken / escaped from the compilation unit. Is that the case?
Yes.
>
> > 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).
>
> Which bits exactly are ignored on arm64?
>
> e.g. are these encoded into UDF immediates?
In aarch64, I currently ignore bit [28:27]. IUCC, according to the manual[1],
it is a UDF instruction only when the upper 16 bits are all 0.
But due to this has too much impact on the entropy of typeid, so I (not
rigorously) only ignore 2 bits here, and most of the instruction codes covered
by it belong to 'Reserved' or 'UNALLOCATED' (probably not a good idea).
But as Kees said, if clang doesn't handle it here, in order to be consistent,
I think it's better for gcc to not handle it when implementing kernel cfi.
[1] https://developer.arm.com/documentation/ddi0602/2022-06/Index-by-Encoding?lang=en
>
> As a general thing, how does this work with -fpatchable-function-entry=M,N,
> where N is non-zero?
>
> We still need to fix that for LLVM, and it would be good to align on the same behaviour.
>
Yeah, it makes sense.
Currently, it is consistent with llvm. Taking -fpatchable-function-entry=2,1
as an example, the currently generated code is as follows:
__cfi_main:
.4byte 0x439d3502
.global main
.section __patchable_function_entries
.align 3
.8byte .LPFE3
.text
.LPFE3:
nop
.type main, %function
main:
nop
.LFB2:
.cfi_startproc
stp x29, x30, [sp, -32]!
Finally, do we want to generate code like this?
nop
.4byte 0x439d3502
main:
nop
...
Thanks,
Dan.
> >
> > --
> > 2.17.1
> >
Hi Kees,
On 01/06, Kees Cook wrote:
> On Mon, Dec 19, 2022 at 05:32:04AM -0800, Dan Li wrote:
> > Hi Peter,
> Hi!
>
> First of all, thank you thank you for working on this in GCC. This will
> make a big difference for folks that don't have the option to build with
> Clang to gain CFI coverage.
>
> As for the implementation details, the core issue is really that this
> type of CFI is specifically designed for the Linux kernel, and it took a
> rather long time to figure out all the specifics needed (down to the
> byte counts and instruction layouts). GCC's version will ultimately need
> to exactly match the Clang output, or Linux is unlikely to support it.
>
> We're already on our second CFI -- the original Clang CFI was just too
> clunky for long-term use in Linux, so unless we're going to improve on
> the latest Clang KCFI implementation in some way, it's better to stick
> to exactly byte-for-byte identical results. The KCFI support in Linux
> depends on the arm64 and x86_64 runtimes for catching the traps, and the
> post-processing done (on x86_64) with objtool that prepares the kernel
> for IBT use, and converts to the optional FineIBT CFI mechanism. With
> all those moving parts, there needs to be a very compelling reason to
> have GCC KCFI implementation differ from Clang's.
>
> Hopefully that context helps a little. I'm excited to try out future
> versions!
Thanks for the context, it makes sense and helped me a lot. :)
In the next version I'll make the gcc implementation consistent with clang.
Thanks,
Dan.
>
> -Kees
>
> --
> Kees Cook
On Sat, Jan 07, 2023 at 07:42:13AM -0800, Dan Li wrote:
> Hi Kees,
>
> On 01/06, Kees Cook wrote:
> > On Mon, Dec 19, 2022 at 05:32:04AM -0800, Dan Li wrote:
> > > Hi Peter,
> > Hi!
> >
> > First of all, thank you thank you for working on this in GCC. This will
> > make a big difference for folks that don't have the option to build with
> > Clang to gain CFI coverage.
> >
> > As for the implementation details, the core issue is really that this
> > type of CFI is specifically designed for the Linux kernel, and it took a
> > rather long time to figure out all the specifics needed (down to the
> > byte counts and instruction layouts). GCC's version will ultimately need
> > to exactly match the Clang output, or Linux is unlikely to support it.
> >
> > We're already on our second CFI -- the original Clang CFI was just too
> > clunky for long-term use in Linux, so unless we're going to improve on
> > the latest Clang KCFI implementation in some way, it's better to stick
> > to exactly byte-for-byte identical results. The KCFI support in Linux
> > depends on the arm64 and x86_64 runtimes for catching the traps, and the
> > post-processing done (on x86_64) with objtool that prepares the kernel
> > for IBT use, and converts to the optional FineIBT CFI mechanism. With
> > all those moving parts, there needs to be a very compelling reason to
> > have GCC KCFI implementation differ from Clang's.
> >
> > Hopefully that context helps a little. I'm excited to try out future
> > versions!
>
> Thanks for the context, it makes sense and helped me a lot. :)
>
> In the next version I'll make the gcc implementation consistent with clang.
Hi!
Just checking in on this, since there are a lot of interested folks. :)
What's the status on the next version (and has anyone been found to
tackle the x86 backend part)? Is there anything we can help with?
Thanks!
-Kees
--
Kees Cook
On 02/08, Kees Cook wrote:
> On Sat, Jan 07, 2023 at 07:42:13AM -0800, Dan Li wrote:
> > Hi Kees,
> >
> > In the next version I'll make the gcc implementation consistent with clang.
>
> Hi!
>
> Just checking in on this, since there are a lot of interested folks. :)
> What's the status on the next version (and has anyone been found to
> tackle the x86 backend part)? Is there anything we can help with?
>
Hi Kees,
Sorry for the long delay. It's been hard for me to find time to finish this
until the end of February due to job changes (even weekends are busy :( ).
As much as I'd love to get it done as soon as possible, I'm sorry I probably
won't be able to submit the next version until March (maybe mid).
If anyone could help improve this, I'd be very grateful, otherwise I'll have
the next version done by the end of March (as a promise), and also try it in
x86 platform.
Sincerely apologize!
Dan.
> Thanks!
>
> -Kees
>
> --
> Kees Cook
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 (with -fsanitize=kcfi,
-fpatchable-function-entry=3,1) is as follows:
int (*p)(void);
int func (int)
{
p();
}
__kcfi_func:
.4byte 0x439d3502
.global func
.text
.LPFE1:
nop
.type func, %function
func:
nop
nop
.LFB0:
......
adrp x0, p
add x0, x0, :lo12:p
ldr x0, [x0]
ldur w16, [x0, #-8]
movk w17, #23592
movk w17, #17921, lsl #16
cmp w16, w17
b.eq .Lkcfi2
brk #0x8220
.Lkcfi2:
blr x0
......
In the compiler part[4], most of the content is the same as Sami's
implementation[3], except for some minor differences, mainly including:
1. The function typeid is calculated differently and it is difficult
to be consistent.
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 __kcfi_<function> before each function's typeid,
which may be helpful for fine-grained KASLR implementations.
I'm not sure if these distinctions need to be removed, they're kept
in this version for now, and I'm happy to remove them in the next
version if necessary (except for the first one).
The current implementation of gcc only supports the aarch64 platform，
the x86 version is still in progress.
This produces the following oops on CFI failure (generated using lkdtm):
/ # echo CFI_FORWARD_PROTO > /sys/kernel/debug/provoke-crash/DIRECT
[ 17.153364] lkdtm: Performing direct entry CFI_FORWARD_PROTO
[ 17.153675] lkdtm: Calling matched prototype ...
[ 17.154188] lkdtm: Calling mismatched prototype ...
[ 17.154553] CFI failure at lkdtm_indirect_call+0x38/0x54 (target: lkdtm_increment_int+0x0/0x2c; expected type: 0xc59c68f1)
[ 17.155627] Internal error: Oops - CFI: 0000000000000000 [#1] PREEMPT SMP
[ 17.156025] Modules linked in:
[ 17.156580] CPU: 1 PID: 110 Comm: sh Not tainted 6.1.0-00001-g7ead10685f57-dirty #2
[ 17.156975] Hardware name: linux,dummy-virt (DT)
[ 17.157290] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 17.157621] pc : lkdtm_indirect_call+0x38/0x54
[ 17.157902] lr : lkdtm_CFI_FORWARD_PROTO+0x48/0x7c
[ 17.158142] sp : ffff80000b083c80
[ 17.158316] x29: ffff80000b083c80 x28: ffff0000068ccc40 x27: 0000000000000000
[ 17.158820] x26: 0000000000000000 x25: 0000000000000000 x24: 000000000bd069a0
[ 17.159239] x23: ffff0000053493c0 x22: ffff80000b083df0 x21: 0000000000000012
[ 17.159673] x20: ffff80000aa29150 x19: ffff000006909000 x18: ffff80000b035068
[ 17.160110] x17: 00000000c59c68f1 x16: 00000000a2d40772 x15: 0000000000000006
[ 17.160530] x14: 0000000000000000 x13: 2e2e2e2065707974 x12: 6f746f7270206465
[ 17.160947] x11: 686374616d73696d x10: ffff80000a7254b8 x9 : ffff800009268330
[ 17.161376] x8 : 00000000ffffefff x7 : ffff80000a7254b8 x6 : 80000000fffff000
[ 17.161789] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
[ 17.162264] x2 : 0000000000000000 x1 : ffff800008a4e248 x0 : ffff80000abca500
[ 17.162865] Call trace:
[ 17.163099] lkdtm_indirect_call+0x38/0x54
[ 17.163411] lkdtm_CFI_FORWARD_PROTO+0x48/0x7c
[ 17.163663] lkdtm_do_action+0x48/0x5c
[ 17.163876] direct_entry+0x144/0x160
[ 17.164084] full_proxy_write+0x84/0xe4
[ 17.164298] vfs_write+0xe8/0x32c
The gcc-related patches[4] are based on tag: releases/gcc-12.2.0.
This patch is based on tag: v6.2.
Sorry for the long delay, the next version should not be that slow.
Any suggestion please let me know :).
Thanks,
Dan.
[1] https://lore.kernel.org/all/[email protected]/
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107048
[3] https://reviews.llvm.org/D119296
[4] https://lore.kernel.org/all/[email protected]/
Signed-off-by: Dan Li <[email protected]>
---
RFC/RFT V2:
- The CFI typeid check is changed from the form of calling the callback
function to the calling of the brk instruction.
- Add support for -fpatchable-function-entry=M,N
---
Makefile | 2 +-
arch/Kconfig | 21 +++++++++++----------
arch/arm64/Kconfig | 2 +-
arch/arm64/kernel/traps.c | 8 ++++----
arch/x86/Kconfig | 12 ++++++------
arch/x86/include/asm/cfi.h | 4 ++--
arch/x86/kernel/Makefile | 2 +-
arch/x86/purgatory/Makefile | 2 +-
drivers/misc/lkdtm/cfi.c | 2 +-
include/asm-generic/vmlinux.lds.h | 2 +-
include/linux/cfi.h | 4 ++--
include/linux/cfi_types.h | 6 +++---
include/linux/compiler-gcc.h | 4 ++++
kernel/Makefile | 2 +-
kernel/module/Kconfig | 2 +-
scripts/kallsyms.c | 3 ++-
tools/perf/util/include/linux/linkage.h | 2 +-
17 files changed, 43 insertions(+), 37 deletions(-)
diff --git a/Makefile b/Makefile
index 3f6628780eb2..0dd22c76c39e 100644
--- a/Makefile
+++ b/Makefile
@@ -1014,7 +1014,7 @@ KBUILD_AFLAGS += -fno-lto
export CC_FLAGS_LTO
endif
-ifdef CONFIG_CFI_CLANG
+ifdef CONFIG_CFI
CC_FLAGS_CFI := -fsanitize=kcfi
KBUILD_CFLAGS += $(CC_FLAGS_CFI)
export CC_FLAGS_CFI
diff --git a/arch/Kconfig b/arch/Kconfig
index 12e3ddabac9d..ff015a611091 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -748,33 +748,34 @@ config LTO_CLANG_THIN
If unsure, say Y.
endchoice
-config ARCH_SUPPORTS_CFI_CLANG
+config ARCH_SUPPORTS_CFI
bool
help
- An architecture should select this option if it can support Clang's
- Control-Flow Integrity (CFI) checking.
+ An architecture should select this option if it can support the
+ compiler'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 ARCH_SUPPORTS_CFI_CLANG
+config CFI
+ bool "Use Control Flow Integrity (CFI)"
+ depends on ARCH_SUPPORTS_CFI
depends on $(cc-option,-fsanitize=kcfi)
help
- This option enables Clang’s forward-edge Control Flow Integrity
+ This option enables the compiler’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 Clang's documentation:
+ found from the compiler's documentation:
- https://clang.llvm.org/docs/ControlFlowIntegrity.html
+ - 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
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 c5ccca26a408..8795d4f68420 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -90,7 +90,7 @@ config ARM64
select ARCH_SUPPORTS_SHADOW_CALL_STACK if CC_HAVE_SHADOW_CALL_STACK
select ARCH_SUPPORTS_LTO_CLANG if CPU_LITTLE_ENDIAN
select ARCH_SUPPORTS_LTO_CLANG_THIN
- select ARCH_SUPPORTS_CFI_CLANG
+ select ARCH_SUPPORTS_CFI
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
select ARCH_SUPPORTS_NUMA_BALANCING
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 4c0caa589e12..26d29dfdf3eb 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -981,7 +981,7 @@ static struct break_hook bug_break_hook = {
.imm = BUG_BRK_IMM,
};
-#ifdef CONFIG_CFI_CLANG
+#ifdef CONFIG_CFI
static int cfi_handler(struct pt_regs *regs, unsigned long esr)
{
unsigned long target;
@@ -1011,7 +1011,7 @@ static struct break_hook cfi_break_hook = {
.imm = CFI_BRK_IMM_BASE,
.mask = CFI_BRK_IMM_MASK,
};
-#endif /* CONFIG_CFI_CLANG */
+#endif /* CONFIG_CFI*/
static int reserved_fault_handler(struct pt_regs *regs, unsigned long esr)
{
@@ -1084,7 +1084,7 @@ 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
+#ifdef CONFIG_CFI
if ((esr_comment(esr) & ~CFI_BRK_IMM_MASK) == CFI_BRK_IMM_BASE)
return cfi_handler(regs, esr) != DBG_HOOK_HANDLED;
#endif
@@ -1098,7 +1098,7 @@ 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
+#ifdef CONFIG_CFI
register_kernel_break_hook(&cfi_break_hook);
#endif
register_kernel_break_hook(&fault_break_hook);
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3604074a878b..f014c58ef8a3 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -110,8 +110,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_CFI if X86_64
+ select ARCH_USES_CFI_TRAPS if X86_64 && CFI
select ARCH_SUPPORTS_LTO_CLANG
select ARCH_SUPPORTS_LTO_CLANG_THIN
select ARCH_USE_BUILTIN_BSWAP
@@ -2501,11 +2501,11 @@ config FUNCTION_PADDING_CFI
default 3 if FUNCTION_ALIGNMENT_8B
default 0
-# Basically: FUNCTION_ALIGNMENT - 5*CFI_CLANG
+# Basically: FUNCTION_ALIGNMENT - 5*CFI
# except Kconfig can't do arithmetic :/
config FUNCTION_PADDING_BYTES
int
- default FUNCTION_PADDING_CFI if CFI_CLANG
+ default FUNCTION_PADDING_CFI if CFI
default FUNCTION_ALIGNMENT
config CALL_PADDING
@@ -2515,7 +2515,7 @@ config CALL_PADDING
config FINEIBT
def_bool y
- depends on X86_KERNEL_IBT && CFI_CLANG && RETPOLINE
+ depends on X86_KERNEL_IBT && CFI && RETPOLINE
select CALL_PADDING
config HAVE_CALL_THUNKS
@@ -2528,7 +2528,7 @@ config CALL_THUNKS
config PREFIX_SYMBOLS
def_bool y
- depends on CALL_PADDING && !CFI_CLANG
+ depends on CALL_PADDING && !CFI
menuconfig SPECULATION_MITIGATIONS
bool "Mitigations for speculative execution vulnerabilities"
diff --git a/arch/x86/include/asm/cfi.h b/arch/x86/include/asm/cfi.h
index 58dacd90daef..dad029608ac2 100644
--- a/arch/x86/include/asm/cfi.h
+++ b/arch/x86/include/asm/cfi.h
@@ -10,13 +10,13 @@
#include <linux/cfi.h>
-#ifdef CONFIG_CFI_CLANG
+#ifdef CONFIG_CFI
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 /* CONFIG_CFI */
#endif /* _ASM_X86_CFI_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 96d51bbc2bd4..2e756e8f5bc1 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -141,7 +141,7 @@ obj-$(CONFIG_UNWINDER_GUESS) += unwind_guess.o
obj-$(CONFIG_AMD_MEM_ENCRYPT) += sev.o
-obj-$(CONFIG_CFI_CLANG) += cfi.o
+obj-$(CONFIG_CFI) += cfi.o
obj-$(CONFIG_CALL_THUNKS) += callthunks.o
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 17f09dc26381..a88b7212d39e 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -56,7 +56,7 @@ ifdef CONFIG_RETPOLINE
PURGATORY_CFLAGS_REMOVE += $(RETPOLINE_CFLAGS)
endif
-ifdef CONFIG_CFI_CLANG
+ifdef CONFIG_CFI
PURGATORY_CFLAGS_REMOVE += $(CC_FLAGS_CFI)
endif
diff --git a/drivers/misc/lkdtm/cfi.c b/drivers/misc/lkdtm/cfi.c
index fc28714ae3a6..2720fe4c57a6 100644
--- a/drivers/misc/lkdtm/cfi.c
+++ b/drivers/misc/lkdtm/cfi.c
@@ -43,7 +43,7 @@ static void lkdtm_CFI_FORWARD_PROTO(void)
lkdtm_indirect_call((void *)lkdtm_increment_int);
pr_err("FAIL: survived mismatched prototype function call!\n");
- pr_expected_config(CONFIG_CFI_CLANG);
+ pr_expected_config(CONFIG_CFI);
}
/*
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 659bf3b31c91..1276722fdf95 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -162,7 +162,7 @@
#define PATCHABLE_DISCARDS *(__patchable_function_entries)
#endif
-#ifndef CONFIG_ARCH_SUPPORTS_CFI_CLANG
+#ifndef CONFIG_ARCH_SUPPORTS_CFI
/*
* Simply points to ftrace_stub, but with the proper protocol.
* Defined by the linker script in linux/vmlinux.lds.h
diff --git a/include/linux/cfi.h b/include/linux/cfi.h
index 5e134f4ce8b7..d15cfc3c748b 100644
--- a/include/linux/cfi.h
+++ b/include/linux/cfi.h
@@ -10,7 +10,7 @@
#include <linux/bug.h>
#include <linux/module.h>
-#ifdef CONFIG_CFI_CLANG
+#ifdef CONFIG_CFI
enum bug_trap_type report_cfi_failure(struct pt_regs *regs, unsigned long addr,
unsigned long *target, u32 type);
@@ -23,7 +23,7 @@ static inline enum bug_trap_type report_cfi_failure_noaddr(struct pt_regs *regs,
#ifdef CONFIG_ARCH_USES_CFI_TRAPS
bool is_cfi_trap(unsigned long addr);
#endif
-#endif /* CONFIG_CFI_CLANG */
+#endif /* CONFIG_CFI */
#ifdef CONFIG_MODULES
#ifdef CONFIG_ARCH_USES_CFI_TRAPS
diff --git a/include/linux/cfi_types.h b/include/linux/cfi_types.h
index 6b8713675765..2e098274e45c 100644
--- a/include/linux/cfi_types.h
+++ b/include/linux/cfi_types.h
@@ -8,7 +8,7 @@
#ifdef __ASSEMBLY__
#include <linux/linkage.h>
-#ifdef CONFIG_CFI_CLANG
+#ifdef CONFIG_CFI
/*
* Use the __kcfi_typeid_<function> type identifier symbol to
* annotate indirectly called assembly functions. The compiler emits
@@ -29,12 +29,12 @@
#define SYM_TYPED_START(name, linkage, align...) \
SYM_TYPED_ENTRY(name, linkage, align)
-#else /* CONFIG_CFI_CLANG */
+#else /* CONFIG_CFI */
#define SYM_TYPED_START(name, linkage, align...) \
SYM_START(name, linkage, align)
-#endif /* CONFIG_CFI_CLANG */
+#endif /* CONFIG_CFI */
#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 7af9e34ec261..4838081392e2 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
+#define __nocfi __attribute__((no_sanitize("kcfi")))
+#endif
+
#define __no_sanitize_address __attribute__((__no_sanitize_address__))
#if defined(__SANITIZE_THREAD__)
diff --git a/kernel/Makefile b/kernel/Makefile
index 10ef068f598d..756abc34140e 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -112,7 +112,7 @@ obj-$(CONFIG_KCSAN) += kcsan/
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) += cfi.o
obj-$(CONFIG_PERF_EVENTS) += events/
diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 424b3bc58f3f..e40de2aaea87 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -289,6 +289,6 @@ config UNUSED_KSYMS_WHITELIST
config MODULES_TREE_LOOKUP
def_bool y
- depends on PERF_EVENTS || TRACING || CFI_CLANG
+ depends on PERF_EVENTS || TRACING || CFI
endif # MODULES
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 8a68179a98a3..bbb6d3a91089 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -136,7 +136,8 @@ static bool is_ignored_symbol(const char *name, char type)
"__ThumbV7PILongThunk_",
"__LA25Thunk_", /* mips lld */
"__microLA25Thunk_",
- "__kcfi_typeid_", /* CFI type identifiers */
+ "__kcfi_", /* CFI type identifiers */
+ "__pi___kcfi", /* CFI type identifiers */
NULL
};
diff --git a/tools/perf/util/include/linux/linkage.h b/tools/perf/util/include/linux/linkage.h
index 75e2248416f5..20d87771f6ac 100644
--- a/tools/perf/util/include/linux/linkage.h
+++ b/tools/perf/util/include/linux/linkage.h
@@ -116,7 +116,7 @@
#endif
// In the kernel sources (include/linux/cfi_types.h), this has a different
-// definition when CONFIG_CFI_CLANG is used, for tools/ just use the !clang
+// definition when CONFIG_CFI is used, for tools/ just use the !clang
// definition:
#ifndef SYM_TYPED_START
#define SYM_TYPED_START(name, linkage, align...) \
--
2.17.1
On Sat, Mar 25, 2023 at 01:54:16AM -0700, Dan Li wrote:
> In the compiler part[4], most of the content is the same as Sami's
> implementation[3], except for some minor differences, mainly including:
>
> 1. The function typeid is calculated differently and it is difficult
> to be consistent.
This means there is an effective ABI break between the compilers, which
is sad :-( Is there really nothing to be done about this?
On Mon, Mar 27, 2023 at 2:30 AM Peter Zijlstra <[email protected]> wrote:
>
> On Sat, Mar 25, 2023 at 01:54:16AM -0700, Dan Li wrote:
>
> > In the compiler part[4], most of the content is the same as Sami's
> > implementation[3], except for some minor differences, mainly including:
> >
> > 1. The function typeid is calculated differently and it is difficult
> > to be consistent.
>
> This means there is an effective ABI break between the compilers, which
> is sad :-( Is there really nothing to be done about this?
I agree, this would be unfortunate, and would also be a compatibility
issue with rustc where there's ongoing work to support
clang-compatible CFI type hashes:
https://github.com/rust-lang/rust/pull/105452
Sami
On 03/27, Peter Zijlstra wrote:
> On Sat, Mar 25, 2023 at 01:54:16AM -0700, Dan Li wrote:
>
> > In the compiler part[4], most of the content is the same as Sami's
> > implementation[3], except for some minor differences, mainly including:
> >
> > 1. The function typeid is calculated differently and it is difficult
> > to be consistent.
>
> This means there is an effective ABI break between the compilers, which
> is sad :-( Is there really nothing to be done about this?
Hi Peter,
I'm not so sure right now. According to Sami's tip, I need to learn more about
related patches. I'll make it ABI compatible in the next version if possible :)
Thanks,
Dan
On 03/27, Sami Tolvanen wrote:
> On Mon, Mar 27, 2023 at 2:30 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Sat, Mar 25, 2023 at 01:54:16AM -0700, Dan Li wrote:
> >
> > > In the compiler part[4], most of the content is the same as Sami's
> > > implementation[3], except for some minor differences, mainly including:
> > >
> > > 1. The function typeid is calculated differently and it is difficult
> > > to be consistent.
> >
> > This means there is an effective ABI break between the compilers, which
> > is sad :-( Is there really nothing to be done about this?
>
> I agree, this would be unfortunate, and would also be a compatibility
> issue with rustc where there's ongoing work to support
> clang-compatible CFI type hashes:
>
> https://github.com/rust-lang/rust/pull/105452
>
> Sami
Hi Sami,
Thanks for the info, I need to learn about it :)
Is there anything else that needs to be improved?
Thanks,
Dan
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 <[email protected]> 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/[email protected]/
> [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107048
> [3] https://reviews.llvm.org/D119296
> [4] https://lore.kernel.org/linux-hardening/[email protected]/
>
> Signed-off-by: Dan Li <[email protected]>
> ---
> 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
>
+ Likun
On Tue, 28 Mar 2023 at 06:18, Sami Tolvanen <[email protected]> wrote:
>
> On Mon, Mar 27, 2023 at 2:30 AM Peter Zijlstra <[email protected]> wrote:
> >
> > On Sat, Mar 25, 2023 at 01:54:16AM -0700, Dan Li wrote:
> >
> > > In the compiler part[4], most of the content is the same as Sami's
> > > implementation[3], except for some minor differences, mainly including:
> > >
> > > 1. The function typeid is calculated differently and it is difficult
> > > to be consistent.
> >
> > This means there is an effective ABI break between the compilers, which
> > is sad :-( Is there really nothing to be done about this?
>
> I agree, this would be unfortunate, and would also be a compatibility
> issue with rustc where there's ongoing work to support
> clang-compatible CFI type hashes:
>
> https://github.com/rust-lang/rust/pull/105452
>
> Sami
On Wed, Dec 13, 2023 at 05:01:07PM +0800, Wang wrote:
> On 2023/12/13 16:48, Dan Li wrote:
> > + Likun
> >
> > On Tue, 28 Mar 2023 at 06:18, Sami Tolvanen wrote:
> >> On Mon, Mar 27, 2023 at 2:30 AM Peter Zijlstra wrote:
> >>> On Sat, Mar 25, 2023 at 01:54:16AM -0700, Dan Li wrote:
> >>>
> >>>> In the compiler part[4], most of the content is the same as Sami's
> >>>> implementation[3], except for some minor differences, mainly including:
> >>>>
> >>>> 1. The function typeid is calculated differently and it is difficult
> >>>> to be consistent.
> >>> This means there is an effective ABI break between the compilers, which
> >>> is sad :-( Is there really nothing to be done about this?
> >> I agree, this would be unfortunate, and would also be a compatibility
> >> issue with rustc where there's ongoing work to support
> >> clang-compatible CFI type hashes:
> >>
> >> https://github.com/rust-lang/rust/pull/105452
> >>
> >> Sami
>
> Hi Peter and Sami
>
> I am Dan Li's colleague, and I will take over and continue the work of CFI.
>
> Regarding the issue of gcc cfi type id being compatible with clang, we
> have analyzed and verified:
>
> 1. clang uses Mangling defined in Itanium C++ ABI to encode the function
> prototype, and uses the encoding result as input to generate cfi type id;
> 2. Currently, gcc only implements mangling for the C++ compiler, and the
> function prototype coding generated by these interfaces is compatible
> with clang, but gcc's c compiler does not support mangling.;
>
> Adding mangling to gcc's c compiler is a huge and difficult task，because
> we have to refactor the mangling of C++, splitting it into basic
> mangling and language specific mangling, and adding support for the c
> language which requires a deep understanding of the compiler and
> language processing parts.
>
> And for the kernel cfi, I suggest separating type compatibility from CFI
> basic functions. Type compatibility is independent from CFI basic
> funcitons and should be dealt with under another topic. Should we focus
> on the main issus of cfi, and let it work first on linux kernel, and
> left the compatible issue to be solved later?
I'm not sure what you're suggesting here exactly, do you mean to add a type ID
scheme that's incompatible with clang, leaving everything else the same? If so,
what sort of scheme are you proposing?
It seems unfortunate to have a different scheme, but IIUC we expect all kernel
objects to be built with the same compiler.
Mark.
On Wed, Dec 13, 2023 at 05:01:07PM +0800, Wang wrote:
> On 2023/12/13 16:48, Dan Li wrote:
> > + Likun
> >
> > On Tue, 28 Mar 2023 at 06:18, Sami Tolvanen <[email protected]> wrote:
> >> On Mon, Mar 27, 2023 at 2:30 AM Peter Zijlstra <[email protected]> wrote:
> >>> On Sat, Mar 25, 2023 at 01:54:16AM -0700, Dan Li wrote:
> >>>
> >>>> In the compiler part[4], most of the content is the same as Sami's
> >>>> implementation[3], except for some minor differences, mainly including:
> >>>>
> >>>> 1. The function typeid is calculated differently and it is difficult
> >>>> to be consistent.
> >>> This means there is an effective ABI break between the compilers, which
> >>> is sad :-( Is there really nothing to be done about this?
> >> I agree, this would be unfortunate, and would also be a compatibility
> >> issue with rustc where there's ongoing work to support
> >> clang-compatible CFI type hashes:
> >>
> >> https://github.com/rust-lang/rust/pull/105452
> >>
> >> Sami
>
>
> Hi Peter and Sami
>
> I am Dan Li's colleague, and I will take over and continue the work of CFI.
Welcome; this is great news! :) Thanks for picking up the work.
>
> Regarding the issue of gcc cfi type id being compatible with clang, we
> have analyzed and verified:
>
> 1. clang uses Mangling defined in Itanium C++ ABI to encode the function
> prototype, and uses the encoding result as input to generate cfi type id;
> 2. Currently, gcc only implements mangling for the C++ compiler, and the
> function prototype coding generated by these interfaces is compatible
> with clang, but gcc's c compiler does not support mangling.;
>
> Adding mangling to gcc's c compiler is a huge and difficult task，because
> we have to refactor the mangling of C++, splitting it into basic
> mangling and language specific mangling, and adding support for the c
> language which requires a deep understanding of the compiler and
> language processing parts.
>
> And for the kernel cfi, I suggest separating type compatibility from CFI
> basic functions. Type compatibility is independent from CFI basic
> funcitons and should be dealt with under another topic. Should we focus
> on the main issus of cfi, and let it work first on linux kernel, and
> left the compatible issue to be solved later?
If you mean keeping the hashes identical between Clang/LLVM and GCC,
I think this is going to be a requirement due to adding Rust to the
build environment (which uses the LLVM mangling and hashing).
FWIW, I think the subset of type mangling needed isn't the entirely C++
language spec, so it shouldn't be hard to add this to GCC.
-Kees
--
Kees Cook