Message-ID: <Z9RXLU0w59h1QEtR@debug.ba.rivosinc.com>
Date: Fri, 14 Mar 2025 09:19:57 -0700
From: Deepak Gupta <debug@...osinc.com>
To: Zong Li <zong.li@...ive.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Liam R. Howlett" <Liam.Howlett@...cle.com>,
	Vlastimil Babka <vbabka@...e.cz>,
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>, Conor Dooley <conor@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Christian Brauner <brauner@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Eric Biederman <ebiederm@...ssion.com>, Kees Cook <kees@...nel.org>,
	Jonathan Corbet <corbet@....net>, Shuah Khan <shuah@...nel.org>,
	Jann Horn <jannh@...gle.com>, Conor Dooley <conor+dt@...nel.org>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-mm@...ck.org, linux-riscv@...ts.infradead.org,
	devicetree@...r.kernel.org, linux-arch@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org,
	alistair.francis@....com, richard.henderson@...aro.org,
	jim.shu@...ive.com, andybnac@...il.com, kito.cheng@...ive.com,
	charlie@...osinc.com, atishp@...osinc.com, evan@...osinc.com,
	cleger@...osinc.com, alexghiti@...osinc.com,
	samitolvanen@...gle.com, broonie@...nel.org,
	rick.p.edgecombe@...el.com
Subject: Re: [PATCH v11 13/27] prctl: arch-agnostic prctl for indirect branch
 tracking

On Fri, Mar 14, 2025 at 04:25:59PM +0800, Zong Li wrote:
>On Mon, Mar 10, 2025 at 11:42 PM Deepak Gupta <debug@...osinc.com> wrote:
>>
>> Three architectures (x86, aarch64, riscv) have support for indirect branch
>> tracking feature in a very similar fashion. On a very high level, indirect
>> branch tracking is a CPU feature where CPU tracks branches which uses
>> memory operand to perform control transfer in program. As part of this
>> tracking on indirect branches, CPU goes in a state where it expects a
>> landing pad instr on target and if not found then CPU raises some fault
>> (architecture dependent)
>>
>> x86 landing pad instr - `ENDBRANCH`
>> aarch64 landing pad instr - `BTI`
>> riscv landing instr - `lpad`
>>
>> Given that three major arches have support for indirect branch tracking,
>> This patch makes `prctl` for indirect branch tracking arch agnostic.
>>
>> To allow userspace to enable this feature for itself, following prtcls are
>> defined:
>>  - PR_GET_INDIR_BR_LP_STATUS: Gets current configured status for indirect
>>    branch tracking.
>>  - PR_SET_INDIR_BR_LP_STATUS: Sets a configuration for indirect branch
>>    tracking.
>>    Following status options are allowed
>>        - PR_INDIR_BR_LP_ENABLE: Enables indirect branch tracking on user
>>          thread.
>>        - PR_INDIR_BR_LP_DISABLE; Disables indirect branch tracking on user
>>          thread.
>>  - PR_LOCK_INDIR_BR_LP_STATUS: Locks configured status for indirect branch
>>    tracking for user thread.
>>
>> Signed-off-by: Deepak Gupta <debug@...osinc.com>
>> Reviewed-by: Mark Brown <broonie@...nel.org>
>> ---
>>  arch/riscv/include/asm/usercfi.h | 16 ++++++++-
>>  arch/riscv/kernel/entry.S        |  2 +-
>>  arch/riscv/kernel/process.c      |  5 +++
>>  arch/riscv/kernel/usercfi.c      | 76 ++++++++++++++++++++++++++++++++++++++++
>>  include/linux/cpu.h              |  4 +++
>>  include/uapi/linux/prctl.h       | 27 ++++++++++++++
>>  kernel/sys.c                     | 30 ++++++++++++++++
>>  7 files changed, 158 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/usercfi.h b/arch/riscv/include/asm/usercfi.h
>> index c4dcd256f19a..a8cec7c14d1d 100644
>> --- a/arch/riscv/include/asm/usercfi.h
>> +++ b/arch/riscv/include/asm/usercfi.h
>> @@ -16,7 +16,9 @@ struct kernel_clone_args;
>>  struct cfi_status {
>>         unsigned long ubcfi_en : 1; /* Enable for backward cfi. */
>>         unsigned long ubcfi_locked : 1;
>> -       unsigned long rsvd : ((sizeof(unsigned long) * 8) - 2);
>> +       unsigned long ufcfi_en : 1; /* Enable for forward cfi. Note that ELP goes in sstatus */
>> +       unsigned long ufcfi_locked : 1;
>> +       unsigned long rsvd : ((sizeof(unsigned long) * 8) - 4);
>>         unsigned long user_shdw_stk; /* Current user shadow stack pointer */
>>         unsigned long shdw_stk_base; /* Base address of shadow stack */
>>         unsigned long shdw_stk_size; /* size of shadow stack */
>> @@ -33,6 +35,10 @@ 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_status(struct task_struct *task, bool enable);
>> +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);
>>
>>  #define PR_SHADOW_STACK_SUPPORTED_STATUS_MASK (PR_SHADOW_STACK_ENABLE)
>>
>> @@ -58,6 +64,14 @@ void set_shstk_status(struct task_struct *task, bool enable);
>>
>>  #define set_shstk_status(task, enable)
>>
>> +#define is_indir_lp_enabled(task) false
>> +
>> +#define is_indir_lp_locked(task) false
>> +
>> +#define set_indir_lp_status(task, enable)
>> +
>> +#define set_indir_lp_lock(task)
>> +
>>  #endif /* CONFIG_RISCV_USER_CFI */
>>
>>  #endif /* __ASSEMBLY__ */
>> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
>> index 68c99124ea55..00494b54ff4a 100644
>> --- a/arch/riscv/kernel/entry.S
>> +++ b/arch/riscv/kernel/entry.S
>> @@ -143,7 +143,7 @@ SYM_CODE_START(handle_exception)
>>          * Disable the FPU/Vector to detect illegal usage of floating point
>>          * or vector in kernel space.
>>          */
>> -       li t0, SR_SUM | SR_FS_VS
>> +       li t0, SR_SUM | SR_FS_VS | SR_ELP
>>
>>         REG_L s0, TASK_TI_USER_SP(tp)
>>         csrrc s1, CSR_STATUS, t0
>> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
>> index cd11667593fe..4587201dd81d 100644
>> --- a/arch/riscv/kernel/process.c
>> +++ b/arch/riscv/kernel/process.c
>> @@ -160,6 +160,11 @@ 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);
>> +       /*
>> +        * disable indirect branch tracking on exec.
>> +        * libc will enable it later via prctl.
>> +        */
>> +       set_indir_lp_status(current, false);
>
>In set_indir_lp_status and set_shstk_status, the $senvcfg.LPE and
>$senvcfg.SSE fields are set. However, if the CPU does not support this
>CSR, writing to it will trigger an illegal instruction exception.
>Should we add sanity checks to handle this situation? Thanks

hmm. these were two patches. something happened in my workflow and two
were squash together it seems. I need to split them (one for introduction
of generic prctls and another which implements them on riscv)

Being said that, yes good point here. I'll make that change.
>>
>>  #ifdef CONFIG_64BIT
>>         regs->status &= ~SR_UXL;
>> diff --git a/arch/riscv/kernel/usercfi.c b/arch/riscv/kernel/usercfi.c
>> index 37d6fb8144e7..3a66f149a4ef 100644
>> --- a/arch/riscv/kernel/usercfi.c
>> +++ b/arch/riscv/kernel/usercfi.c
>> @@ -69,6 +69,32 @@ void set_shstk_lock(struct task_struct *task)
>>         task->thread_info.user_cfi_state.ubcfi_locked = 1;
>>  }
>>
>> +bool is_indir_lp_enabled(struct task_struct *task)
>> +{
>> +       return task->thread_info.user_cfi_state.ufcfi_en ? true : false;
>> +}
>> +
>> +bool is_indir_lp_locked(struct task_struct *task)
>> +{
>> +       return task->thread_info.user_cfi_state.ufcfi_locked ? true : false;
>> +}
>> +
>> +void set_indir_lp_status(struct task_struct *task, bool enable)
>> +{
>> +       task->thread_info.user_cfi_state.ufcfi_en = enable ? 1 : 0;
>> +
>> +       if (enable)
>> +               task->thread.envcfg |= ENVCFG_LPE;
>> +       else
>> +               task->thread.envcfg &= ~ENVCFG_LPE;
>> +
>> +       csr_write(CSR_ENVCFG, task->thread.envcfg);
>> +}
>> +
>> +void set_indir_lp_lock(struct task_struct *task)
>> +{
>> +       task->thread_info.user_cfi_state.ufcfi_locked = 1;
>> +}
>>  /*
>>   * If size is 0, then to be compatible with regular stack we want it to be as big as
>>   * regular stack. Else PAGE_ALIGN it and return back
>> @@ -369,3 +395,53 @@ int arch_lock_shadow_stack_status(struct task_struct *task,
>>
>>         return 0;
>>  }
>> +
>> +int arch_get_indir_br_lp_status(struct task_struct *t, unsigned long __user *status)
>> +{
>> +       unsigned long fcfi_status = 0;
>> +
>> +       if (!cpu_supports_indirect_br_lp_instr())
>> +               return -EINVAL;
>> +
>> +       /* indirect branch tracking is enabled on the task or not */
>> +       fcfi_status |= (is_indir_lp_enabled(t) ? PR_INDIR_BR_LP_ENABLE : 0);
>> +
>> +       return copy_to_user(status, &fcfi_status, sizeof(fcfi_status)) ? -EFAULT : 0;
>> +}
>> +
>> +int arch_set_indir_br_lp_status(struct task_struct *t, unsigned long status)
>> +{
>> +       bool enable_indir_lp = false;
>> +
>> +       if (!cpu_supports_indirect_br_lp_instr())
>> +               return -EINVAL;
>> +
>> +       /* indirect branch tracking is locked and further can't be modified by user */
>> +       if (is_indir_lp_locked(t))
>> +               return -EINVAL;
>> +
>> +       /* Reject unknown flags */
>> +       if (status & ~PR_INDIR_BR_LP_ENABLE)
>> +               return -EINVAL;
>> +
>> +       enable_indir_lp = (status & PR_INDIR_BR_LP_ENABLE) ? true : false;
>> +       set_indir_lp_status(t, enable_indir_lp);
>> +
>> +       return 0;
>> +}
>> +
>> +int arch_lock_indir_br_lp_status(struct task_struct *task,
>> +                                unsigned long arg)
>> +{
>> +       /*
>> +        * If indirect branch tracking is not supported or not enabled on task,
>> +        * nothing to lock here
>> +        */
>> +       if (!cpu_supports_indirect_br_lp_instr() ||
>> +           !is_indir_lp_enabled(task) || arg != 0)
>> +               return -EINVAL;
>> +
>> +       set_indir_lp_lock(task);
>> +
>> +       return 0;
>> +}
>> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
>> index 6a0a8f1c7c90..fb0c394430c6 100644
>> --- a/include/linux/cpu.h
>> +++ b/include/linux/cpu.h
>> @@ -204,4 +204,8 @@ static inline bool cpu_mitigations_auto_nosmt(void)
>>  }
>>  #endif
>>
>> +int arch_get_indir_br_lp_status(struct task_struct *t, unsigned long __user *status);
>> +int arch_set_indir_br_lp_status(struct task_struct *t, unsigned long status);
>> +int arch_lock_indir_br_lp_status(struct task_struct *t, unsigned long status);
>> +
>>  #endif /* _LINUX_CPU_H_ */
>> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
>> index 5c6080680cb2..6cd90460cbad 100644
>> --- a/include/uapi/linux/prctl.h
>> +++ b/include/uapi/linux/prctl.h
>> @@ -353,4 +353,31 @@ struct prctl_mm_map {
>>   */
>>  #define PR_LOCK_SHADOW_STACK_STATUS      76
>>
>> +/*
>> + * Get the current indirect branch tracking configuration for the current
>> + * thread, this will be the value configured via PR_SET_INDIR_BR_LP_STATUS.
>> + */
>> +#define PR_GET_INDIR_BR_LP_STATUS      77
>> +
>> +/*
>> + * Set the indirect branch tracking configuration. PR_INDIR_BR_LP_ENABLE will
>> + * enable cpu feature for user thread, to track all indirect branches and ensure
>> + * they land on arch defined landing pad instruction.
>> + * x86 - If enabled, an indirect branch must land on `ENDBRANCH` instruction.
>> + * arch64 - If enabled, an indirect branch must land on `BTI` instruction.
>> + * riscv - If enabled, an indirect branch must land on `lpad` instruction.
>> + * PR_INDIR_BR_LP_DISABLE will disable feature for user thread and indirect
>> + * branches will no more be tracked by cpu to land on arch defined landing pad
>> + * instruction.
>> + */
>> +#define PR_SET_INDIR_BR_LP_STATUS      78
>> +# define PR_INDIR_BR_LP_ENABLE            (1UL << 0)
>> +
>> +/*
>> + * Prevent further changes to the specified indirect branch tracking
>> + * configuration.  All bits may be locked via this call, including
>> + * undefined bits.
>> + */
>> +#define PR_LOCK_INDIR_BR_LP_STATUS      79
>> +
>>  #endif /* _LINUX_PRCTL_H */
>> diff --git a/kernel/sys.c b/kernel/sys.c
>> index cb366ff8703a..f347f3518d0b 100644
>> --- a/kernel/sys.c
>> +++ b/kernel/sys.c
>> @@ -2336,6 +2336,21 @@ int __weak arch_lock_shadow_stack_status(struct task_struct *t, unsigned long st
>>         return -EINVAL;
>>  }
>>
>> +int __weak arch_get_indir_br_lp_status(struct task_struct *t, unsigned long __user *status)
>> +{
>> +       return -EINVAL;
>> +}
>> +
>> +int __weak arch_set_indir_br_lp_status(struct task_struct *t, unsigned long status)
>> +{
>> +       return -EINVAL;
>> +}
>> +
>> +int __weak arch_lock_indir_br_lp_status(struct task_struct *t, unsigned long status)
>> +{
>> +       return -EINVAL;
>> +}
>> +
>>  #define PR_IO_FLUSHER (PF_MEMALLOC_NOIO | PF_LOCAL_THROTTLE)
>>
>>  #ifdef CONFIG_ANON_VMA_NAME
>> @@ -2811,6 +2826,21 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>>                         return -EINVAL;
>>                 error = arch_lock_shadow_stack_status(me, arg2);
>>                 break;
>> +       case PR_GET_INDIR_BR_LP_STATUS:
>> +               if (arg3 || arg4 || arg5)
>> +                       return -EINVAL;
>> +               error = arch_get_indir_br_lp_status(me, (unsigned long __user *)arg2);
>> +               break;
>> +       case PR_SET_INDIR_BR_LP_STATUS:
>> +               if (arg3 || arg4 || arg5)
>> +                       return -EINVAL;
>> +               error = arch_set_indir_br_lp_status(me, arg2);
>> +               break;
>> +       case PR_LOCK_INDIR_BR_LP_STATUS:
>> +               if (arg3 || arg4 || arg5)
>> +                       return -EINVAL;
>> +               error = arch_lock_indir_br_lp_status(me, arg2);
>> +               break;
>>         default:
>>                 trace_task_prctl_unknown(option, arg2, arg3, arg4, arg5);
>>                 error = -EINVAL;
>>
>> --
>> 2.34.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists