linux-kernel - Re: [PATCH v3 1/1] x86/shstk: don't create the shadow stack for PF_USER

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b91c72b6-cbe0-4767-8d65-05f804733a55@intel.com>
Date: Wed, 3 Sep 2025 09:14:30 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Oleg Nesterov <oleg@...hat.com>, Borislav Petkov <bp@...en8.de>,
 Dave Hansen <dave.hansen@...ux.intel.com>, Deepak Gupta
 <debug@...osinc.com>, "H. Peter Anvin" <hpa@...or.com>,
 Ingo Molnar <mingo@...nel.org>,
 Linus Torvalds <torvalds@...ux-foundation.org>,
 Mark Brown <broonie@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
 Rick Edgecombe <rick.p.edgecombe@...el.com>,
 Sohil Mehta <sohil.mehta@...el.com>, Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v3 1/1] x86/shstk: don't create the shadow stack for
 PF_USER_WORKERs

On 9/3/25 06:42, Oleg Nesterov wrote:
>  arch/x86/include/asm/shstk.h | 4 ++--
>  arch/x86/kernel/process.c    | 2 +-
>  arch/x86/kernel/shstk.c      | 9 +++++++--
>  3 files changed, 10 insertions(+), 5 deletions(-)

That's not a great diffstat for a "cleanup". It's also not fixing any
end-user-visible issues as far as I can tell.

>  static inline void shstk_free(struct task_struct *p) {}
>  static inline int setup_signal_shadow_stack(struct ksignal *ksig) { return 0; }
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 1b7960cf6eb0..e932e0e53972 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -209,7 +209,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
>  	 * is disabled, new_ssp will remain 0, and fpu_clone() will know not to
>  	 * update it.
>  	 */
> -	new_ssp = shstk_alloc_thread_stack(p, clone_flags, args->stack_size);
> +	new_ssp = shstk_alloc_thread_stack(p, clone_flags, args->fn, args->stack_size);

Passing 'args->fn' as a 'bool' argument is a bit cruel, don't you think?

>  	if (IS_ERR_VALUE(new_ssp))
>  		return PTR_ERR((void *)new_ssp);
>  
> diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
> index 2ddf23387c7e..6c8c4593e202 100644
> --- a/arch/x86/kernel/shstk.c
> +++ b/arch/x86/kernel/shstk.c
> @@ -192,7 +192,7 @@ void reset_thread_features(void)
>  }
>  
>  unsigned long shstk_alloc_thread_stack(struct task_struct *tsk, unsigned long clone_flags,
> -				       unsigned long stack_size)
> +				       bool minimal, unsigned long stack_size)
'minimal' is an awfully meaningless name for this.

This doesn't clean things up or clarify the situation enough to make me
want to apply it immediately.