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

Alexandre Ghiti
2025-04-08T12:45:34Z

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.

> +
> # 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
>