mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Valentin Schneider <vschneid@redhat.com>
To: 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
Subject: Re: [PATCH v2] sched: move stack_canary to the start of the randomizable region
Date: Mon, 09 Nov 2026 12:15:18 +0100	[thread overview]
Message-ID: <xhsmhy0b2ntmh.mognet@vschneid-thinkpadt14sgen2i.remote.csb> (raw)
In-Reply-To: <20261106160730.GJ1779021@noisy.programming.kicks-ass.net>

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;
-- 
2.55.0


  parent reply	other threads:[~2026-11-09 11:15 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 [this message]
2026-11-12 13:33     ` Ron Economos
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=xhsmhy0b2ntmh.mognet@vschneid-thinkpadt14sgen2i.remote.csb \
    --to=vschneid@redhat.com \
    --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=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=oliver.yang@linux.alibaba.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tianruidong@linux.alibaba.com \
    --cc=vincent.guittot@linaro.org \
    /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®