2025-03-14T14:39:38-07:00, Deepak Gupta <debug@xxxxxxxxxxxx>:
> Expose a new register type NT_RISCV_USER_CFI for risc-v cfi status and
> state. Intentionally both landing pad and shadow stack status and state
> are rolled into cfi state. Creating two different NT_RISCV_USER_XXX would
> not be useful and wastage of a note type. Enabling or disabling of feature
> is not allowed via ptrace set interface. However setting `elp` state or
> setting shadow stack pointer are allowed via ptrace set interface. It is
> expected `gdb` might have use to fixup `elp` state or `shadow stack`
> pointer.
>
> Signed-off-by: Deepak Gupta <debug@xxxxxxxxxxxx>
> ---
> arch/riscv/include/uapi/asm/ptrace.h | 18 ++++++++
> arch/riscv/kernel/ptrace.c | 83 ++++++++++++++++++++++++++++++++++++
> include/uapi/linux/elf.h | 1 +
> 3 files changed, 102 insertions(+)
>
> diff --git a/arch/riscv/include/uapi/asm/ptrace.h b/arch/riscv/include/uapi/asm/ptrace.h
> index 659ea3af5680..e6571fba8a8a 100644
> --- a/arch/riscv/include/uapi/asm/ptrace.h
> +++ b/arch/riscv/include/uapi/asm/ptrace.h
> @@ -131,6 +131,24 @@ struct __sc_riscv_cfi_state {
> unsigned long ss_ptr; /* shadow stack pointer */
> };
>
> +struct __cfi_status {
> + /* indirect branch tracking state */
> + __u64 lp_en : 1;
> + __u64 lp_lock : 1;
> + __u64 elp_state : 1;
> +
> + /* shadow stack status */
> + __u64 shstk_en : 1;
> + __u64 shstk_lock : 1;
I remember there was deep hatred towards bitfields in the Linux
community, have things changes?
> + __u64 rsvd : sizeof(__u64) - 5;
I think you meant "64 - 5".
> +};
> +
> +struct user_cfi_state {
> + struct __cfi_status cfi_status;
> + __u64 shstk_ptr;
> +};
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* _UAPI_ASM_RISCV_PTRACE_H */
> diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> @@ -224,6 +297,16 @@ static const struct user_regset riscv_user_regset[] = {
> .set = tagged_addr_ctrl_set,
> },
> #endif
> +#ifdef CONFIG_RISCV_USER_CFI
> + [REGSET_CFI] = {
> + .core_note_type = NT_RISCV_USER_CFI,
> + .align = sizeof(__u64),
> + .n = sizeof(struct user_cfi_state) / sizeof(__u64),
> + .size = sizeof(__u64),
Why not `size = sizeof(struct user_cfi_state)` and `n = 1`?
> + .regset_get = riscv_cfi_get,
> + .set = riscv_cfi_set,
> + },
> +#endif
[I haven't yet reviewed if a new register is the right thing to do nor
looked at the rest of the patch.]
