On Thu, Mar 20, 2025 at 3:24 PM Radim Krčmář rkrcmar@ventanamicro.com wrote:
2025-03-14T14:39:38-07:00, Deepak Gupta debug@rivosinc.com:
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@rivosinc.com
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?
hmm. I didn't know about the strong hatred. Although I can see lots of examples of this pattern in existing kernel code. No strong feelings on my side, I can change this and have it single 64bit field and accessed via bitmasks.
__u64 rsvd : sizeof(__u64) - 5;I think you meant "64 - 5".
eeh. bad bug. thanks.
+};
+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`?
yeah another good catch. Should write a kselftest against it, so that it can be caught.
.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.]