On Tue, Oct 08, 2024 at 10:55:29PM +0000, Edgecombe, Rick P wrote:
On Tue, 2024-10-08 at 15:36 -0700, Deepak Gupta wrote:
+unsigned long shstk_alloc_thread_stack(struct task_struct *tsk,
+ const struct kernel_clone_args *args)
+{
+ unsigned long addr, size;
+
+ /* If shadow stack is not supported, return 0 */
+ if (!cpu_supports_shadow_stack())
+ return 0;
+
+ /*
+ * If shadow stack is not enabled on the new thread, skip any
+ * switch to a new shadow stack.
+ */
+ if (!is_shstk_enabled(tsk))
+ return 0;
+
+ /*
+ * For CLONE_VFORK the child will share the parents shadow stack.
+ * Set base = 0 and size = 0, this is special means to track this state
+ * so the freeing logic run for child knows to leave it alone.
+ */
+ if (args->flags & CLONE_VFORK) {
+ set_shstk_base(tsk, 0, 0);
+ return 0;
+ }
+
+ /*
+ * For !CLONE_VM the child will use a copy of the parents shadow
+ * stack.
+ */
+ if (!(args->flags & CLONE_VM))
+ return 0;
+
+ /*
+ * reaching here means, CLONE_VM was specified and thus a separate shadow
+ * stack is needed for new cloned thread. Note: below allocation is happening
+ * using current mm.
+ */
+ size = calc_shstk_size(args->stack_size);
+ addr = allocate_shadow_stack(0, size, 0, false);
+ if (IS_ERR_VALUE(addr))
+ return addr;
+
+ set_shstk_base(tsk, addr, size);
+
+ return addr + size;
+}
A lot of this patch and the previous one is similar to x86's and arm's. It great
that we can have consistency around this behavior.
There might be enough consistency to refactor some of the arch code into a
kernel/shstk.c.
Should we try?
Yeah you're right. Honestly, I've been shameless in adapting most of the flows
from x86 `shstk.c` for risc-v. So thank you for that.
Now that we've `ARCH_HAS_USER_SHADOW_STACK` part of multiple patch series (riscv
shadowstack, clone3 and I think arm64 gcs series as well). It's probably the
appropriate time to find common grounds.
This is what I suggest
- move most of the common/arch agnostic shadow stack stuff in kernel/shstk.c
This gets part of compile if `ARCH_HAS_USER_SHADOW_STACK` is enabled/selected.
- allow arch specific branch out guard checks for "if cpu supports", "is shadow stack
enabled on the task_struct" (I expect each arch layout of task_struct will be
different, no point finding common ground there), etc.
I think it's worth a try.
If you already don't have patches, I'll spend some time to see what it takes to
converge in my next version. If I end up into some roadblock, will use this thread
for further discussion.
