Re: [PATCH v12 24/28] arch/riscv: compile vdso with landing pad

Deepak Gupta
2025-04-09T14:58:29Z

On Tue, Apr 08, 2025 at 02:45:25PM +0200, Alexandre Ghiti wrote:
>
>On 14/03/2025 22:39, Deepak Gupta wrote:
>>From: Jim Shu <jim.shu@sifive.com>
>>
>>user mode tasks compiled with zicfilp may call indirectly into vdso (like
>>hwprobe indirect calls). Add landing pad compile support in vdso. vdso
>>with landing pad in it will be nop for tasks which have not enabled
>>landing pad.
>>This patch allows to run user mode tasks with cfi eanbled and do no harm.
>>
>>Future work can be done on this to do below
>> - labeled landing pad on vdso functions (whenever labeling support shows
>> up in gnu-toolchain)
>> - emit shadow stack instructions only in vdso compiled objects as part of
>> kernel compile.
>>
>>Signed-off-by: Jim Shu <jim.shu@sifive.com>
>>Reviewed-by: Zong Li <zong.li@sifive.com>
>>Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>>---
>> arch/riscv/Makefile | 5 +++-
>> arch/riscv/include/asm/assembler.h | 44 +++++++++++++++++++++++++++++++++++
>> arch/riscv/kernel/vdso/Makefile | 12 ++++++++++
>> arch/riscv/kernel/vdso/flush_icache.S | 4 ++++
>> arch/riscv/kernel/vdso/getcpu.S | 4 ++++
>> arch/riscv/kernel/vdso/rt_sigreturn.S | 4 ++++
>> arch/riscv/kernel/vdso/sys_hwprobe.S | 4 ++++
>> 7 files changed, 76 insertions(+), 1 deletion(-)
>>
>>diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
>>index 13fbc0f94238..eca94246cda6 100644
>>--- a/arch/riscv/Makefile
>>+++ b/arch/riscv/Makefile
>>@@ -88,9 +88,12 @@ riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZACAS) := $(riscv-march-y)_zacas
>> # Check if the toolchain supports Zabha
>> riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZABHA) := $(riscv-march-y)_zabha
>>+KBUILD_BASE_ISA = -march=$(shell echo $(riscv-march-y) | sed -E 's/(rv32ima|rv64ima)fd([^v_]*)v?/\1\2/')
>>+export KBUILD_BASE_ISA
>>+
>> # Remove F,D,V from isa string for all. Keep extensions between "fd" and "v" by
>> # matching non-v and non-multi-letter extensions out with the filter ([^v_]*)
>>-KBUILD_CFLAGS += -march=$(shell echo $(riscv-march-y) | sed -E 's/(rv32ima|rv64ima)fd([^v_]*)v?/\1\2/')
>>+KBUILD_CFLAGS += $(KBUILD_BASE_ISA)
>> KBUILD_AFLAGS += -march=$(riscv-march-y)
>>diff --git a/arch/riscv/include/asm/assembler.h b/arch/riscv/include/asm/assembler.h
>>index 44b1457d3e95..a058ea5e9c58 100644
>>--- a/arch/riscv/include/asm/assembler.h
>>+++ b/arch/riscv/include/asm/assembler.h
>>@@ -80,3 +80,47 @@
>> .endm
>> #endif /* __ASM_ASSEMBLER_H */
>>+
>>+#if defined(CONFIG_RISCV_USER_CFI) && (__riscv_xlen == 64)
>>+.macro vdso_lpad
>>+lpad 0
>>+.endm
>>+#else
>>+.macro vdso_lpad
>>+.endm
>>+#endif
>>+
>>+/*
>>+ * This macro emits a program property note section identifying
>>+ * architecture features which require special handling, mainly for
>>+ * use in assembly files included in the VDSO.
>>+ */
>>+#define NT_GNU_PROPERTY_TYPE_0 5
>>+#define GNU_PROPERTY_RISCV_FEATURE_1_AND 0xc0000000
>>+
>>+#define GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP (1U << 0)
>>+#define GNU_PROPERTY_RISCV_FEATURE_1_ZICFISS (1U << 1)
>>+
>>+#if defined(CONFIG_RISCV_USER_CFI) && (__riscv_xlen == 64)
>>+#define GNU_PROPERTY_RISCV_FEATURE_1_DEFAULT \
>>+ (GNU_PROPERTY_RISCV_FEATURE_1_ZICFILP)
>>+#endif
>>+
>>+#ifdef GNU_PROPERTY_RISCV_FEATURE_1_DEFAULT
>>+.macro emit_riscv_feature_1_and, feat = GNU_PROPERTY_RISCV_FEATURE_1_DEFAULT
>>+ .pushsection .note.gnu.property, "a"
>>+ .p2align 3
>>+ .word 4
>>+ .word 16
>>+ .word NT_GNU_PROPERTY_TYPE_0
>>+ .asciz "GNU"
>>+ .word GNU_PROPERTY_RISCV_FEATURE_1_AND
>>+ .word 4
>>+ .word \feat
>>+ .word 0
>>+ .popsection
>>+.endm
>>+#else
>>+.macro emit_riscv_feature_1_and, feat = 0
>>+.endm
>>+#endif
>>diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
>>index 9a1b555e8733..daa10c2b0dd1 100644
>>--- a/arch/riscv/kernel/vdso/Makefile
>>+++ b/arch/riscv/kernel/vdso/Makefile
>>@@ -13,12 +13,18 @@ vdso-syms += flush_icache
>> vdso-syms += hwprobe
>> vdso-syms += sys_hwprobe
>>+ifdef CONFIG_RISCV_USER_CFI
>>+LPAD_MARCH = _zicfilp
>>+endif
>>+
>> # Files to link into the vdso
>> obj-vdso = $(patsubst %, %.o, $(vdso-syms)) note.o
>> ccflags-y := -fno-stack-protector
>> ccflags-y += -DDISABLE_BRANCH_PROFILING
>> ccflags-y += -fno-builtin
>>+ccflags-y += $(KBUILD_BASE_ISA)$(LPAD_MARCH)
>>+asflags-y += $(KBUILD_BASE_ISA)$(LPAD_MARCH)
>> ifneq ($(c-gettimeofday-y),)
>> CFLAGS_vgettimeofday.o += -fPIC -include $(c-gettimeofday-y)
>>@@ -40,6 +46,12 @@ endif
>> CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
>> CFLAGS_REMOVE_hwprobe.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
>>+# Disable profiling and instrumentation for VDSO code
>>+GCOV_PROFILE := n
>>+KCOV_INSTRUMENT := n
>>+KASAN_SANITIZE := n
>>+UBSAN_SANITIZE := n
>
>
>So I looked into this and it seems it is not needed. All the profilers
>above only add their corresponding "clags" if the object
>"is-kernel-object" (https://elixir.bootlin.com/linux/v6.14-rc6/source/scripts/Makefile.lib#L147)
>which is not the case for hwprobe.c since it is not added to obj-y.

Noted. Will remove it.

>
>
>>+
>> # Force dependency
>> $(obj)/vdso.o: $(obj)/vdso.so
>>diff --git a/arch/riscv/kernel/vdso/flush_icache.S b/arch/riscv/kernel/vdso/flush_icache.S
>>index 8f884227e8bc..e4c56970905e 100644
>>--- a/arch/riscv/kernel/vdso/flush_icache.S
>>+++ b/arch/riscv/kernel/vdso/flush_icache.S
>>@@ -5,11 +5,13 @@
>> #include <linux/linkage.h>
>> #include <asm/unistd.h>
>>+#include <asm/assembler.h>
>> .text
>> /* int __vdso_flush_icache(void *start, void *end, unsigned long flags); */
>> SYM_FUNC_START(__vdso_flush_icache)
>> .cfi_startproc
>>+ vdso_lpad
>> #ifdef CONFIG_SMP
>> li a7, __NR_riscv_flush_icache
>> ecall
>>@@ -20,3 +22,5 @@ SYM_FUNC_START(__vdso_flush_icache)
>> ret
>> .cfi_endproc
>> SYM_FUNC_END(__vdso_flush_icache)
>>+
>>+emit_riscv_feature_1_and
>>diff --git a/arch/riscv/kernel/vdso/getcpu.S b/arch/riscv/kernel/vdso/getcpu.S
>>index 9c1bd531907f..5c1ecc4e1465 100644
>>--- a/arch/riscv/kernel/vdso/getcpu.S
>>+++ b/arch/riscv/kernel/vdso/getcpu.S
>>@@ -5,14 +5,18 @@
>> #include <linux/linkage.h>
>> #include <asm/unistd.h>
>>+#include <asm/assembler.h>
>> .text
>> /* int __vdso_getcpu(unsigned *cpu, unsigned *node, void *unused); */
>> SYM_FUNC_START(__vdso_getcpu)
>> .cfi_startproc
>>+ vdso_lpad
>> /* For now, just do the syscall. */
>> li a7, __NR_getcpu
>> ecall
>> ret
>> .cfi_endproc
>> SYM_FUNC_END(__vdso_getcpu)
>>+
>>+emit_riscv_feature_1_and
>>diff --git a/arch/riscv/kernel/vdso/rt_sigreturn.S b/arch/riscv/kernel/vdso/rt_sigreturn.S
>>index 3dc022aa8931..e82987dc3739 100644
>>--- a/arch/riscv/kernel/vdso/rt_sigreturn.S
>>+++ b/arch/riscv/kernel/vdso/rt_sigreturn.S
>>@@ -5,12 +5,16 @@
>> #include <linux/linkage.h>
>> #include <asm/unistd.h>
>>+#include <asm/assembler.h>
>> .text
>> SYM_FUNC_START(__vdso_rt_sigreturn)
>> .cfi_startproc
>> .cfi_signal_frame
>>+ vdso_lpad
>> li a7, __NR_rt_sigreturn
>> ecall
>> .cfi_endproc
>> SYM_FUNC_END(__vdso_rt_sigreturn)
>>+
>>+emit_riscv_feature_1_and
>>diff --git a/arch/riscv/kernel/vdso/sys_hwprobe.S b/arch/riscv/kernel/vdso/sys_hwprobe.S
>>index 77e57f830521..f1694451a60c 100644
>>--- a/arch/riscv/kernel/vdso/sys_hwprobe.S
>>+++ b/arch/riscv/kernel/vdso/sys_hwprobe.S
>>@@ -3,13 +3,17 @@
>> #include <linux/linkage.h>
>> #include <asm/unistd.h>
>>+#include <asm/assembler.h>
>> .text
>> SYM_FUNC_START(riscv_hwprobe)
>> .cfi_startproc
>>+ vdso_lpad
>> li a7, __NR_riscv_hwprobe
>> ecall
>> ret
>> .cfi_endproc
>> SYM_FUNC_END(riscv_hwprobe)
>>+
>>+emit_riscv_feature_1_and
>>