This series adds KCFI support for RISC-V. KCFI is a fine-grained
forward-edge control-flow integrity scheme supported in Clang >=16,
which ensures indirect calls in instrumented code can only branch to
functions whose type matches the function pointer type, thus making
code reuse attacks more difficult.
Patch 1 implements a pt_regs based syscall wrapper to address
function pointer type mismatches in syscall handling. Patches 2 and 3
annotate indirectly called assembly functions with CFI types. Patch 4
implements error handling for indirect call checks. Patch 5 disables
CFI for arch/riscv/purgatory. Patch 6 finally allows CONFIG_CFI_CLANG
to be enabled for RISC-V.
Note that Clang 16 has a generic architecture-agnostic KCFI
implementation, which does work with the kernel, but doesn't produce
a stable code sequence for indirect call checks, which means
potential failures just trap and won't result in informative error
messages. Clang 17 includes a RISC-V specific back-end implementation
for KCFI, which emits a predictable code sequence for the checks and a
.kcfi_traps section with locations of the traps, which patch 5 uses to
produce more useful errors.
The type mismatch fixes and annotations in the first three patches
also become necessary in future if the kernel decides to support
fine-grained CFI implemented using the hardware landing pad
feature proposed in the in-progress Zicfisslp extension. Once the
specification is ratified and hardware support emerges, implementing
runtime patching support that replaces KCFI instrumentation with
Zicfisslp landing pads might also be feasible (similarly to KCFI to
FineIBT patching on x86_64), allowing distributions to ship a unified
kernel binary for all devices.
Sami Tolvanen (6):
riscv: Implement syscall wrappers
riscv: Add types to indirectly called assembly functions
riscv: Add ftrace_stub_graph
riscv: Add CFI error handling
riscv/purgatory: Disable CFI
riscv: Allow CONFIG_CFI_CLANG to be selected
arch/riscv/Kconfig | 3 +
arch/riscv/include/asm/cfi.h | 22 ++++++
arch/riscv/include/asm/insn.h | 10 +++
arch/riscv/include/asm/syscall.h | 5 +-
arch/riscv/include/asm/syscall_wrapper.h | 87 ++++++++++++++++++++++++
arch/riscv/kernel/Makefile | 2 +
arch/riscv/kernel/cfi.c | 77 +++++++++++++++++++++
arch/riscv/kernel/compat_syscall_table.c | 8 ++-
arch/riscv/kernel/mcount.S | 9 ++-
arch/riscv/kernel/suspend_entry.S | 5 +-
arch/riscv/kernel/sys_riscv.c | 6 ++
arch/riscv/kernel/syscall_table.c | 8 ++-
arch/riscv/kernel/traps.c | 4 +-
arch/riscv/purgatory/Makefile | 4 ++
14 files changed, 238 insertions(+), 12 deletions(-)
create mode 100644 arch/riscv/include/asm/cfi.h
create mode 100644 arch/riscv/include/asm/syscall_wrapper.h
create mode 100644 arch/riscv/kernel/cfi.c
base-commit: c6b0271053e7a5ae57511363213777f706b60489
--
2.41.0.255.g8b1d071c50-goog
Filter out CC_FLAGS_CFI when CONFIG_CFI_CLANG.
Signed-off-by: Sami Tolvanen <[email protected]>
---
arch/riscv/purgatory/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/riscv/purgatory/Makefile b/arch/riscv/purgatory/Makefile
index dc20e166983e..9e6476719abb 100644
--- a/arch/riscv/purgatory/Makefile
+++ b/arch/riscv/purgatory/Makefile
@@ -77,6 +77,10 @@ ifdef CONFIG_STACKPROTECTOR_STRONG
PURGATORY_CFLAGS_REMOVE += -fstack-protector-strong
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.41.0.255.g8b1d071c50-goog
Commit 883bbbffa5a4 ("ftrace,kcfi: Separate ftrace_stub() and
ftrace_stub_graph()") added a separate ftrace_stub_graph function for
CFI_CLANG. Add the stub to fix FUNCTION_GRAPH_TRACER compatibility
with CFI.
Signed-off-by: Sami Tolvanen <[email protected]>
---
arch/riscv/kernel/mcount.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
index 712c1d2c2723..c73d7514e45f 100644
--- a/arch/riscv/kernel/mcount.S
+++ b/arch/riscv/kernel/mcount.S
@@ -57,6 +57,10 @@ SYM_TYPED_FUNC_START(ftrace_stub)
SYM_FUNC_END(ftrace_stub)
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+SYM_TYPED_FUNC_START(ftrace_stub_graph)
+ ret
+SYM_FUNC_END(ftrace_stub_graph)
+
ENTRY(return_to_handler)
/*
* On implementing the frame point test, the ideal way is to compare the
--
2.41.0.255.g8b1d071c50-goog
On Thu, Jun 29, 2023 at 11:42:50PM +0000, Sami Tolvanen wrote:
> Filter out CC_FLAGS_CFI when CONFIG_CFI_CLANG.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Thu, Jun 29, 2023 at 11:42:48PM +0000, Sami Tolvanen wrote:
> Commit 883bbbffa5a4 ("ftrace,kcfi: Separate ftrace_stub() and
> ftrace_stub_graph()") added a separate ftrace_stub_graph function for
> CFI_CLANG. Add the stub to fix FUNCTION_GRAPH_TRACER compatibility
> with CFI.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
Hey Sami,
On Thu, Jun 29, 2023 at 11:42:45PM +0000, Sami Tolvanen wrote:
> This series adds KCFI support for RISC-V. KCFI is a fine-grained
> forward-edge control-flow integrity scheme supported in Clang >=16,
> which ensures indirect calls in instrumented code can only branch to
> functions whose type matches the function pointer type, thus making
> code reuse attacks more difficult.
> base-commit: c6b0271053e7a5ae57511363213777f706b60489
Could you please rebase this on top of v6.5-rc1 when that comes out?
This base-commit is some random commit from Linus' tree, that because we
are currently in the merge window has is not in the RISC-V trees yet,
and means the series wasn't applied by our CI stuff.
Cheers,
Conor.
On Fri, Jun 30, 2023 at 07:48:23PM +0100, Conor Dooley wrote:
> Hey Sami,
>
> On Thu, Jun 29, 2023 at 11:42:45PM +0000, Sami Tolvanen wrote:
> > This series adds KCFI support for RISC-V. KCFI is a fine-grained
> > forward-edge control-flow integrity scheme supported in Clang >=16,
> > which ensures indirect calls in instrumented code can only branch to
> > functions whose type matches the function pointer type, thus making
> > code reuse attacks more difficult.
>
> > base-commit: c6b0271053e7a5ae57511363213777f706b60489
>
> Could you please rebase this on top of v6.5-rc1 when that comes out?
> This base-commit is some random commit from Linus' tree, that because we
> are currently in the merge window has is not in the RISC-V trees yet,
> and means the series wasn't applied by our CI stuff.
In other news, I gave it a go with 03b118c7e456 ("[SLP] Fix crash on
attempt to access on invalid iterator state.") & have been running it
for a bit. All seems in order so far, nice :)
Hi Sami,
On Thu, Jun 29, 2023 at 11:42:45PM +0000, Sami Tolvanen wrote:
> This series adds KCFI support for RISC-V. KCFI is a fine-grained
> forward-edge control-flow integrity scheme supported in Clang >=16,
> which ensures indirect calls in instrumented code can only branch to
> functions whose type matches the function pointer type, thus making
> code reuse attacks more difficult.
>
> Patch 1 implements a pt_regs based syscall wrapper to address
> function pointer type mismatches in syscall handling. Patches 2 and 3
> annotate indirectly called assembly functions with CFI types. Patch 4
> implements error handling for indirect call checks. Patch 5 disables
> CFI for arch/riscv/purgatory. Patch 6 finally allows CONFIG_CFI_CLANG
> to be enabled for RISC-V.
>
> Note that Clang 16 has a generic architecture-agnostic KCFI
> implementation, which does work with the kernel, but doesn't produce
> a stable code sequence for indirect call checks, which means
> potential failures just trap and won't result in informative error
> messages. Clang 17 includes a RISC-V specific back-end implementation
> for KCFI, which emits a predictable code sequence for the checks and a
> .kcfi_traps section with locations of the traps, which patch 5 uses to
> produce more useful errors.
>
> The type mismatch fixes and annotations in the first three patches
> also become necessary in future if the kernel decides to support
> fine-grained CFI implemented using the hardware landing pad
> feature proposed in the in-progress Zicfisslp extension. Once the
> specification is ratified and hardware support emerges, implementing
> runtime patching support that replaces KCFI instrumentation with
> Zicfisslp landing pads might also be feasible (similarly to KCFI to
> FineIBT patching on x86_64), allowing distributions to ship a unified
> kernel binary for all devices.
I boot tested ARCH=riscv defconfig + CONFIG_CFI_CLANG=y with both clang
16.0.6 and a recent LLVM 17.0.0 from tip of tree and saw no issues while
booting. I can confirm that both kernels panic when running the
CFI_FORWARD_PROTO LKDTM test.
LLVM 17.0.0:
[ 100.722815] lkdtm: Performing direct entry CFI_FORWARD_PROTO
[ 100.723061] lkdtm: Calling matched prototype ...
[ 100.723217] lkdtm: Calling mismatched prototype ...
[ 100.723861] CFI failure at lkdtm_indirect_call+0x22/0x32 (target: lkdtm_increment_int+0x0/0x18; expected type: 0x3ad55aca)
[ 100.724191] Kernel BUG [#1]
[ 100.724226] Modules linked in:
[ 100.724343] CPU: 0 PID: 42 Comm: sh Not tainted 6.4.0-08887-ga68cded684a2 #1
[ 100.724450] Hardware name: riscv-virtio,qemu (DT)
[ 100.724552] epc : lkdtm_indirect_call+0x22/0x32
[ 100.724586] ra : lkdtm_CFI_FORWARD_PROTO+0x40/0x74
[ 100.724603] epc : ffffffff805ee84c ra : ffffffff805ee6de sp : ff200000001a3cb0
[ 100.724617] gp : ffffffff8130ab70 tp : ff60000001b9d240 t0 : ff200000001a3b38
[ 100.724631] t1 : 000000003ad55aca t2 : 000000007e0c52a5 s0 : ff200000001a3cc0
[ 100.724644] s1 : 0000000000000001 a0 : ffffffff8130edc8 a1 : ffffffff805ee876
[ 100.724658] a2 : b5352d9a12ee0700 a3 : ffffffff8122e5c8 a4 : 0000000000000fff
[ 100.724671] a5 : 0000000000000004 a6 : 00000000000000b4 a7 : 0000000000000000
[ 100.724683] s2 : ff200000001a3e38 s3 : ffffffffffffffea s4 : 0000000000000012
[ 100.724696] s5 : ff6000000804c000 s6 : 0000000000000006 s7 : ffffffff80e8ca88
[ 100.724709] s8 : 0000000000000008 s9 : 0000000000000002 s10: ffffffff812bfd10
[ 100.724722] s11: ffffffff812bfd10 t3 : 0000000000000003 t4 : 0000000000000000
[ 100.724735] t5 : ff60000001858000 t6 : ff60000001858f00
[ 100.724746] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
[ 100.724825] [<ffffffff805ee84c>] lkdtm_indirect_call+0x22/0x32
[ 100.724886] [<ffffffff805ee6de>] lkdtm_CFI_FORWARD_PROTO+0x40/0x74
[ 100.724898] [<ffffffff805eabbe>] lkdtm_do_action+0x22/0x32
[ 100.724908] [<ffffffff805eab78>] direct_entry+0x124/0x136
[ 100.724918] [<ffffffff8034af5a>] full_proxy_write+0x58/0xb2
[ 100.724930] [<ffffffff801e139e>] vfs_write+0x14c/0x350
[ 100.724941] [<ffffffff801e16fc>] ksys_write+0x64/0xd4
[ 100.724951] [<ffffffff801e1782>] __riscv_sys_write+0x16/0x22
[ 100.724961] [<ffffffff80005cec>] syscall_handler+0x4c/0x58
[ 100.724973] [<ffffffff809355ac>] do_trap_ecall_u+0x3e/0x88
[ 100.724996] [<ffffffff80003678>] ret_from_exception+0x0/0x64
[ 100.725150] Code: 0513 5945 a303 ffc5 53b7 7e0c 839b 2a53 0363 0073 (9002) 9582
[ 100.731204] ---[ end trace 0000000000000000 ]---
[ 100.731327] Kernel panic - not syncing: Fatal exception in interrupt
[ 100.731910] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---
LLVM 16.0.6:
[ 10.227530] lkdtm: Performing direct entry CFI_FORWARD_PROTO
[ 10.227755] lkdtm: Calling matched prototype ...
[ 10.227900] lkdtm: Calling mismatched prototype ...
[ 10.228721] Oops - illegal instruction [#1]
[ 10.228856] Modules linked in:
[ 10.228978] CPU: 0 PID: 1 Comm: sh Not tainted 6.4.0-08887-ga68cded684a2 #1
[ 10.229077] Hardware name: riscv-virtio,qemu (DT)
[ 10.229160] epc : lkdtm_indirect_call+0x2c/0x32
[ 10.229242] ra : lkdtm_CFI_FORWARD_PROTO+0x40/0x74
[ 10.229259] epc : ffffffff805ef190 ra : ffffffff805ef018 sp : ff2000000000bcb0
[ 10.229272] gp : ffffffff8130a958 tp : ff600000018c8000 t0 : ff2000000000bb38
[ 10.229285] t1 : ff2000000000baa8 t2 : 0000000000000018 s0 : ff2000000000bcc0
[ 10.229298] s1 : 0000000000000001 a0 : 000000003ad55aca a1 : ffffffff805ef1b0
[ 10.229310] a2 : 000000007e0c52a5 a3 : ffffffff8122e548 a4 : 0000000000000fff
[ 10.229322] a5 : 0000000000000004 a6 : 00000000000000b4 a7 : 0000000000000000
[ 10.229335] s2 : ff2000000000be38 s3 : ffffffffffffffea s4 : 0000000000000012
[ 10.229347] s5 : ff6000000802f000 s6 : 0000000000000006 s7 : ffffffff80e8ca88
[ 10.229360] s8 : 0000000000000008 s9 : 0000000000000002 s10: ffffffff812bfc90
[ 10.229372] s11: ffffffff812bfc90 t3 : 0000000000000003 t4 : 0000000000000000
[ 10.229385] t5 : ff60000001858000 t6 : ff60000001858f00
[ 10.229396] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000002
[ 10.229478] [<ffffffff805ef190>] lkdtm_indirect_call+0x2c/0x32
[ 10.229538] [<ffffffff805ef018>] lkdtm_CFI_FORWARD_PROTO+0x40/0x74
[ 10.229550] [<ffffffff805eb4d4>] lkdtm_do_action+0x20/0x34
[ 10.229560] [<ffffffff805eb490>] direct_entry+0x124/0x136
[ 10.229570] [<ffffffff80349cf0>] full_proxy_write+0x56/0xb2
[ 10.229582] [<ffffffff801e0620>] vfs_write+0x14a/0x34e
[ 10.229593] [<ffffffff801e097e>] ksys_write+0x64/0xd4
[ 10.229602] [<ffffffff801e0a04>] __riscv_sys_write+0x16/0x22
[ 10.229611] [<ffffffff800056fe>] syscall_handler+0x4a/0x58
[ 10.229622] [<ffffffff80936428>] do_trap_ecall_u+0x3e/0x88
[ 10.229649] [<ffffffff80003678>] ret_from_exception+0x0/0x64
[ 10.229860] Code: 00c5 1517 00d2 0513 c4a5 9582 60a2 6402 0141 8082 (0000) 52a5
[ 10.235769] ---[ end trace 0000000000000000 ]---
[ 10.235892] Kernel panic - not syncing: Fatal exception in interrupt
[ 10.236488] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---
Tested-by: Nathan Chancellor <[email protected]>
Cheers,
Nathan
Hi Conor,
On Fri, Jun 30, 2023 at 11:48 AM Conor Dooley <[email protected]> wrote:
>
> Hey Sami,
>
> On Thu, Jun 29, 2023 at 11:42:45PM +0000, Sami Tolvanen wrote:
> > This series adds KCFI support for RISC-V. KCFI is a fine-grained
> > forward-edge control-flow integrity scheme supported in Clang >=16,
> > which ensures indirect calls in instrumented code can only branch to
> > functions whose type matches the function pointer type, thus making
> > code reuse attacks more difficult.
>
> > base-commit: c6b0271053e7a5ae57511363213777f706b60489
>
> Could you please rebase this on top of v6.5-rc1 when that comes out?
> This base-commit is some random commit from Linus' tree, that because we
> are currently in the merge window has is not in the RISC-V trees yet,
> and means the series wasn't applied by our CI stuff.
Sure, I'll send v2 rebased on top of -rc1 once it's out. The random
commit was the ToT at the time this series was sent out.
Sami