Re: [PATCH v2] sched: move stack_canary to the start of the randomizable region

[Peter Zijlstra]

On Fri, Nov 06, 2026 at 05:07:30PM +0100, Peter Zijlstra wrote:

> Why not do something a little more structured; like so?
> 
> ---
>  arch/riscv/include/asm/stackprotector.h | 4 ++--
>  arch/riscv/include/asm/thread_info.h    | 7 +++++++
>  arch/riscv/kernel/asm-offsets.c         | 7 +++----
>  drivers/misc/lkdtm/bugs.c               | 4 ++--
>  include/linux/sched.h                   | 6 +++++-
>  kernel/fork.c                           | 2 +-
>  6 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/stackprotector.h b/arch/riscv/include/asm/stackprotector.h
> index 43895b90fe3f..b56e49ce0580 100644
> --- a/arch/riscv/include/asm/stackprotector.h
> +++ b/arch/riscv/include/asm/stackprotector.h
> @@ -15,8 +15,8 @@ static __always_inline void boot_init_stack_canary(void)
>  {
>  	unsigned long canary = get_random_canary();
>  
> -	current->stack_canary = canary;
> +	tsk_stack_canary(current) = canary;
>  	if (!IS_ENABLED(CONFIG_STACKPROTECTOR_PER_TASK))
> -		__stack_chk_guard = current->stack_canary;
> +		__stack_chk_guard = tsk_stack_canary(current);
>  }
>  #endif /* _ASM_RISCV_STACKPROTECTOR_H */
> diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> index 55019fdfa9ec..eed764f06682 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -73,6 +73,9 @@ struct thread_info {
>  	 */
>  	unsigned long		a0, a1, a2;
>  #endif
> +#ifdef CONFIG_STACKPROTECTOR
> +	unsigned long		stack_canary;
> +#endif
>  #ifdef CONFIG_RISCV_USER_CFI
>  	struct cfi_state	user_cfi_state;
>  #endif
> @@ -86,6 +89,10 @@ struct thread_info {
>  #define INIT_SCS
>  #endif
>  

This should probably have a comment about 12 bit immediates somewhere.

> +#ifdef CONFIG_STACKPROTECTOR
> +#define tsk_stack_canary(p) ((p)->thread_info.stack_canary)
> +#endif
> +
>  /*
>   * macros/functions for gaining access to the thread information structure
>   *
> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
> index a75f0cfea1e9..eb694e1215bb 100644
> --- a/arch/riscv/kernel/asm-offsets.c
> +++ b/arch/riscv/kernel/asm-offsets.c
> @@ -49,7 +49,9 @@ void asm_offsets(void)
>  	OFFSET(TASK_TI_A1, task_struct, thread_info.a1);
>  	OFFSET(TASK_TI_A2, task_struct, thread_info.a2);
>  #endif
> -
> +#ifdef CONFIG_STACKPROTECTOR
> +	OFFSET(TSK_STACK_CANARY, task_struct, thread_info.stack_canary);
> +#endif
>  	OFFSET(TASK_TI_CPU_NUM, task_struct, thread_info.cpu);
>  #ifdef CONFIG_RISCV_USER_CFI
>  	OFFSET(TASK_TI_CFI_STATE, task_struct, thread_info.user_cfi_state);
> @@ -88,9 +90,6 @@ void asm_offsets(void)
>  	OFFSET(TASK_THREAD_F30, task_struct, thread.fstate.f[30]);
>  	OFFSET(TASK_THREAD_F31, task_struct, thread.fstate.f[31]);
>  	OFFSET(TASK_THREAD_FCSR, task_struct, thread.fstate.fcsr);
> -#ifdef CONFIG_STACKPROTECTOR
> -	OFFSET(TSK_STACK_CANARY, task_struct, stack_canary);
> -#endif
>  
>  	DEFINE(PT_SIZE, sizeof(struct pt_regs));
>  	OFFSET(PT_EPC, pt_regs, epc);
> diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
> index 2761685cf4f8..977ee7584249 100644
> --- a/drivers/misc/lkdtm/bugs.c
> +++ b/drivers/misc/lkdtm/bugs.c
> @@ -247,9 +247,9 @@ static noinline void __lkdtm_REPORT_STACK_CANARY(void *stack)
>  	for (i = 1; i < 16; i++) {
>  		canary = (unsigned long *)stack + i;
>  #ifdef CONFIG_STACKPROTECTOR
> -		if (*canary == current->stack_canary)
> +		if (*canary == tsk_stack_canary(current))
>  			current_offset = i;
> -		if (*canary == init_task.stack_canary)
> +		if (*canary == tsk_stack_canary(&init_task))
>  			init_offset = i;
>  #endif
>  	}
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index e4f6323d7855..b7702fefed63 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1083,7 +1083,7 @@ struct task_struct {
>  	pid_t				pid;
>  	pid_t				tgid;
>  
> -#ifdef CONFIG_STACKPROTECTOR
> +#if defined(CONFIG_STACKPROTECTOR) && !defined(tsk_stack_canary)
>  	/* Canary value for the -fstack-protector GCC feature: */
>  	unsigned long			stack_canary;
>  #endif
> @@ -1697,6 +1697,10 @@ struct task_struct {
>  	randomized_struct_fields_end
>  } __attribute__ ((aligned (64)));
>  
> +#if defined(CONFIG_STACKPROTECTOR) && !defined(tsk_stack_canary)
> +#define tsk_stack_canary(p) ((p)->stack_canary)
> +#endif
> +
>  #ifdef CONFIG_SCHED_PROXY_EXEC
>  DECLARE_STATIC_KEY_TRUE(__sched_proxy_exec);
>  static inline bool sched_proxy_exec(void)
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 5efceb8d58a7..878d586bda25 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -964,7 +964,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
>  	clear_syscall_work_syscall_user_dispatch(tsk);
>  
>  #ifdef CONFIG_STACKPROTECTOR
> -	tsk->stack_canary = get_random_canary();
> +	tsk_stack_canary(tsk) = get_random_canary();
>  #endif
>  	if (orig->cpus_ptr == &orig->cpus_mask)
>  		tsk->cpus_ptr = &tsk->cpus_mask;
> 
>