Skip to content
  • Christian Brauner's avatar
    clone3: validate stack arguments · fa729c4d
    Christian Brauner authored
    Validate the stack arguments and setup the stack depening on whether or not
    it is growing down or up.
    
    Legacy clone() required userspace to know in which direction the stack is
    growing and pass down the stack pointer appropriately. To make things more
    confusing microblaze uses a variant of the clone() syscall selected by
    CONFIG_CLONE_BACKWARDS3 that takes an additional stack_size argument.
    IA64 has a separate clone2() syscall which also takes an additional
    stack_size argument. Finally, parisc has a stack that is growing upwards.
    Userspace therefore has a lot nasty code like the following:
    
     #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;
     }
    
    or even crazier variants such as [3].
    
    With clone3() we have the ability to validate the stack. We can check that
    when stack_size is passed, the stack pointer is valid and the other way
    around. We can also check that the memory area userspace gave us is fine to
    use via access_ok(). Furthermore, we probably should not require
    userspace to know in which direction the stack is growing. It is easy
    for us to do this in the kernel and I couldn't find the original
    reasoning behind exposing this detail to userspace.
    
    /* Intentional user visible API change */
    clone3() was released with 5.3. Currently, it is not documented and very
    unclear to userspace how the stack and stack_size argument have to be
    passed. After talking to glibc folks we concluded that trying to change
    clone3() to setup the stack instead of requiring userspace to do this is
    the right course of action.
    Note, that this is an explicit change in user visible behavior we introduce
    with this patch. If it breaks someone's use-case we will revert! (And then
    e.g. place the new behavior under an appropriate flag.)
    Breaking someone's use-case is very unlikely though. First, neither glibc
    nor musl currently expose a wrapper for clone3(). Second, there is no real
    motivation for anyone to use clone3() directly since it does not provide
    features that legacy clone doesn't. New features for clone3() will first
    happen in v5.5 which is why v5.4 is still a good time to try and make that
    change now and backport it to v5.3. Searches on [4] did not reveal any
    packages calling clone3().
    
    [1]: https://lore.kernel.org/r/CAG48ez3q=BeNcuVTKBN79kJui4vC6nw0Bfq6xc-i0neheT17TA@mail.gmail.com
    [2]: https://lore.kernel.org/r/20191028172143.4vnnjpdljfnexaq5@wittgenstein
    [3]: https://github.com/systemd/systemd/blob/5238e9575906297608ff802a27e2ff9effa3b338/src/basic/raw-clone.h#L31
    [4]: https://codesearch.debian.net
    Fixes: 7f192e3c
    
     ("fork: add clone3")
    Cc: Kees Cook <keescook@chromium.org>
    Cc: Jann Horn <jannh@google.com>
    Cc: David Howells <dhowells@redhat.com>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Oleg Nesterov <oleg@redhat.com>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Florian Weimer <fweimer@redhat.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: linux-api@vger.kernel.org
    Cc: linux-kernel@vger.kernel.org
    Cc: <stable@vger.kernel.org> # 5.3
    Cc: GNU C Library <libc-alpha@sourceware.org>
    Signed-off-by: default avatarChristian Brauner <christian.brauner@ubuntu.com>
    Acked-by: default avatarArnd Bergmann <arnd@arndb.de>
    Acked-by: default avatarAleksa Sarai <cyphar@cyphar.com>
    Link: https://lore.kernel.org/r/20191031113608.20713-1-christian.brauner@ubuntu.com
    fa729c4d