Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 10 Jul 2023 12:08:09 -0700 (PDT)
Subject:     Re: [PATCH v2 0/6] riscv: KCFI support
In-Reply-To: <20230710183544.999540-8-samitolvanen@google.com>
CC:     Paul Walmsley <paul.walmsley@sifive.com>, aou@eecs.berkeley.edu,
        keescook@chromium.org, nathan@kernel.org, ndesaulniers@google.com,
        Conor Dooley <conor.dooley@microchip.com>,
        linux-riscv@lists.infradead.org, llvm@lists.linux.dev,
        linux-kernel@vger.kernel.org, samitolvanen@google.com
From:   Palmer Dabbelt <palmer@dabbelt.com>
To:     samitolvanen@google.com
Message-ID: <mhng-2fe7822f-f869-4513-b8a6-7da21ce5ccd8@palmer-ri-x1c9a>
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Mon, 10 Jul 2023 11:35:45 PDT (-0700), samitolvanen@google.com 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.
>
> ---
>
> Changes in v2:
>   - Rebased on 6.5-rc1.
>   - Sorted Kconfig entries alphabetically.
>
>
> 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: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5

Thanks, this generally LGTM it was just a bit late for the last merge 
window.  I'm still poking around to make sure there's no fallout in rc1, 
this is in the queue (but there's some other stuff too).