[PATCH] riscv: cif: clear CFI lock status in start_thread

[Zong Li]

.

On Tue, Mar 10, 2026 at 8:04 AM Deepak Gupta <debug at rivosinc.com> wrote:
>
> On Fri, Mar 06, 2026 at 12:06:22AM -0800, Zong Li wrote:
> >When libc locks the CFI status through the following prctl:
> > - PR_LOCK_SHADOW_STACK_STATUS
> > - PR_LOCK_INDIR_BR_LP_STATUS
> >
> >A newly forked process will inherit the lock status if it
>
> Might want use term "newly execd address space" or something like that.
>
> libc shouldn't be enabling cfi after `fork` or `clone`.
> `exec*` are the ones which should have their slate clean and it seems
> like `lock` status was not set to clear which this patch fixes. Thanks
> for that.

Thanks for clarifying it. I will re-write the message in the next version.

>
> >does not clear the lock bits. Since the lock bits remain
> >set, libc will later fail to enable the landing pad and
> >shadow stack.
> >
> >Signed-off-by: Zong Li <zong.li at sifive.com>
> >---
> > arch/riscv/include/asm/usercfi.h |  8 ++++----
> > arch/riscv/kernel/process.c      |  2 ++
> > arch/riscv/kernel/usercfi.c      | 12 ++++++------
> > 3 files changed, 12 insertions(+), 10 deletions(-)
> >
> >diff --git a/arch/riscv/include/asm/usercfi.h b/arch/riscv/include/asm/usercfi.h
> >index f7fa9d602aae..c4ab11378308 100644
> >--- a/arch/riscv/include/asm/usercfi.h
> >+++ b/arch/riscv/include/asm/usercfi.h
> >@@ -39,7 +39,7 @@ void set_active_shstk(struct task_struct *task, unsigned long shstk_addr);
> > bool is_shstk_enabled(struct task_struct *task);
> > bool is_shstk_locked(struct task_struct *task);
> > bool is_shstk_allocated(struct task_struct *task);
> >-void set_shstk_lock(struct task_struct *task);
> >+void set_shstk_lock(struct task_struct *task, bool lock);
> > void set_shstk_status(struct task_struct *task, bool enable);
> > unsigned long get_active_shstk(struct task_struct *task);
> > int restore_user_shstk(struct task_struct *tsk, unsigned long shstk_ptr);
> >@@ -47,7 +47,7 @@ int save_user_shstk(struct task_struct *tsk, unsigned long *saved_shstk_ptr);
> > bool is_indir_lp_enabled(struct task_struct *task);
> > bool is_indir_lp_locked(struct task_struct *task);
> > void set_indir_lp_status(struct task_struct *task, bool enable);
> >-void set_indir_lp_lock(struct task_struct *task);
> >+void set_indir_lp_lock(struct task_struct *task, bool lock);
> >
> > #define PR_SHADOW_STACK_SUPPORTED_STATUS_MASK (PR_SHADOW_STACK_ENABLE)
> >
> >@@ -69,7 +69,7 @@ void set_indir_lp_lock(struct task_struct *task);
> >
> > #define is_shstk_allocated(task) false
> >
> >-#define set_shstk_lock(task) do {} while (0)
> >+#define set_shstk_lock(task, lock) do {} while (0)
> >
> > #define set_shstk_status(task, enable) do {} while (0)
> >
> >@@ -79,7 +79,7 @@ void set_indir_lp_lock(struct task_struct *task);
> >
> > #define set_indir_lp_status(task, enable) do {} while (0)
> >
> >-#define set_indir_lp_lock(task) do {} while (0)
> >+#define set_indir_lp_lock(task, lock) do {} while (0)
> >
> > #define restore_user_shstk(tsk, shstk_ptr) -EINVAL
> >
> >diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> >index 6b3648256a0f..36bac478f1e1 100644
> >--- a/arch/riscv/kernel/process.c
> >+++ b/arch/riscv/kernel/process.c
> >@@ -164,11 +164,13 @@ void start_thread(struct pt_regs *regs, unsigned long pc,
> >       set_shstk_status(current, false);
> >       set_shstk_base(current, 0, 0);
> >       set_active_shstk(current, 0);
> >+      set_shstk_lock(current, false);
> >       /*
> >        * disable indirect branch tracking on exec.
> >        * libc will enable it later via prctl.
> >        */
> >       set_indir_lp_status(current, false);
> >+      set_indir_lp_lock(current, false);
>
> Perhaps set status field to zero to prevent any future regression too.

I don’t completely understand this. I think you’re not referring to
set_shstk_status or set_indir_lp_status, because you have cleaned
them. Could you clarify what the status refers to? Thanks

>
> >
> > #ifdef CONFIG_64BIT
> >       regs->status &= ~SR_UXL;
> >diff --git a/arch/riscv/kernel/usercfi.c b/arch/riscv/kernel/usercfi.c
> >index a8530e6afb1e..a101e317fe5e 100644
> >--- a/arch/riscv/kernel/usercfi.c
> >+++ b/arch/riscv/kernel/usercfi.c
> >@@ -74,9 +74,9 @@ void set_shstk_status(struct task_struct *task, bool enable)
> >       csr_write(CSR_ENVCFG, task->thread.envcfg);
> > }
> >
> >-void set_shstk_lock(struct task_struct *task)
> >+void set_shstk_lock(struct task_struct *task, bool lock)
> > {
> >-      task->thread_info.user_cfi_state.ubcfi_locked = 1;
> >+      task->thread_info.user_cfi_state.ubcfi_locked = lock;
> > }
> >
> > bool is_indir_lp_enabled(struct task_struct *task)
> >@@ -104,9 +104,9 @@ void set_indir_lp_status(struct task_struct *task, bool enable)
> >       csr_write(CSR_ENVCFG, task->thread.envcfg);
> > }
> >
> >-void set_indir_lp_lock(struct task_struct *task)
> >+void set_indir_lp_lock(struct task_struct *task, bool lock)
> > {
> >-      task->thread_info.user_cfi_state.ufcfi_locked = 1;
> >+      task->thread_info.user_cfi_state.ufcfi_locked = lock;
> > }
> > /*
> >  * If size is 0, then to be compatible with regular stack we want it to be as big as
> >@@ -452,7 +452,7 @@ int arch_lock_shadow_stack_status(struct task_struct *task,
> >           !is_shstk_enabled(task) || arg != 0)
> >               return -EINVAL;
> >
> >-      set_shstk_lock(task);
> >+      set_shstk_lock(task, true);
> >
> >       return 0;
> > }
> >@@ -502,7 +502,7 @@ int arch_lock_indir_br_lp_status(struct task_struct *task,
> >           !is_indir_lp_enabled(task) || arg != 0)
> >               return -EINVAL;
> >
> >-      set_indir_lp_lock(task);
> >+      set_indir_lp_lock(task, true);
> >
> >       return 0;
> > }
> >--
> >2.43.7
> >