| Message ID | 20240409061043.3269676-3-debug@rivosinc.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC,01/12] riscv: zicfiss / zicfilp extension csr and bit definitions | expand |
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. Sami
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
On Thu, Apr 11, 2024 at 5:53 PM Deepak Gupta <debug@rivosinc.com> wrote: > > 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). I'm not saying the schemes have to be compatible, just that it would be great to avoid reinventing type annotations etc. For example, when you implement the fine-grained variant, you could simply override SYM_TYPED_ENTRY (defined in include/linux/cfi_types.h) to move __CFI_TYPE inside the function, and then redefine __CFI_TYPE to emit a landing pad with the correct label. This allows SYM_TYPED_* in assembly code to work with both KCFI and fine-grained Zicfilp. For the coarse-grained variant, your current macros are perfectly fine. > But yes in next version, I'll take a look and try to converge as much as > possible. Great, sounds good! Sami
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. + */ +#define SYM_FUNC_START(name) \ + SYM_START(name, SYM_L_GLOBAL, SYM_A_ALIGN) \ + lpad 0x1; + +#define SYM_FUNC_START_NOALIGN(name) \ + SYM_START(name, SYM_L_GLOBAL, SYM_A_NONE) \ + lpad 0x1; + +#define SYM_FUNC_START_LOCAL(name) \ + SYM_START(name, SYM_L_LOCAL, SYM_A_ALIGN) \ + lpad 0x1; + +#define SYM_FUNC_START_LOCAL_NOALIGN(name) \ + SYM_START(name, SYM_L_LOCAL, SYM_A_NONE) \ + lpad 0x1; + +#define SYM_FUNC_START_WEAK(name) \ + SYM_START(name, SYM_L_WEAK, SYM_A_ALIGN) \ + lpad 0x1; + +#define SYM_FUNC_START_WEAK_NOALIGN(name) \ + SYM_START(name, SYM_L_WEAK, SYM_A_NONE) \ + lpad 0x1; + +#define SYM_TYPED_FUNC_START(name) \ + SYM_TYPED_START(name, SYM_L_GLOBAL, SYM_A_ALIGN) \ + lpad 0x1; + +#endif + #endif /* _ASM_RISCV_LINKAGE_H */
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(+)