Re: [RFC PATCH 02/12] riscv: add landing pad for asm routines.

[Deepak Gupta <debug@rivosinc.com>]

On Thu, Apr 11, 2024 at 05:15:17PM +0000, Sami Tolvanen wrote:
>On Tue, Apr 9, 2024 at 6:12 AM Deepak Gupta <debug@rivosinc.com> wrote:
>>
>> SYM_* macros are used to define assembly routines. In this patch series,
>> re-define those macros in risc-v arch specific include file to include
>> a landing pad instruction at the beginning. This is done only when the
>> compiler flag for landing pad is enabled (i.e. __riscv_zicfilp).
>>
>> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>> ---
>>  arch/riscv/include/asm/linkage.h | 42 ++++++++++++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>
>> diff --git a/arch/riscv/include/asm/linkage.h b/arch/riscv/include/asm/linkage.h
>> index 9e88ba23cd2b..bb43ae7dadeb 100644
>> --- a/arch/riscv/include/asm/linkage.h
>> +++ b/arch/riscv/include/asm/linkage.h
>> @@ -6,7 +6,49 @@
>>  #ifndef _ASM_RISCV_LINKAGE_H
>>  #define _ASM_RISCV_LINKAGE_H
>>
>> +#ifdef __ASSEMBLY__
>> +#include <asm/assembler.h>
>> +#endif
>> +
>>  #define __ALIGN                .balign 4
>>  #define __ALIGN_STR    ".balign 4"
>>
>> +#ifdef __riscv_zicfilp
>> +/*
>> + * A landing pad instruction is needed at start of asm routines
>> + * re-define macros for asm routines to have a landing pad at
>> + * the beginning of function. Currently use label value of 0x1.
>> + * Eventually, label should be calculated as a hash over function
>> + * signature.
>> + */
>
>I haven't seen the compiler implementation for fine-grained Zicfilp
>yet, but in the kernel at least, this would ideally reuse as much of
>the KCFI plumbing as possible. For example, since only C code has type
>information, we left the type hash computation for the compiler, which
>allows assembly functions to just reference the appropriate
>__kcfi_typeid_* symbol.

Fine-grained compiler support hasn't made it in yet.

For reference, compiler that I've been using
https://github.com/sifive/riscv-gnu-toolchain/tree/cfi-dev

Honestly speaking, I didn't realize that kcfi plumbing has made it into
riscv as well. I realized that just after sending the patches.

In principle, I agree it should converge with software based kcfi scheme
as much as possible. However blocker that I see is `hash` is placed just
before function. This breaks for code mapped as execute only scenarios.
And ideally would like to have immediates at callsites instead of loads
(purely perf reason and not security).

But yes in next version, I'll take a look and try to converge as much as
possible.

>
>Sami