Re: [PATCH v3 1/1] x86/shstk: don't create the shadow stack for PF_USER_WORKERs

[Dave Hansen <dave.hansen@intel.com>]

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.