Skip to content

Commit 26bc672

Browse files
committed
Merge tag 'for-linus-2019-11-05' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux
Pull clone3 stack argument update from Christian Brauner: "This changes clone3() to do basic stack validation and to set up the stack depending on whether or not it is growing up or down. With clone3() the expectation is now very simply that the .stack argument points to the lowest address of the stack and that .stack_size specifies the initial stack size. This is diferent from legacy clone() where the "stack" argument had to point to the lowest or highest address of the stack depending on the architecture. 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 changing clone3() to determine stack direction and doing basic validation is the right course of action. Note, this is a potentially user visible change. In the very unlikely case, that it breaks someone's use-case we will revert. (And then e.g. place the new behavior under an appropriate flag.) Note that passing an empty stack will continue working just as before. Breaking someone's use-case is very unlikely. Neither glibc nor musl currently expose a wrapper for clone3(). There is currently also no real motivation for anyone to use clone3() directly. First, because using clone{3}() with stacks requires some assembly (see glibc and musl). Second, because 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. I did a codesearch on https://codesearch.debian.net, github, and gitlab and could not find any software currently relying directly on clone3(). I expect this to change once we land CLONE_CLEAR_SIGHAND which was a request coming from glibc at which point they'll likely start using it" * tag 'for-linus-2019-11-05' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux: clone3: validate stack arguments
2 parents 7111fa1 + fa729c4 commit 26bc672

File tree

2 files changed

+36
-1
lines changed

2 files changed

+36
-1
lines changed

include/uapi/linux/sched.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@
5151
* sent when the child exits.
5252
* @stack: Specify the location of the stack for the
5353
* child process.
54+
* Note, @stack is expected to point to the
55+
* lowest address. The stack direction will be
56+
* determined by the kernel and set up
57+
* appropriately based on @stack_size.
5458
* @stack_size: The size of the stack for the child process.
5559
* @tls: If CLONE_SETTLS is set, the tls descriptor
5660
* is set to tls.

kernel/fork.c

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2561,7 +2561,35 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs,
25612561
return 0;
25622562
}
25632563

2564-
static bool clone3_args_valid(const struct kernel_clone_args *kargs)
2564+
/**
2565+
* clone3_stack_valid - check and prepare stack
2566+
* @kargs: kernel clone args
2567+
*
2568+
* Verify that the stack arguments userspace gave us are sane.
2569+
* In addition, set the stack direction for userspace since it's easy for us to
2570+
* determine.
2571+
*/
2572+
static inline bool clone3_stack_valid(struct kernel_clone_args *kargs)
2573+
{
2574+
if (kargs->stack == 0) {
2575+
if (kargs->stack_size > 0)
2576+
return false;
2577+
} else {
2578+
if (kargs->stack_size == 0)
2579+
return false;
2580+
2581+
if (!access_ok((void __user *)kargs->stack, kargs->stack_size))
2582+
return false;
2583+
2584+
#if !defined(CONFIG_STACK_GROWSUP) && !defined(CONFIG_IA64)
2585+
kargs->stack += kargs->stack_size;
2586+
#endif
2587+
}
2588+
2589+
return true;
2590+
}
2591+
2592+
static bool clone3_args_valid(struct kernel_clone_args *kargs)
25652593
{
25662594
/*
25672595
* All lower bits of the flag word are taken.
@@ -2581,6 +2609,9 @@ static bool clone3_args_valid(const struct kernel_clone_args *kargs)
25812609
kargs->exit_signal)
25822610
return false;
25832611

2612+
if (!clone3_stack_valid(kargs))
2613+
return false;
2614+
25842615
return true;
25852616
}
25862617

0 commit comments

Comments
 (0)