Re: [PATCH RFT v9 4/8] fork: Add shadow stack support to clone3()

[Christian Brauner <brauner@kernel.org>]

On Fri, Sep 27, 2024 at 03:21:59PM GMT, Edgecombe, Rick P wrote:
> On Fri, 2024-09-27 at 10:50 +0200, Christian Brauner wrote:
> > The legacy clone system call had required userspace to know in which
> > direction the stack was growing and then pass down the stack pointer
> > appropriately (e.g., parisc grows upwards).
> > 
> > And in fact, the old clone() system call did take an additional
> > stack_size argument on specific architectures. For example, on
> > microblaze.
> > 
> > Also, when clone3() was done we still had ia64 in the tree which had a
> > separate clone2() system call that also required a stack_size argument.
> > 
> > So userspace ended up with code like this or worse:
> > 
> >      #define __STACK_SIZE (8 * 1024 * 1024)
> >      pid_t sys_clone(int (*fn)(void *), void *arg, int flags, int *pidfd)
> >      {
> >              pid_t ret;
> >              void *stack;
> > 
> >              stack = malloc(__STACK_SIZE);
> >              if (!stack)
> >                      return -ENOMEM;
> > 
> >      #ifdef __ia64__
> >              ret = __clone2(fn, stack, __STACK_SIZE, flags | SIGCHLD, arg, pidfd);
> >      #elif defined(__parisc__) /* stack grows up */
> >              ret = clone(fn, stack, flags | SIGCHLD, arg, pidfd);
> >      #else
> >              ret = clone(fn, stack + __STACK_SIZE, flags | SIGCHLD, arg, pidfd);
> >      #endif
> >              return ret;
> >      }
> > 
> > So we talked to the glibc folks which preferred the kernel to do all
> > this nonsense for them as it has that knowledge.
> 
> Thanks for the info!
> 
> > 
> > My preference is to keep the api consistent and require a stack_size for
> > shadow stacks as well.
> 
> Did you catch that a token can be at a different offsets location on the stack
> depending on args passed to map_shadow_stack? So userspace will need something
> like the code above, but that adjusts the 'shadow_stack_size' such that the
> kernel looks for the token in the right place. It will be even weirder if
> someone uses clone3 to switch to a stack that has already been used, and pivoted
> off of, such that a token was left in the middle of the stack. In that case
> userspace would have to come up with args disconnected from the actual size of
> the shadow stack such that the kernel would be cajoled into looking for the
> token in the right place.
> 
> A shadow stack size is more symmetric on the surface, but I'm not sure it will
> be easier for userspace to handle. So I think we should just have a pointer to
> the token. But it will be a usable implementation either way.

Maybe it's best to let glibc folks decide what is better/more ergonomic for them.