Re: [PATCH v2 6/7] riscv: Add RISC-V Kernel Control Flow Integrity implementation

[Jeff Law <jeffreyalaw@gmail.com>]

On 9/4/25 18:24, Kees Cook wrote:
> Implement RISC-V-specific KCFI backend.
> 
> - Function preamble generation using .word directives for type ID storage
>    at offset from function entry point (no alignment NOPs needed due to
>    fix 4-byte instruction size).
> 
> - Scratch register allocation using t1/t2 (x6/x7) following RISC-V
>    procedure call standard for temporary registers.
> 
> - Integration with .kcfi_traps section for debugger/runtime metadata
>    (like x86_64).
> 
> Assembly Code Pattern for RISC-V:
>    lw      t1, -4(target_reg)         ; Load actual type ID from preamble
>    lui     t2, %hi(expected_type)     ; Load expected type (upper 20 bits)
>    addiw   t2, t2, %lo(expected_type) ; Add lower 12 bits (sign-extended)
>    beq     t1, t2, .Lkcfi_call        ; Branch if types match
>    .Lkcfi_trap: ebreak                ; Environment break trap on mismatch
>    .Lkcfi_call: jalr/jr target_reg    ; Execute validated indirect transfer
> 
> Build and run tested with Linux kernel ARCH=riscv.
> 
> gcc/ChangeLog:
> 
> 	config/riscv/riscv-protos.h: Declare KCFI helpers.
> 	config/riscv/riscv.cc (riscv_maybe_wrap_call_with_kcfi): New
> 	function, to wrap calls.
> 	(riscv_maybe_wrap_call_value_with_kcfi): New function, to
> 	wrap calls with return values.
> 	(riscv_output_kcfi_insn): New function to emit KCFI assembly.
> 	config/riscv/riscv.md: Add KCFI RTL patterns and hook expansion.
> 	doc/invoke.texi: Document riscv nuances.
> 
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
>   gcc/config/riscv/riscv-protos.h |   3 +
>   gcc/config/riscv/riscv.cc       | 147 ++++++++++++++++++++++++++++++++
>   gcc/config/riscv/riscv.md       |  74 ++++++++++++++--
>   gcc/doc/invoke.texi             |  13 +++
>   4 files changed, 231 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index 2d60a0ad44b3..0e916fbdde13 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -126,6 +126,9 @@ extern bool riscv_split_64bit_move_p (rtx, rtx);
>   extern void riscv_split_doubleword_move (rtx, rtx);
>   extern const char *riscv_output_move (rtx, rtx);
>   extern const char *riscv_output_return ();
> +extern rtx riscv_maybe_wrap_call_with_kcfi (rtx, rtx);
> +extern rtx riscv_maybe_wrap_call_value_with_kcfi (rtx, rtx);
> +extern const char *riscv_output_kcfi_insn (rtx_insn *, rtx *);
>   extern void riscv_declare_function_name (FILE *, const char *, tree);
>   extern void riscv_declare_function_size (FILE *, const char *, tree);
>   extern void riscv_asm_output_alias (FILE *, const tree, const tree);

> @@ -11346,6 +11347,149 @@ riscv_convert_vector_chunks (struct gcc_options *opts)
>       return 1;
>   }
>   
> +/* Apply KCFI wrapping to call pattern if needed.  */
> +rtx
> +riscv_maybe_wrap_call_with_kcfi (rtx pat, rtx addr)
So our coding standards require a bit more for that function comment. 
What are PAT and ADDR and how are they used?



> +}
> +
> +/* Apply KCFI wrapping to call_value pattern if needed.  */
> +rtx
> +riscv_maybe_wrap_call_value_with_kcfi (rtx pat, rtx addr)
Similarly here.


> +
> +/* Output the assembly for a KCFI checked call instruction.  */
> +const char *
> +riscv_output_kcfi_insn (rtx_insn *insn, rtx *operands)
And here.


> +{
> +  /* Target register.  */
> +  rtx target_reg = operands[0];
> +  gcc_assert (REG_P (target_reg));
> +
> +  /* Get KCFI type ID.  */
> +  uint32_t expected_type = (uint32_t) INTVAL (operands[3]);
Do we know operands[3] is a CONST_INT?

> +
> +  /* Calculate typeid offset from call target.  */
> +  HOST_WIDE_INT offset = -(4 + kcfi_patchable_entry_prefix_nops);
> +
> +  /* Choose scratch registers that don't conflict with target.  */
> +  unsigned temp1_regnum = T1_REGNUM;
> +  unsigned temp2_regnum = T2_REGNUM;
ISTM that this will need some kidn of adjustment if someone were to 
compile with -ffixed-reg.  Maybe all we really need is a sorry() so that 
if someone were to try to fix the temporary registers they'd get a loud 
complaint from teh compiler.


> +
> +  /* Load actual type from memory at offset.  */
> +  temp_operands[0] = gen_rtx_REG (SImode, temp1_regnum);
> +  temp_operands[1] = gen_rtx_MEM (SImode,
> +                      gen_rtx_PLUS (DImode, target_reg,
> +                                   GEN_INT (offset)));
> +  output_asm_insn ("lw\t%0, %1", temp_operands);
Rather than using DImode for the PLUS, shouldn't it instead use Pmode so 
that it at least tries to work on rv32?  Or is this stuff somehow 
defined as only working for rv64?



> +
> +  /* Execute the indirect call.  */
> +  if (SIBLING_CALL_P (insn))
> +    {
> +      /* Tail call uses x0 (zero register) to avoid saving return address.  */
> +      temp_operands[0] = gen_rtx_REG (DImode, 0);  /* x0 */
> +      temp_operands[1] = target_reg;  /* target register */
> +      temp_operands[2] = const0_rtx;
> +      output_asm_insn ("jalr\t%0, %1, %2", temp_operands);
> +    }
> +  else
> +    {
> +      /* Regular call uses x1 (return address register).  */
> +      temp_operands[0] = gen_rtx_REG (DImode, RETURN_ADDR_REGNUM);  /* x1 */
> +      temp_operands[1] = target_reg;  /* target register */
> +      temp_operands[2] = const0_rtx;
> +      output_asm_insn ("jalr\t%0, %1, %2", temp_operands);
> +    }
More cases where we probably should be using Pmode.

We generally prefer to not generate assembly code like you've done, but 
instead prefer to generate actual RTL.  Is there some reason why you 
decided to use output_asm_insn rather than generating RTL and letting 
usual mechanisms for generating assembly code kick in?


>     rtx target = riscv_legitimize_call_address (XEXP (operands[0], 0));
> -  emit_call_insn (gen_sibcall_internal (target, operands[1], operands[2]));
> +  rtx pat = gen_sibcall_internal (target, operands[1], operands[2]);
> +  pat = riscv_maybe_wrap_call_with_kcfi (pat, target);
> +  emit_call_insn (pat);
>     DONE;
>   })
>   
> +;; KCFI sibling call - matches KCFI wrapper RTL
> +(define_insn "*kcfi_sibcall_insn"
> +  [(kcfi (call (mem:SI (match_operand:DI 0 "call_insn_operand" "l"))
> +               (match_operand 1 ""))
> +         (match_operand 3 "const_int_operand"))
> +   (use (unspec:SI [(match_operand 2 "const_int_operand")] UNSPEC_CALLEE_CC))]
> +  "SIBLING_CALL_P (insn)"
I think the DI for the memory operand should probably be :P instead so 
that we're not so tied to rv64.

> +;; KCFI sibling call with return value - matches KCFI wrapper RTL
> +(define_insn "*kcfi_sibcall_value_insn"
> +  [(set (match_operand 0 "")
> +	(kcfi (call (mem:SI (match_operand:DI 1 "call_insn_operand" "l"))
> +		    (match_operand 2 ""))
> +	      (match_operand 4 "const_int_operand")))
> +   (use (unspec:SI [(match_operand 3 "const_int_operand")] UNSPEC_CALLEE_CC))]
> +  "SIBLING_CALL_P (insn)"
> +{
> +  return riscv_output_kcfi_insn (insn, &operands[1]);
> +}
> +  [(set_attr "type" "call")
> +   (set_attr "length" "24")])
Similarly for this pattern.



>   
> +;; KCFI indirect call - matches KCFI wrapper RTL
> +(define_insn "*kcfi_call_internal"
> +  [(kcfi (call (mem:SI (match_operand:DI 0 "call_insn_operand" "l"))
> +               (match_operand 1 "" ""))
> +         (match_operand 3 "const_int_operand"))
> +   (use (unspec:SI [(match_operand 2 "const_int_operand")] UNSPEC_CALLEE_CC))
> +   (clobber (reg:SI RETURN_ADDR_REGNUM))]
> +  "!SIBLING_CALL_P (insn)"
> +{
> +  return riscv_output_kcfi_insn (insn, operands);
> +}
> +  [(set_attr "type" "call")
> +   (set_attr "length" "24")])
And this one.


>   
> +;; KCFI call with return value - matches KCFI wrapper RTL
> +(define_insn "*kcfi_call_value_insn"
> +  [(set (match_operand 0 "" "")
> +	(kcfi (call (mem:SI (match_operand:DI 1 "call_insn_operand" "l"))
> +		    (match_operand 2 "" ""))
> +	      (match_operand 4 "const_int_operand")))
> +   (use (unspec:SI [(match_operand 3 "const_int_operand")] UNSPEC_CALLEE_CC))
> +   (clobber (reg:SI RETURN_ADDR_REGNUM))]
> +  "!SIBLING_CALL_P (insn)"
> +{
> +  return riscv_output_kcfi_insn (insn, &operands[1]);
> +}
> +  [(set_attr "type" "call")
> +   (set_attr "length" "24")])
THis one too.


64).
>   
> +On RISC-V, KCFI type identifiers are emitted as a @code{.word ID}
> +directive (a 32-bit constant) before the function entry, similar to AArch64.
> +RISC-V's natural 4-byte instruction alignment eliminates the need for
> +additional padding NOPs.  When used with @option{-fpatchable-function-entry},
> +the type identifier is placed before any patchable NOPs. 
Note that many designs implement the "C" extension and as a result only 
have a 2 byte alignment for instructions.


Jeff