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.