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 <[email protected]>
---
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.
+ */
+#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 */
--
2.43.2
On Tue, Apr 9, 2024 at 6:12 AM Deepak Gupta <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]> 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 <[email protected]>
>> ---
>> 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 <[email protected]> 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