mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ron Economos <re@w6rz.net>
To: Valentin Schneider <vschneid@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ruidong Tian <tianruidong@linux.alibaba.com>
Cc: mingo@redhat.com, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
	kprateek.nayak@amd.com, linux-kernel@vger.kernel.org,
	oliver.yang@linux.alibaba.com, Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Alexandre Ghiti <alex@ghiti.fr>,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v2] sched: move stack_canary to the start of the randomizable region
Date: Thu, 12 Nov 2026 05:33:16 -0800	[thread overview]
Message-ID: <34f2ff11-8156-4c32-91dc-e4576807cf3b@w6rz.net> (raw)
In-Reply-To: <xhsmhy0b2ntmh.mognet@vschneid-thinkpadt14sgen2i.remote.csb>

On 11/9/26 03:15, Valentin Schneider wrote:

> On 06/11/26 17:07, Peter Zijlstra wrote:
>> On Sat, May 09, 2026 at 11:50:14AM +0800, Ruidong Tian wrote:
>>> @@ -848,9 +848,15 @@ struct task_struct {
>>>   
>>>   	int				on_cpu;
>>>   	struct __call_single_node	wake_entry;
>>> -	unsigned int			wakee_flips;
>>> +
>>> +#ifdef CONFIG_STACKPROTECTOR
>>> +	/* Canary value for the -fstack-protector GCC feature: */
>>> +	unsigned long			stack_canary;
>>> +#endif
>>> +
>>>   	unsigned long			wakee_flip_decay_ts;
>>>   	struct task_struct		*last_wakee;
>>> +	unsigned int			wakee_flips;
>>>   
>>>   	/*
>>>   	 * recent_used_cpu is initially set as the last CPU used by a task
>>> @@ -1060,10 +1066,6 @@ struct task_struct {
>>>   	pid_t				pid;
>>>   	pid_t				tgid;
>>>   
>>> -#ifdef CONFIG_STACKPROTECTOR
>>> -	/* Canary value for the -fstack-protector GCC feature: */
>>> -	unsigned long			stack_canary;
>>> -#endif
>>>   	/*
>>>   	 * Pointers to the (original) parent process, youngest child, younger sibling,
>>>   	 * older sibling, respectively.  (p->father can be replaced with
>> This seems exceedingly random; and very prone to also randomly breaking
>> as task_struct is further modified.
>>
>> Why not do something a little more structured; like so?
>> until someone smarter can come with a risc-v specific improvement
> See :-)
>
> How's this? I fluffed the changelog and added a comment to that
> stack_canary helper
>
> ---
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Fri, 6 Nov 2026 17:07:30 +0100
> Subject: [PATCH] riscv, sched: move stack_canary to thread_info
>
> task_struct keeps growing over time. On architectures that compute the
> per-task stack canary offset from asm-offsets.h and pass it to the
> compiler via -mstack-protector-guard-offset=, this growth eventually
> pushes stack_canary beyond what the target ISA can encode.
>
> On RISC-V, canary loads are emitted as
>
>      ld	t0, TSK_STACK_CANARY(tp)
>
> where TSK_STACK_CANARY must fit into a 12-bit signed immediate, i.e.
> [-2048, 2047].  Once stack_canary sits past byte 2047 of task_struct,
> the build fails with
>
>      cc1: error: '<N>' is not a valid offset in
> 	'-mstack-protector-guard-offset='
>
> On RISC-V, CONFIG_STACKPROTECTOR_PER_TASK depends on !RANDSTRUCT, thus
> stack_canary does not benefit from being in the task_struct randomized
> region.
>
> Move the RISC-V stack_canary to thread_info, and introduce an access macro
> to point to the right stack_canary location. Other archs are unaffected.
>
> Not-signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
> Not-signed-off-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
> ---
>   arch/riscv/include/asm/stackprotector.h |  4 ++--
>   arch/riscv/include/asm/thread_info.h    | 16 ++++++++++++++++
>   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, 29 insertions(+), 10 deletions(-)
>
> diff --git a/arch/riscv/include/asm/stackprotector.h b/arch/riscv/include/asm/stackprotector.h
> index 43895b90fe3f6..b56e49ce0580e 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 55019fdfa9eca..b3a1e1ddbfb0e 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,19 @@ struct thread_info {
>   #define INIT_SCS
>   #endif
>   
> +#ifdef CONFIG_STACKPROTECTOR
> +/*
> + * The canary is loaded via
> + *
> + *   ld	t0, TSK_STACK_CANARY(tp)
> + *
> + * This imposes a 2047 byte (12-bit signed immediate) limit for the stack_canary
> + * offset, which is a RISC-V specific limit. Because of that, we keep the
> + * stack_canary high up in task_struct, in thread_info itself.
> + */
> +#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 a75f0cfea1e9f..eb694e1215bbb 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 2761685cf4f87..977ee75842490 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 e4f6323d78555..b7702fefed635 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 5efceb8d58a78..878d586bda256 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;

I'm seeing this build failure on RISC-V with gcc 16 and CONFIG_SCHED_CLASS_EXT=y. The build does not fail with gcc 14. I've tested 
the proposed patch with gcc 16 and it builds and boots (on real hardware) just fine. So for what it's worth:

Tested-by: Ron Economos <re@w6rz.net>

Also re-adding the RISC-V folks.


  reply	other threads:[~2026-11-12 13:33 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-09  3:50 Ruidong Tian
2026-05-12 16:16 ` Valentin Schneider
2026-08-21 17:11   ` Charles Mirabile
2026-11-06 15:29     ` Valentin Schneider
2026-11-06 16:07 ` Peter Zijlstra
2026-11-06 16:14   ` Peter Zijlstra
2026-11-09 11:15   ` Valentin Schneider
2026-11-12 13:33     ` Ron Economos [this message]
2026-11-12 15:34       ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=34f2ff11-8156-4c32-91dc-e4576807cf3b@w6rz.net \
    --to=re@w6rz.net \
    --cc=alex@ghiti.fr \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=oliver.yang@linux.alibaba.com \
    --cc=palmer@dabbelt.com \
    --cc=peterz@infradead.org \
    --cc=pjw@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tianruidong@linux.alibaba.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®