| Message ID | 20240403234054.2020347-23-debug@rivosinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv control-flow integrity for usermode | expand |
Hi Deepak, On Thu, Apr 4, 2024 at 7:42 AM Deepak Gupta <debug@rivosinc.com> wrote: > > Shadow stack needs to be saved and restored on signal delivery and signal > return. > > sigcontext embedded in ucontext is extendible. Adding cfi state in there > which can be used to save cfi state before signal delivery and restore > cfi state on sigreturn > > Signed-off-by: Deepak Gupta <debug@rivosinc.com> > --- > arch/riscv/include/uapi/asm/sigcontext.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/riscv/include/uapi/asm/sigcontext.h b/arch/riscv/include/uapi/asm/sigcontext.h > index cd4f175dc837..5ccdd94a0855 100644 > --- a/arch/riscv/include/uapi/asm/sigcontext.h > +++ b/arch/riscv/include/uapi/asm/sigcontext.h > @@ -21,6 +21,10 @@ struct __sc_riscv_v_state { > struct __riscv_v_ext_state v_state; > } __attribute__((aligned(16))); > > +struct __sc_riscv_cfi_state { > + unsigned long ss_ptr; /* shadow stack pointer */ > + unsigned long rsvd; /* keeping another word reserved in case we need it */ > +}; > /* > * Signal context structure > * > @@ -29,6 +33,7 @@ struct __sc_riscv_v_state { > */ > struct sigcontext { > struct user_regs_struct sc_regs; > + struct __sc_riscv_cfi_state sc_cfi_state; I am concerned about this change as this could potentially break uabi. Let's say there is a pre-CFI program running on this kernel. It receives a signal so the kernel lays out the sig-stack as presented in this structure. If the program accesses sc_fpregs, it would now get sc_cfi_state. As the offset has changed, and the pre-CFI program has not been re-compiled. > union { > union __riscv_fp_state sc_fpregs; > struct __riscv_extra_ext_header sc_extdesc; > -- > 2.43.2 > There may be two ways to deal with this. One is to use a different signal ABI for CFI-enabled programs. This may complicate the user space because new programs will have to determine whether it should use the CFI-ABI at run time. Another way is to follow what Vector does for signal stack. It adds a way to introduce new extensions on signal stack without impacting ABI. Please let me know if I misunderstand anything, thanks. Cheers, Andy
On Fri, May 24, 2024 at 05:46:16PM +0800, Andy Chiu wrote: >Hi Deepak, > >On Thu, Apr 4, 2024 at 7:42 AM Deepak Gupta <debug@rivosinc.com> wrote: >> >> Shadow stack needs to be saved and restored on signal delivery and signal >> return. >> >> sigcontext embedded in ucontext is extendible. Adding cfi state in there >> which can be used to save cfi state before signal delivery and restore >> cfi state on sigreturn >> >> Signed-off-by: Deepak Gupta <debug@rivosinc.com> >> --- >> arch/riscv/include/uapi/asm/sigcontext.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/arch/riscv/include/uapi/asm/sigcontext.h b/arch/riscv/include/uapi/asm/sigcontext.h >> index cd4f175dc837..5ccdd94a0855 100644 >> --- a/arch/riscv/include/uapi/asm/sigcontext.h >> +++ b/arch/riscv/include/uapi/asm/sigcontext.h >> @@ -21,6 +21,10 @@ struct __sc_riscv_v_state { >> struct __riscv_v_ext_state v_state; >> } __attribute__((aligned(16))); >> >> +struct __sc_riscv_cfi_state { >> + unsigned long ss_ptr; /* shadow stack pointer */ >> + unsigned long rsvd; /* keeping another word reserved in case we need it */ >> +}; >> /* >> * Signal context structure >> * >> @@ -29,6 +33,7 @@ struct __sc_riscv_v_state { >> */ >> struct sigcontext { >> struct user_regs_struct sc_regs; >> + struct __sc_riscv_cfi_state sc_cfi_state; > >I am concerned about this change as this could potentially break uabi. >Let's say there is a pre-CFI program running on this kernel. It >receives a signal so the kernel lays out the sig-stack as presented in >this structure. If the program accesses sc_fpregs, it would now get >sc_cfi_state. As the offset has changed, and the pre-CFI program has >not been re-compiled. Yeah this is a problem if program was built with older kernel/old toolchain (or cfi unaware toolchain). Thanks. > >> union { >> union __riscv_fp_state sc_fpregs; >> struct __riscv_extra_ext_header sc_extdesc; >> -- >> 2.43.2 >> > >There may be two ways to deal with this. One is to use a different >signal ABI for CFI-enabled programs. This may complicate the user >space because new programs will have to determine whether it should >use the CFI-ABI at run time. Another way is to follow what Vector does >for signal stack. It adds a way to introduce new extensions on signal >stack without impacting ABI. > >Please let me know if I misunderstand anything, thanks. I think following how vector does would be cleaner. Let me munch on this a little bit. > >Cheers, >Andy
diff --git a/arch/riscv/include/uapi/asm/sigcontext.h b/arch/riscv/include/uapi/asm/sigcontext.h index cd4f175dc837..5ccdd94a0855 100644 --- a/arch/riscv/include/uapi/asm/sigcontext.h +++ b/arch/riscv/include/uapi/asm/sigcontext.h @@ -21,6 +21,10 @@ struct __sc_riscv_v_state { struct __riscv_v_ext_state v_state; } __attribute__((aligned(16))); +struct __sc_riscv_cfi_state { + unsigned long ss_ptr; /* shadow stack pointer */ + unsigned long rsvd; /* keeping another word reserved in case we need it */ +}; /* * Signal context structure * @@ -29,6 +33,7 @@ struct __sc_riscv_v_state { */ struct sigcontext { struct user_regs_struct sc_regs; + struct __sc_riscv_cfi_state sc_cfi_state; union { union __riscv_fp_state sc_fpregs; struct __riscv_extra_ext_header sc_extdesc;
Shadow stack needs to be saved and restored on signal delivery and signal return. sigcontext embedded in ucontext is extendible. Adding cfi state in there which can be used to save cfi state before signal delivery and restore cfi state on sigreturn Signed-off-by: Deepak Gupta <debug@rivosinc.com> --- arch/riscv/include/uapi/asm/sigcontext.h | 5 +++++ 1 file changed, 5 insertions(+)