Re: [PATCH 1/3] sched/core: Skip the shadow stack end check when SCS is disabled

From: Peter Zijlstra <peterz@infradead.org>
To: Chris Down <chris@chrisdown.name>
Cc: Ingo Molnar <mingo@redhat.com>, Kees Cook <kees@kernel.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>,
	Ard Biesheuvel <ardb@kernel.org>, Will Deacon <will@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org, linux-doc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@meta.com
Subject: Re: [PATCH 1/3] sched/core: Skip the shadow stack end check when SCS is disabled
Date: Tue, 27 Oct 2026 11:08:43 +0100	[thread overview]
Message-ID: <20261027100843.GA2668564@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <0e607d3aa7c2c4cc1b5b8ba7b0e25ebdb55726af.1793082868.git.chris@chrisdown.name>

On Tue, Oct 27, 2026 at 02:36:09PM +0800, Chris Down wrote:
> With CONFIG_DYNAMIC_SCS, shadow call stacks are only used when the boot
> CPU lacks pointer authentication. scs_release() already checks
> scs_is_enabled() before it looks at the shadow stack since commit
> 9beccca09840 ("scs: add support for dynamic shadow call stacks"), but
> schedule_debug() does not.
> 
> With CONFIG_SCHED_STACK_END_CHECK=y it therefore checks init_task's
> unused shadow stack on every context switch. That is harmless right now,
> but the check is redundant and costs a load from an unrelated cache line
> in __schedule().
> 
> So, let's guard on scs_is_enabled(). It is a static branch, and it folds
> to true when CONFIG_DYNAMIC_SCS is not set.
> 
> Signed-off-by: Chris Down <chris@chrisdown.name>
> ---
>  kernel/sched/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index d7c3b43158de..fc269fd10192 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6152,7 +6152,7 @@ static inline void schedule_debug(struct task_struct *prev, bool preempt)
>  	if (task_stack_end_corrupted(prev))
>  		panic("corrupted stack end detected inside scheduler\n");
>  
> -	if (task_scs_end_corrupted(prev))
> +	if (scs_is_enabled() && task_scs_end_corrupted(prev))
>  		panic("corrupted shadow stack detected inside scheduler\n");

How about you push that scs_is_enabled() into task_scs_end_corrupted(),
this makes less noise here, and also keeps the next patch saner, no?