Re: [PATCH v12 23/28] riscv: kernel command line option to opt out of user cfi

Deepak Gupta
2025-03-20T22:31:33Z

On Thu, Mar 20, 2025 at 2:35 PM Radim Krčmář <rkrcmar@ventanamicro.com> wrote:
>
> 2025-03-14T14:39:42-07:00, Deepak Gupta <debug@rivosinc.com>:
> > This commit adds a kernel command line option using which user cfi can be
> > disabled.
> >
> > Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> > ---
> > arch/riscv/kernel/usercfi.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/arch/riscv/kernel/usercfi.c b/arch/riscv/kernel/usercfi.c
> > index d31d89618763..813162ce4f15 100644
> > --- a/arch/riscv/kernel/usercfi.c
> > +++ b/arch/riscv/kernel/usercfi.c
> > @@ -17,6 +17,8 @@
> > #include <asm/csr.h>
> > #include <asm/usercfi.h>
> >
> > +bool disable_riscv_usercfi;
> > +
> > #define SHSTK_ENTRY_SIZE sizeof(void *)
> >
> > bool is_shstk_enabled(struct task_struct *task)
> > @@ -396,6 +398,9 @@ int arch_set_shadow_stack_status(struct task_struct *t, unsigned long status)
> > unsigned long size = 0, addr = 0;
> > bool enable_shstk = false;
> >
> > + if (disable_riscv_usercfi)
> > + return 0;
> > +
> > if (!cpu_supports_shadow_stack())
> > return -EINVAL;
> >
> > @@ -475,6 +480,9 @@ int arch_set_indir_br_lp_status(struct task_struct *t, unsigned long status)
> > {
> > bool enable_indir_lp = false;
> >
> > + if (disable_riscv_usercfi)
> > + return 0;
> > +
> > if (!cpu_supports_indirect_br_lp_instr())
> > return -EINVAL;
> >
> > @@ -507,3 +515,16 @@ int arch_lock_indir_br_lp_status(struct task_struct *task,
> >
> > return 0;
> > }
> > +
> > +static int __init setup_global_riscv_enable(char *str)
> > +{
> > + if (strcmp(str, "true") == 0)
> > + disable_riscv_usercfi = true;
> > +
> > + pr_info("Setting riscv usercfi to be %s\n",
> > + (disable_riscv_usercfi ? "disabled" : "enabled"));
> > +
> > + return 1;
> > +}
> > +
> > +__setup("disable_riscv_usercfi=", setup_global_riscv_enable);
>
> I'd prefer two command line options instead.

One for zicfilp and one for zicfiss ?
>
> In any case, I think we still document params in kernel-parameters.txt.

Noted, will do that.