Re: [PATCH 5/6] x86/shstk: don't create the shadow stack for PF_USER_WORKERs
From: Oleg Nesterov
Date: Fri Aug 15 2025 - 12:56:53 EST
On 08/15, Edgecombe, Rick P wrote:
>
> The bit in thread.features is like a sticky bit that is inherrited whenver a
> thread is cloned.
...
> You don't want to allow a protected app to spawn a new thread that
> escapes the enforcement.
Ah, this is clear. But again, PF_USER_WORKER is the kernel thread cloned
by the kernel. Yes, it shares the same thread-group, but this is only to
make SIGKILL/exit_group/etc work. It is not that userspace app can create
it via something like pthread_create().
> So what are we trying to do for PF_USER_WORKER? Prevent them from wasting a VMA
> with an unused shadow stack? Or set PF_USER_WORKER's aside from the logic that
> is about more than protecting the individual thread in the process?
Let me quote my answer to Mark:
The fact that a kernel thread can have the pointless ARCH_SHSTK_SHSTK is
the only reason I know why x86_task_fpu(PF_USER_WORKER) has to work.
I'd like to make this logic consistent with PF_KTHREAD, and in the longer
term change the x86 FPU code so that the kernel threads can run without
without "struct fpu" attached to task_struct.
And again, please see
Warning from x86_task_fpu()
https://lore.kernel.org/all/aJVuZZgYjEMxiUYq@ly-workstation/
PF_USER_WORKERs and shadow stack
https://lore.kernel.org/all/20250813162824.GA15234@xxxxxxxxxx/
and 6/6 in this series.
> No, to make it have the same logic as the vfork case (which doesn't allocate a
> new shadow stack).
>
> Like:
> if ((clone_flags & CLONE_VFORK) || minimal) {
> shstk->base = 0;
> shstk->size = 0;
> return 0;
> }
Aha, got it. Agreed, but I think we also need to clear ARCH_SHSTK_SHSTK
copied by arch_dup_task_struct() ?
Oleg.